Introduce NOT logical operator and add tests (#192)

hypertrace · Jan 17, 2024 · d4e5bff · d4e5bff
1 parent 1078778
commit d4e5bff
Show file tree

Hide file tree

Showing 9 changed files with 221 additions and 9 deletions.
diff --git a/...egrationTest/java/org/hypertrace/core/documentstore/ArrayFiltersQueryIntegrationTest.java b/...egrationTest/java/org/hypertrace/core/documentstore/ArrayFiltersQueryIntegrationTest.java
@@ -1,6 +1,7 @@
 package org.hypertrace.core.documentstore;
 
 import static org.hypertrace.core.documentstore.expression.impl.LogicalExpression.and;
+import static org.hypertrace.core.documentstore.expression.impl.LogicalExpression.not;
 import static org.hypertrace.core.documentstore.expression.operators.ArrayOperator.ANY;
 import static org.hypertrace.core.documentstore.model.config.DatabaseType.MONGO;
 import static org.hypertrace.core.documentstore.model.config.DatabaseType.POSTGRES;
@@ -161,7 +162,7 @@ public Stream<Arguments> provideArguments(final ExtensionContext context) {
 
   @ParameterizedTest
   @ArgumentsSource(AllProvider.class)
-  void getAllSolarSystemsWithAtLeastOnePlanetHavingBothWaterAndOxygen(final String dataStoreName)
+  void getSolarSystemsWithAtLeastOnePlanetHavingBothWaterAndOxygen(final String dataStoreName)
       throws JSONException {
     final Collection collection = getCollection(dataStoreName);
 
@@ -203,6 +204,98 @@ void getAllSolarSystemsWithAtLeastOnePlanetHavingBothWaterAndOxygen(final String
     JSONAssert.assertEquals(expected, actual, JSONCompareMode.LENIENT);
   }
 
+  @ParameterizedTest
+  @ArgumentsSource(AllProvider.class)
+  void getSolarSystemsWithAllThePlanetsHavingBothWaterAndOxygen(final String dataStoreName)
+      throws JSONException {
+    final Collection collection = getCollection(dataStoreName);
+
+    // All planets having both oxygen and water = No planet without both oxygen and water
+    final Query query =
+        Query.builder()
+            .setFilter(
+                not(
+                    DocumentArrayFilterExpression.builder()
+                        .operator(ANY)
+                        .arraySource(IdentifierExpression.of("additional_info.planets"))
+                        .filter(
+                            not(
+                                and(
+                                    ArrayRelationalFilterExpression.builder()
+                                        .operator(ANY)
+                                        .filter(
+                                            RelationalExpression.of(
+                                                IdentifierExpression.of("elements"),
+                                                RelationalOperator.EQ,
+                                                ConstantExpression.of("Oxygen")))
+                                        .build(),
+                                    ArrayRelationalFilterExpression.builder()
+                                        .operator(ANY)
+                                        .filter(
+                                            RelationalExpression.of(
+                                                IdentifierExpression.of("elements"),
+                                                RelationalOperator.EQ,
+                                                ConstantExpression.of("Water")))
+                                        .build(),
+                                    RelationalExpression.of(
+                                        IdentifierExpression.of("meta.num_moons"),
+                                        RelationalOperator.GT,
+                                        ConstantExpression.of(0)))))
+                        .build()))
+            .build();
+
+    final Iterator<Document> documents = collection.aggregate(query);
+    final String expected = readResource("all_planets_having_both_oxygen_and_water.json");
+    final String actual = iteratorToJson(documents);
+
+    JSONAssert.assertEquals(expected, actual, JSONCompareMode.LENIENT);
+  }
+
+  @ParameterizedTest
+  @ArgumentsSource(AllProvider.class)
+  void getSolarSystemsWithNoneOfThePlanetsHavingBothWaterAndOxygenTogether(
+      final String dataStoreName) throws JSONException {
+    final Collection collection = getCollection(dataStoreName);
+
+    final Query query =
+        Query.builder()
+            .setFilter(
+                not(
+                    DocumentArrayFilterExpression.builder()
+                        .operator(ANY)
+                        .arraySource(IdentifierExpression.of("additional_info.planets"))
+                        .filter(
+                            and(
+                                ArrayRelationalFilterExpression.builder()
+                                    .operator(ANY)
+                                    .filter(
+                                        RelationalExpression.of(
+                                            IdentifierExpression.of("elements"),
+                                            RelationalOperator.EQ,
+                                            ConstantExpression.of("Oxygen")))
+                                    .build(),
+                                ArrayRelationalFilterExpression.builder()
+                                    .operator(ANY)
+                                    .filter(
+                                        RelationalExpression.of(
+                                            IdentifierExpression.of("elements"),
+                                            RelationalOperator.EQ,
+                                            ConstantExpression.of("Water")))
+                                    .build(),
+                                RelationalExpression.of(
+                                    IdentifierExpression.of("meta.num_moons"),
+                                    RelationalOperator.GT,
+                                    ConstantExpression.of(0))))
+                        .build()))
+            .build();
+
+    final Iterator<Document> documents = collection.aggregate(query);
+    final String expected = readResource("no_planet_having_both_oxygen_and_water_together.json");
+    final String actual = iteratorToJson(documents);
+
+    JSONAssert.assertEquals(expected, actual, JSONCompareMode.LENIENT);
+  }
+
   private String readResource(final String fileName) {
     try {
       return new String(

diff --git a/...grationTest/resources/query/array_operators/all_planets_having_both_oxygen_and_water.json b/...grationTest/resources/query/array_operators/all_planets_having_both_oxygen_and_water.json
@@ -0,0 +1,31 @@
+[
+  {
+    "name": "Solar System 1",
+    "additional_info": {
+      "planets": [
+        {
+          "name": "Planet 1",
+          "elements": [
+            "Oxygen",
+            "Water",
+            "Nitrogen"
+          ],
+          "meta": {
+            "num_moons": 7
+          }
+        },
+        {
+          "name": "Planet 2",
+          "elements": [
+            "Oxygen",
+            "Helium",
+            "Water"
+          ],
+          "meta": {
+            "num_moons": 1
+          }
+        }
+      ]
+    }
+  }
+]
diff --git a/...Test/resources/query/array_operators/no_planet_having_both_oxygen_and_water_together.json b/...Test/resources/query/array_operators/no_planet_having_both_oxygen_and_water_together.json
@@ -0,0 +1,56 @@
+[
+  {
+    "name": "Solar System 2",
+    "additional_info": {
+      "planets": [
+        {
+          "name": "Planet 1"
+        },
+        {
+          "name": "Planet 2"
+        },
+        {
+          "name": "Planet 3",
+          "elements": [
+            "Oxygen",
+            "Nitrogen"
+          ]
+        },
+        {
+          "name": "Planet 4",
+          "elements": [
+            "Iron"
+          ]
+        },
+        {
+          "name": "Planet 5",
+          "elements": []
+        },
+        {
+          "name": "Planet 6",
+          "elements": []
+        },
+        {
+          "name": "Planet 7",
+          "elements": [
+            "Hydrogen",
+            "Helium",
+            "Methane"
+          ],
+          "meta": {
+            "num_moons": 10
+          }
+        },
+        {
+          "name": "Planet 8",
+          "elements": [
+            "Hydrogen",
+            "Helium",
+            "Methane",
+            "Ammonia"
+          ]
+        }
+      ]
+    }
+  }
+]
diff --git a/...re/src/main/java/org/hypertrace/core/documentstore/expression/impl/LogicalExpression.java b/...re/src/main/java/org/hypertrace/core/documentstore/expression/impl/LogicalExpression.java
@@ -3,6 +3,7 @@
 import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toUnmodifiableList;
 import static org.hypertrace.core.documentstore.expression.operators.LogicalOperator.AND;
+import static org.hypertrace.core.documentstore.expression.operators.LogicalOperator.NOT;
 import static org.hypertrace.core.documentstore.expression.operators.LogicalOperator.OR;
 
 import com.google.common.base.Preconditions;
@@ -74,6 +75,10 @@ public static LogicalExpression or(
     return buildLogicalExpression(operand1, operand2, otherOperands, OR);
   }
 
+  public static LogicalExpression not(final FilterTypeExpression operand) {
+    return buildLogicalExpression(List.of(operand), NOT);
+  }
+
   private static LogicalExpression buildLogicalExpression(
       final FilterTypeExpression operand1,
       final FilterTypeExpression operand2,
@@ -102,18 +107,28 @@ public <T> T accept(final FilterTypeExpressionVisitor visitor) {
 
   @Override
   public String toString() {
+    if (NOT.equals(operator)) {
+      return String.format("NOT (%s)", operands.get(0));
+    }
+
     return "("
         + operands.stream().map(String::valueOf).collect(joining(") " + operator + " ("))
         + ")";
   }
 
   public static class LogicalExpressionBuilder {
     public LogicalExpression build() {
+      Preconditions.checkArgument(operator != null, "operator is null");
       Preconditions.checkArgument(operands != null, "operands is null");
-      Preconditions.checkArgument(operands.size() >= 2, "At least 2 operands required");
+
+      if (NOT.equals(operator)) {
+        Preconditions.checkArgument(operands.size() == 1, "Exactly one operand required for NOT");
+      } else {
+        Preconditions.checkArgument(operands.size() >= 2, "At least 2 operands required");
+      }
+
       Preconditions.checkArgument(
           operands.stream().noneMatch(Objects::isNull), "One or more operands is null");
-      Preconditions.checkArgument(operator != null, "operator is null");
       return new LogicalExpression(operands, operator);
     }
   }

diff --git a/...src/main/java/org/hypertrace/core/documentstore/expression/operators/LogicalOperator.java b/...src/main/java/org/hypertrace/core/documentstore/expression/operators/LogicalOperator.java
@@ -3,4 +3,5 @@
 public enum LogicalOperator {
   AND,
   OR,
+  NOT,
 }
diff --git a/...main/java/org/hypertrace/core/documentstore/model/config/mongo/MongoConnectionConfig.java b/...main/java/org/hypertrace/core/documentstore/model/config/mongo/MongoConnectionConfig.java
@@ -140,7 +140,7 @@ private void applyClusterSettings(final Builder settingsBuilder) {
   private void applyConnectionPoolSettings(final Builder settingsBuilder) {
     final ConnectionPoolSettings connectionPoolSettings =
         ConnectionPoolSettings.builder()
-            .maxConnecting(connectionPoolConfig.maxConnections())
+            .maxSize(connectionPoolConfig.maxConnections())
             .maxWaitTime(
                 connectionPoolConfig.connectionAccessTimeout().toMillis(), TimeUnit.MILLISECONDS)
             .maxConnectionIdleTime(

diff --git a/...va/org/hypertrace/core/documentstore/mongo/query/parser/MongoLogicalExpressionParser.java b/...va/org/hypertrace/core/documentstore/mongo/query/parser/MongoLogicalExpressionParser.java
@@ -2,8 +2,10 @@
 
 import static java.util.Collections.unmodifiableMap;
 import static org.hypertrace.core.documentstore.expression.operators.LogicalOperator.AND;
+import static org.hypertrace.core.documentstore.expression.operators.LogicalOperator.NOT;
 import static org.hypertrace.core.documentstore.expression.operators.LogicalOperator.OR;
 import static org.hypertrace.core.documentstore.mongo.MongoUtils.getUnsupportedOperationException;
+import static org.hypertrace.core.documentstore.mongo.query.parser.filter.MongoRelationalFilterParserFactory.FilterLocation.INSIDE_EXPR;
 
 import java.util.EnumMap;
 import java.util.List;
@@ -15,6 +17,10 @@
 import org.hypertrace.core.documentstore.parser.FilterTypeExpressionVisitor;
 
 final class MongoLogicalExpressionParser {
+
+  private static final String NOR_OP = "$nor";
+  private static final String NOT_OP = "$not";
+
   private static final Map<LogicalOperator, String> KEY_MAP =
       unmodifiableMap(
           new EnumMap<>(LogicalOperator.class) {
@@ -32,19 +38,24 @@ final class MongoLogicalExpressionParser {
 
   Map<String, Object> parse(final LogicalExpression expression) {
     LogicalOperator operator = expression.getOperator();
-    String key = KEY_MAP.get(operator);
+    String key = NOT.equals(operator) ? getNotOp() : KEY_MAP.get(operator);
 
     if (key == null) {
       throw getUnsupportedOperationException(operator);
     }
 
     FilterTypeExpressionVisitor parser =
         new MongoFilterTypeExpressionParser(relationalFilterContext);
+
     List<Object> parsed =
         expression.getOperands().stream()
             .map(exp -> exp.accept(parser))
             .collect(Collectors.toList());
 
     return Map.of(key, parsed);
   }
+
+  private String getNotOp() {
+    return INSIDE_EXPR.equals(relationalFilterContext.location()) ? NOT_OP : NOR_OP;
+  }
 }
diff --git a/...ace/core/documentstore/postgres/query/v1/vistors/PostgresFilterTypeExpressionVisitor.java b/...ace/core/documentstore/postgres/query/v1/vistors/PostgresFilterTypeExpressionVisitor.java
@@ -1,6 +1,7 @@
 package org.hypertrace.core.documentstore.postgres.query.v1.vistors;
 
 import static java.util.stream.Collectors.toUnmodifiableList;
+import static org.hypertrace.core.documentstore.expression.operators.LogicalOperator.NOT;
 import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.EQ;
 import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.IN;
 import static org.hypertrace.core.documentstore.postgres.PostgresCollection.ID;
@@ -45,6 +46,10 @@ public PostgresFilterTypeExpressionVisitor(
   @SuppressWarnings("unchecked")
   @Override
   public String visit(final LogicalExpression expression) {
+    if (NOT.equals(expression.getOperator())) {
+      return String.format("NOT (%s)", expression.getOperands().get(0).accept(this).toString());
+    }
+
     Collector<String, ?, String> collector =
         getCollectorForLogicalOperator(expression.getOperator());
     String childList =

diff --git a/...c/test/java/org/hypertrace/core/documentstore/model/config/MongoConnectionConfigTest.java b/...c/test/java/org/hypertrace/core/documentstore/model/config/MongoConnectionConfigTest.java
@@ -27,7 +27,7 @@ void testAllDefaults() {
             .applyToConnectionPoolSettings(
                 builder ->
                     builder
-                        .maxConnecting(16)
+                        .maxSize(16)
                         .maxWaitTime(10, TimeUnit.SECONDS)
                         .maxConnectionIdleTime(5, TimeUnit.MINUTES))
             .build();
@@ -81,7 +81,7 @@ void testAllAssigned() {
             .applyToConnectionPoolSettings(
                 builder ->
                     builder
-                        .maxConnecting(maxPoolConnections)
+                        .maxSize(maxPoolConnections)
                         .maxWaitTime(maxWaitTime.toSeconds(), TimeUnit.SECONDS)
                         .maxConnectionIdleTime(maxIdleTime.toSeconds(), TimeUnit.SECONDS))
             .build();
@@ -124,7 +124,7 @@ void testMissingAuthDb_shouldUseTheSameDatabaseUsedForConnecting() {
             .applyToConnectionPoolSettings(
                 builder ->
                     builder
-                        .maxConnecting(16)
+                        .maxSize(16)
                         .maxWaitTime(10, TimeUnit.SECONDS)
                         .maxConnectionIdleTime(5, TimeUnit.MINUTES))
             .build();
@@ -159,7 +159,7 @@ void testDefaultCredentials_shouldNotSetCredentials() {
             .applyToConnectionPoolSettings(
                 builder ->
                     builder
-                        .maxConnecting(16)
+                        .maxSize(16)
                         .maxWaitTime(10, TimeUnit.SECONDS)
                         .maxConnectionIdleTime(5, TimeUnit.MINUTES))
             .build();
-Original file line number
+Diff line change
@@ Expand Up / @@ -3,4 +3,5 @@ @@
     public enum LogicalOperator {
       AND,
       OR,
+      NOT,
     }