From 757b4c945ef631999f1121bb655c19f8b44662cd Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Wed, 20 Dec 2023 22:42:53 -0500 Subject: [PATCH 01/19] First version of `$not` operator --- .../filter/ArrayComparisonOperator.java | 16 ++++- .../clause/filter/ComparisonExpression.java | 28 ++++++++- .../filter/ElementComparisonOperator.java | 5 ++ .../command/clause/filter/FilterOperator.java | 7 +++ .../clause/filter/LogicalExpression.java | 58 ++++++++++++++++++- .../filter/ValueComparisonOperator.java | 23 ++++++++ .../FilterClauseDeserializer.java | 5 ++ .../operation/model/impl/DBFilterBase.java | 4 +- .../impl/matcher/FilterableResolver.java | 12 +++- .../model/impl/FindOperationTest.java | 5 +- 10 files changed, 155 insertions(+), 8 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ArrayComparisonOperator.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ArrayComparisonOperator.java index a4f034b2e3..9046dfb461 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ArrayComparisonOperator.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ArrayComparisonOperator.java @@ -3,7 +3,9 @@ /** List of element level operator that can be used in Filter clause */ public enum ArrayComparisonOperator implements FilterOperator { ALL("$all"), - SIZE("$size"); + SIZE("$size"), + // Will not be supported from outside + NOTANY("$notany"); private String operator; @@ -15,4 +17,16 @@ public enum ArrayComparisonOperator implements FilterOperator { public String getOperator() { return operator; } + + @Override + public FilterOperator flip() { + switch (this) { + case ALL: + return NOTANY; + case NOTANY: + return ALL; + default: + return this; + } + } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ComparisonExpression.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ComparisonExpression.java index bc4fa2b179..566cf2db5b 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ComparisonExpression.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ComparisonExpression.java @@ -21,7 +21,7 @@ public class ComparisonExpression { private final String path; - @Valid @NotEmpty private final List> filterOperations; + @Valid @NotEmpty private List> filterOperations; private List dbFilters; @@ -33,6 +33,32 @@ public List> getFilterOperations() { return filterOperations; } + /** This method will be called when not operation is pushed down */ + public void flip() { + List> filterOperations = new ArrayList<>(this.filterOperations.size()); + for (FilterOperation filterOperation : this.filterOperations) { + final FilterOperator flipedOperator = filterOperation.operator().flip(); + JsonLiteral operand = + getFlippedOperandValue(filterOperation.operator(), filterOperation.operand()); + filterOperations.add(new ValueComparisonOperation<>(flipedOperator, operand)); + } + this.filterOperations = filterOperations; + } + + /** + * This method is used to flip the operand value when not operator is applied, e.g. $exists, $size + */ + private JsonLiteral getFlippedOperandValue(FilterOperator operator, JsonLiteral operand) { + if (operator == ElementComparisonOperator.EXISTS) { + return new JsonLiteral(!((Boolean) operand.value()), operand.type()); + } else if (operator == ArrayComparisonOperator.SIZE) { + return new JsonLiteral( + ((BigDecimal) operand.value()).multiply(new BigDecimal(-1)), operand.type()); + } else { + return operand; + } + } + public ComparisonExpression( @NotBlank(message = "json node path can not be null in filter") String path, List> filterOperations, diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ElementComparisonOperator.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ElementComparisonOperator.java index 227be6d95f..c840ae8ba4 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ElementComparisonOperator.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ElementComparisonOperator.java @@ -14,4 +14,9 @@ public enum ElementComparisonOperator implements FilterOperator { public String getOperator() { return operator; } + + @Override + public FilterOperator flip() { + return this; + } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/FilterOperator.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/FilterOperator.java index 9e311b50c1..661ed5a44c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/FilterOperator.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/FilterOperator.java @@ -9,6 +9,13 @@ public interface FilterOperator { String getOperator(); + /** + * Flip comparison operator when `$not` is pushed down + * + * @return + */ + FilterOperator flip(); + class FilterOperatorUtils { private static Map operatorMap = new HashMap<>(); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java index 4df4151e7b..e2bec4bbd4 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java @@ -13,7 +13,8 @@ public class LogicalExpression { public enum LogicalOperator { AND("$and"), - OR("$or"); + OR("$or"), + NOT("$not"); private String operator; LogicalOperator(String operator) { @@ -25,9 +26,58 @@ public String getOperator() { } } - private final LogicalOperator logicalRelation; + private LogicalOperator logicalRelation; private int totalComparisonExpressionCount; private int totalIdComparisonExpressionCount; + private boolean notFlip = false; + + /** This method will flip the operators and operand if logical operator is not */ + public void checkAndFlip() { + if (logicalRelation == LogicalOperator.NOT) { + flip(); + } + for (LogicalExpression logicalExpression : logicalExpressions) { + logicalExpression.checkAndFlip(); + } + handleChild(); + } + + private void handleChild() { + List newLogicalList = new ArrayList<>(); + List newComparisonList = new ArrayList<>(); + for (LogicalExpression logicalExpression : logicalExpressions) { + if (logicalExpression.logicalRelation == LogicalOperator.NOT) { + newLogicalList.addAll(logicalExpression.logicalExpressions); + if (logicalExpression.comparisonExpressions.size() > 1) { + // Multiple conditions in not, after push down will become or + final LogicalExpression orLogic = LogicalExpression.or(); + orLogic.comparisonExpressions = logicalExpression.comparisonExpressions; + newLogicalList.add(orLogic); + } else { + // Unary not, after push down will become additional condition + newComparisonList.addAll(logicalExpression.comparisonExpressions); + } + } else { + newLogicalList.add(logicalExpression); + } + } + logicalExpressions = newLogicalList; + comparisonExpressions.addAll(newComparisonList); + } + + private void flip() { + if (logicalRelation == LogicalOperator.AND) { + logicalRelation = LogicalOperator.OR; + } else if (logicalRelation == LogicalOperator.OR) { + logicalRelation = LogicalOperator.AND; + } + for (LogicalExpression logicalExpression : logicalExpressions) { + logicalExpression.flip(); + } + for (ComparisonExpression comparisonExpression : comparisonExpressions) { + comparisonExpression.flip(); + } + } public List logicalExpressions; public List comparisonExpressions; @@ -51,6 +101,10 @@ public static LogicalExpression or() { return new LogicalExpression(LogicalOperator.OR, 0, new ArrayList<>(), new ArrayList<>()); } + public static LogicalExpression not() { + return new LogicalExpression(LogicalOperator.NOT, 0, new ArrayList<>(), new ArrayList<>()); + } + public void addLogicalExpression(LogicalExpression logicalExpression) { // skip empty logic expression if (logicalExpression.isEmpty()) { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ValueComparisonOperator.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ValueComparisonOperator.java index 48602d86d7..636804e1bd 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ValueComparisonOperator.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ValueComparisonOperator.java @@ -24,4 +24,27 @@ public enum ValueComparisonOperator implements FilterOperator { public String getOperator() { return operator; } + + @Override + public FilterOperator flip() { + switch (this) { + case EQ: + return NE; + case NE: + return EQ; + case IN: + return NIN; + case NIN: + return IN; + case GT: + return LT; + case GTE: + return LTE; + case LT: + return GT; + case LTE: + return GTE; + } + return this; + } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java index 98037c3dc2..29bb915f0a 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java @@ -45,6 +45,8 @@ public FilterClause deserialize( LogicalExpression implicitAnd = LogicalExpression.and(); populateExpression(implicitAnd, filterNode); validate(implicitAnd); + // push down not operator + implicitAnd.checkAndFlip(); return new FilterClause(implicitAnd); } @@ -94,6 +96,9 @@ private void populateExpression( case "$or": innerLogicalExpression = LogicalExpression.or(); break; + case "$not": + innerLogicalExpression = LogicalExpression.not(); + break; case DocumentConstants.Fields.VECTOR_EMBEDDING_FIELD: throw new JsonApiException( ErrorCode.INVALID_FILTER_EXPRESSION, diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java index 501b7055b7..a8503196a0 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java @@ -614,8 +614,8 @@ boolean canAddField() { /** Filter for document where array has specified number of elements */ public static class SizeFilter extends MapFilterBase { - public SizeFilter(String path, Integer size) { - super("array_size", path, Operator.MAP_EQUALS, size); + public SizeFilter(String path, Operator operator, Integer size) { + super("array_size", path, operator, size); } @Override diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterableResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterableResolver.java index ee2a755bed..3263319085 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterableResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterableResolver.java @@ -301,7 +301,17 @@ public static List findDynamic(CaptureExpression captureExpression if (captureExpression.marker() == SIZE_GROUP) { BigDecimal bigDecimal = (BigDecimal) filterOperation.operand().value(); - filters.add(new DBFilterBase.SizeFilter(captureExpression.path(), bigDecimal.intValue())); + // Flipping size operator will multiply the value by -1 + // Negative means check array_size[?] != ? + int size = bigDecimal.intValue(); + DBFilterBase.MapFilterBase.Operator operator; + if (size > 0) { + operator = DBFilterBase.MapFilterBase.Operator.MAP_EQUALS; + } else { + operator = DBFilterBase.MapFilterBase.Operator.MAP_NOT_EQUALS; + } + filters.add( + new DBFilterBase.SizeFilter(captureExpression.path(), operator, Math.abs(size))); } if (captureExpression.marker() == ARRAY_EQUALS) { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java index 8648bff53e..e48bb1fd71 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java @@ -1157,7 +1157,10 @@ public void findWithSizeFilter() throws Exception { LogicalExpression implicitAnd = LogicalExpression.and(); implicitAnd.comparisonExpressions.add(new ComparisonExpression(null, null, null)); - List filters = List.of(new DBFilterBase.SizeFilter("tags", 2)); + List filters = + List.of( + new DBFilterBase.SizeFilter( + "tags", DBFilterBase.MapFilterBase.Operator.MAP_EQUALS, 2)); implicitAnd.comparisonExpressions.get(0).setDBFilters(filters); FindOperation operation = From 56207761ef18bd47c5ac7d338a6562b6b2ac16ab Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Wed, 20 Dec 2023 23:02:48 -0500 Subject: [PATCH 02/19] Checking in with bug of not handling OR logic for All and not(ALL) --- .../command/clause/filter/FilterOperator.java | 2 ++ .../operation/model/impl/DBFilterBase.java | 20 +++++++++++++++++++ .../impl/matcher/FilterableResolver.java | 12 +++++++++++ 3 files changed, 34 insertions(+) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/FilterOperator.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/FilterOperator.java index 661ed5a44c..66326c9651 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/FilterOperator.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/FilterOperator.java @@ -29,6 +29,8 @@ class FilterOperatorUtils { for (FilterOperator filterOperator : ArrayComparisonOperator.values()) { addComparisonOperator(filterOperator); } + // This should not be supported from outside + operatorMap.remove(ArrayComparisonOperator.NOTANY.getOperator()); } private static void addComparisonOperator(FilterOperator filterOperator) { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java index a8503196a0..d87c324c9c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java @@ -612,6 +612,26 @@ boolean canAddField() { } } + /** Filter for document where all values exists for an array */ + public static class NotAnyFilter extends SetFilterBase { + private final Object arrayValue; + + public NotAnyFilter(DocValueHasher hasher, String path, Object arrayValue) { + super("array_contains", path, getHashValue(hasher, path, arrayValue), Operator.NOT_CONTAINS); + this.arrayValue = arrayValue; + } + + @Override + JsonNode asJson(JsonNodeFactory nodeFactory) { + return DBFilterBase.getJsonNode(nodeFactory, arrayValue); + } + + @Override + boolean canAddField() { + return false; + } + } + /** Filter for document where array has specified number of elements */ public static class SizeFilter extends MapFilterBase { public SizeFilter(String path, Operator operator, Integer size) { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterableResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterableResolver.java index 3263319085..17bb2231ee 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterableResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterableResolver.java @@ -39,6 +39,7 @@ public abstract class FilterableResolver { private static final Object DYNAMIC_DATE_GROUP = new Object(); private static final Object EXISTS_GROUP = new Object(); private static final Object ALL_GROUP = new Object(); + private static final Object NOT_ANY_GROUP = new Object(); private static final Object SIZE_GROUP = new Object(); private static final Object ARRAY_EQUALS = new Object(); private static final Object SUB_DOC_EQUALS = new Object(); @@ -126,6 +127,8 @@ public FilterableResolver() { .compareValues("*", EnumSet.of(ElementComparisonOperator.EXISTS), JsonType.BOOLEAN) .capture(ALL_GROUP) .compareValues("*", EnumSet.of(ArrayComparisonOperator.ALL), JsonType.ARRAY) + .capture(NOT_ANY_GROUP) + .compareValues("*", EnumSet.of(ArrayComparisonOperator.NOTANY), JsonType.ARRAY) .capture(SIZE_GROUP) .compareValues("*", EnumSet.of(ArrayComparisonOperator.SIZE), JsonType.NUMBER) .capture(ARRAY_EQUALS) @@ -299,6 +302,15 @@ public static List findDynamic(CaptureExpression captureExpression } } + if (captureExpression.marker() == NOT_ANY_GROUP) { + final DocValueHasher docValueHasher = new DocValueHasher(); + List objects = (List) filterOperation.operand().value(); + for (Object arrayValue : objects) { + filters.add( + new DBFilterBase.NotAnyFilter(docValueHasher, captureExpression.path(), arrayValue)); + } + } + if (captureExpression.marker() == SIZE_GROUP) { BigDecimal bigDecimal = (BigDecimal) filterOperation.operand().value(); // Flipping size operator will multiply the value by -1 From f416392a480b15c7b9e7fb5e0bdd6e6c0bce3f61 Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Thu, 21 Dec 2023 12:47:44 -0500 Subject: [PATCH 03/19] Added test case --- .../clause/filter/LogicalExpression.java | 52 +-- .../FilterClauseDeserializer.java | 2 +- .../FilterClauseDeserializerTest.java | 414 +++++++++++++++++- 3 files changed, 422 insertions(+), 46 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java index e2bec4bbd4..b7068b06d2 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java @@ -29,40 +29,42 @@ public String getOperator() { private LogicalOperator logicalRelation; private int totalComparisonExpressionCount; private int totalIdComparisonExpressionCount; - private boolean notFlip = false; /** This method will flip the operators and operand if logical operator is not */ - public void checkAndFlip() { + public void dfs(LogicalExpression parent) { + List tempLogicalExpressions = new ArrayList<>(logicalExpressions); + for (LogicalExpression logicalExpression : tempLogicalExpressions) { + logicalExpression.dfs(this); + } + + Iterator iterator = logicalExpressions.iterator(); + while (iterator.hasNext()) { + LogicalExpression logicalExpression = iterator.next(); + if (logicalExpression.logicalRelation == LogicalOperator.NOT) { + iterator.remove(); + } + } + if (logicalRelation == LogicalOperator.NOT) { flip(); + addToParent(parent); } - for (LogicalExpression logicalExpression : logicalExpressions) { - logicalExpression.checkAndFlip(); - } - handleChild(); } - private void handleChild() { - List newLogicalList = new ArrayList<>(); - List newComparisonList = new ArrayList<>(); - for (LogicalExpression logicalExpression : logicalExpressions) { - if (logicalExpression.logicalRelation == LogicalOperator.NOT) { - newLogicalList.addAll(logicalExpression.logicalExpressions); - if (logicalExpression.comparisonExpressions.size() > 1) { - // Multiple conditions in not, after push down will become or - final LogicalExpression orLogic = LogicalExpression.or(); - orLogic.comparisonExpressions = logicalExpression.comparisonExpressions; - newLogicalList.add(orLogic); - } else { - // Unary not, after push down will become additional condition - newComparisonList.addAll(logicalExpression.comparisonExpressions); - } - } else { - newLogicalList.add(logicalExpression); + private void addToParent(LogicalExpression parent) { + logicalExpressions.stream().forEach(expression -> parent.addLogicalExpression(expression)); + if (comparisonExpressions.size() > 1) { + // Multiple conditions in not, after push down will become or + final LogicalExpression orLogic = LogicalExpression.or(); + orLogic.comparisonExpressions.addAll(comparisonExpressions); + parent.addLogicalExpression(orLogic); + } else { + if (comparisonExpressions.size() == 1) { + // Unary not, after push down will become additional condition + parent.addComparisonExpression(comparisonExpressions.get(0)); } } - logicalExpressions = newLogicalList; - comparisonExpressions.addAll(newComparisonList); + comparisonExpressions.clear(); } private void flip() { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java index 29bb915f0a..cd1eb31e9e 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java @@ -46,7 +46,7 @@ public FilterClause deserialize( populateExpression(implicitAnd, filterNode); validate(implicitAnd); // push down not operator - implicitAnd.checkAndFlip(); + implicitAnd.dfs(null); return new FilterClause(implicitAnd); } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java index 1e8a6014a2..6f33d04f20 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java @@ -768,30 +768,283 @@ ValueComparisonOperator.EQ, new JsonLiteral("testAge", JsonType.STRING))), } @Test - public void nestedOrAnd() throws Exception { + public void simpleNot() throws Exception { + String json = + """ + { + "$not":[ + {"name" : "testName"}, + {"age" : "testAge"} + ] + } + """; + final ComparisonExpression expectedResult1 = + new ComparisonExpression( + "name", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.NE, new JsonLiteral("testName", JsonType.STRING))), + null); + final ComparisonExpression expectedResult2 = + new ComparisonExpression( + "age", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.NE, new JsonLiteral("testAge", JsonType.STRING))), + null); + FilterClause filterClause = objectMapper.readValue(json, FilterClause.class); + assertThat(filterClause.logicalExpression().logicalExpressions).hasSize(1); + assertThat(filterClause.logicalExpression().logicalExpressions.get(0).getLogicalRelation()) + .isEqualTo(LogicalExpression.LogicalOperator.OR); + assertThat(filterClause.logicalExpression().logicalExpressions.get(0).comparisonExpressions) + .hasSize(2); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(0) + .getFilterOperations()) + .isEqualTo(expectedResult1.getFilterOperations()); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(1) + .getFilterOperations()) + .isEqualTo(expectedResult2.getFilterOperations()); + } + @Test + public void comparisonOperatorsWithNot() throws Exception { String json = """ { - "$and": [ - { - "name": "testName" - }, - { - "age": "testAge" - }, - { - "$or": [ - { - "address": "testAddress" - }, - { - "height": "testHeight" - } - ] - } - ] - } + "$not":[ + {"f1" : {"$eq" : "testName"}}, + {"f2" : {"$ne" : "testName"}}, + {"f3" : {"$in" : ["testName1","testName2"]}}, + {"f4" : {"$nin" : ["testName1","testName2"]}}, + {"f5" : {"$lt" : 5}}, + {"f6" : {"$lte" : 5}}, + {"f7" : {"$gt" : 5}}, + {"f8" : {"$gte" : 5}}, + {"f9" : {"$exists" : true}}, + {"f10" : {"$exists" : false}}, + {"f11" : {"$size" : 1}} + ] + } + """; + final ComparisonExpression eq = + new ComparisonExpression( + "f1", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.NE, new JsonLiteral("testName", JsonType.STRING))), + null); + final ComparisonExpression ne = + new ComparisonExpression( + "f2", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.EQ, new JsonLiteral("testName", JsonType.STRING))), + null); + final ComparisonExpression in = + new ComparisonExpression( + "f3", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.NIN, + new JsonLiteral(List.of("testName1", "testName2"), JsonType.ARRAY))), + null); + + final ComparisonExpression nin = + new ComparisonExpression( + "f4", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.IN, + new JsonLiteral(List.of("testName1", "testName2"), JsonType.ARRAY))), + null); + final ComparisonExpression lt = + new ComparisonExpression( + "f5", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.GT, + new JsonLiteral(new BigDecimal(5), JsonType.NUMBER))), + null); + + final ComparisonExpression lte = + new ComparisonExpression( + "f6", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.GTE, + new JsonLiteral(new BigDecimal(5), JsonType.NUMBER))), + null); + + final ComparisonExpression gt = + new ComparisonExpression( + "f7", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.LT, + new JsonLiteral(new BigDecimal(5), JsonType.NUMBER))), + null); + final ComparisonExpression gte = + new ComparisonExpression( + "f8", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.LTE, + new JsonLiteral(new BigDecimal(5), JsonType.NUMBER))), + null); + + final ComparisonExpression existsTrue = + new ComparisonExpression( + "f9", + List.of( + new ValueComparisonOperation( + ElementComparisonOperator.EXISTS, new JsonLiteral(false, JsonType.BOOLEAN))), + null); + final ComparisonExpression existsFalse = + new ComparisonExpression( + "f10", + List.of( + new ValueComparisonOperation( + ElementComparisonOperator.EXISTS, new JsonLiteral(true, JsonType.BOOLEAN))), + null); + final ComparisonExpression size = + new ComparisonExpression( + "f11", + List.of( + new ValueComparisonOperation( + ArrayComparisonOperator.SIZE, + new JsonLiteral(new BigDecimal(-1), JsonType.NUMBER))), + null); + FilterClause filterClause = objectMapper.readValue(json, FilterClause.class); + assertThat(filterClause.logicalExpression().logicalExpressions).hasSize(1); + assertThat(filterClause.logicalExpression().logicalExpressions.get(0).getLogicalRelation()) + .isEqualTo(LogicalExpression.LogicalOperator.OR); + assertThat(filterClause.logicalExpression().logicalExpressions.get(0).comparisonExpressions) + .hasSize(11); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(0) + .getFilterOperations()) + .isEqualTo(eq.getFilterOperations()); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(1) + .getFilterOperations()) + .isEqualTo(ne.getFilterOperations()); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(2) + .getFilterOperations()) + .isEqualTo(in.getFilterOperations()); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(3) + .getFilterOperations()) + .isEqualTo(nin.getFilterOperations()); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(4) + .getFilterOperations()) + .isEqualTo(lt.getFilterOperations()); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(5) + .getFilterOperations()) + .isEqualTo(lte.getFilterOperations()); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(6) + .getFilterOperations()) + .isEqualTo(gt.getFilterOperations()); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(7) + .getFilterOperations()) + .isEqualTo(gte.getFilterOperations()); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(8) + .getFilterOperations()) + .isEqualTo(existsTrue.getFilterOperations()); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(9) + .getFilterOperations()) + .isEqualTo(existsFalse.getFilterOperations()); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(10) + .getFilterOperations()) + .isEqualTo(size.getFilterOperations()); + } + + @Test + public void twoLevelNot() throws Exception { + String json = + """ + { + "$not":[ + { "$not" : [ + {"name" : "testName"}, + {"age" : "testAge"} + ]} + ] + } """; final ComparisonExpression expectedResult1 = new ComparisonExpression( @@ -807,6 +1060,127 @@ ValueComparisonOperator.EQ, new JsonLiteral("testName", JsonType.STRING))), new ValueComparisonOperation( ValueComparisonOperator.EQ, new JsonLiteral("testAge", JsonType.STRING))), null); + FilterClause filterClause = objectMapper.readValue(json, FilterClause.class); + assertThat(filterClause.logicalExpression().logicalExpressions).hasSize(1); + assertThat(filterClause.logicalExpression().logicalExpressions.get(0).getLogicalRelation()) + .isEqualTo(LogicalExpression.LogicalOperator.AND); + assertThat(filterClause.logicalExpression().logicalExpressions.get(0).comparisonExpressions) + .hasSize(2); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(0) + .getFilterOperations()) + .isEqualTo(expectedResult1.getFilterOperations()); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(1) + .getFilterOperations()) + .isEqualTo(expectedResult2.getFilterOperations()); + } + + @Test + public void multipleNot() throws Exception { + String json = + """ + { + "$not":[ + { "$not" : [ + { "$not" : [ + {"name" : "testName"}, + {"age" : "testAge"} + ]} + ]} + ] + } + """; + final ComparisonExpression expectedResult1 = + new ComparisonExpression( + "name", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.NE, new JsonLiteral("testName", JsonType.STRING))), + null); + final ComparisonExpression expectedResult2 = + new ComparisonExpression( + "age", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.NE, new JsonLiteral("testAge", JsonType.STRING))), + null); + FilterClause filterClause = objectMapper.readValue(json, FilterClause.class); + assertThat(filterClause.logicalExpression().logicalExpressions).hasSize(1); + assertThat(filterClause.logicalExpression().logicalExpressions.get(0).getLogicalRelation()) + .isEqualTo(LogicalExpression.LogicalOperator.OR); + assertThat(filterClause.logicalExpression().logicalExpressions.get(0).comparisonExpressions) + .hasSize(2); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(0) + .getFilterOperations()) + .isEqualTo(expectedResult1.getFilterOperations()); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(1) + .getFilterOperations()) + .isEqualTo(expectedResult2.getFilterOperations()); + } + + @Test + public void nestedOrAnd() throws Exception { + + String json = + """ + { + "$and": [ + { + "name": "testName" + }, + { + "age": "testAge" + }, + { + "$or": [ + { + "address": "testAddress" + }, + { + "height": "testHeight" + } + ] + } + ] + } + """; + final ComparisonExpression expectedResult1 = + new ComparisonExpression( + "name", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.EQ, new JsonLiteral("testName", JsonType.STRING))), + null); + final ComparisonExpression expectedResult2 = + new ComparisonExpression( + "age", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.EQ, new JsonLiteral("testAge", JsonType.STRING))), + null); final ComparisonExpression expectedResult3 = new ComparisonExpression( "address", From 269537e6d8841220dd16de4e5851b85ac38cfd13 Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Thu, 21 Dec 2023 13:39:42 -0500 Subject: [PATCH 04/19] Added test cases --- .../FilterClauseDeserializerTest.java | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java index 6f33d04f20..df77bc34cf 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java @@ -1252,6 +1252,130 @@ ValueComparisonOperator.EQ, new JsonLiteral("testHeight", JsonType.STRING))), .isEqualTo(expectedResult4.getFilterOperations()); } + @Test + public void multipleLevel() throws Exception { + String json = + """ + { + "$and": [ + { + "name": "testName" + }, + { + "age": "testAge" + }, + { + "$not": [ + { + "$or": [ + { + "address": "testAddress" + }, + { + "tags": { "$size" : 1} + } + ] + } + ] + } + ] + } + """; + final ComparisonExpression expectedResult1 = + new ComparisonExpression( + "name", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.EQ, new JsonLiteral("testName", JsonType.STRING))), + null); + final ComparisonExpression expectedResult2 = + new ComparisonExpression( + "age", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.EQ, new JsonLiteral("testAge", JsonType.STRING))), + null); + final ComparisonExpression expectedResult3 = + new ComparisonExpression( + "address", + List.of( + new ValueComparisonOperation( + ValueComparisonOperator.NE, new JsonLiteral("testAddress", JsonType.STRING))), + null); + final ComparisonExpression expectedResult4 = + new ComparisonExpression( + "tags", + List.of( + new ValueComparisonOperation( + ArrayComparisonOperator.SIZE, + new JsonLiteral(new BigDecimal(-1), JsonType.NUMBER))), + null); + FilterClause filterClause = objectMapper.readValue(json, FilterClause.class); + assertThat(filterClause.logicalExpression().logicalExpressions.get(0).getLogicalRelation()) + .isEqualTo(LogicalExpression.LogicalOperator.AND); + assertThat(filterClause.logicalExpression().logicalExpressions.get(0).comparisonExpressions) + .hasSize(2); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(0) + .getFilterOperations()) + .isEqualTo(expectedResult1.getFilterOperations()); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .comparisonExpressions + .get(1) + .getFilterOperations()) + .isEqualTo(expectedResult2.getFilterOperations()); + + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .logicalExpressions + .get(0) + .getLogicalRelation()) + .isEqualTo(LogicalExpression.LogicalOperator.AND); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .logicalExpressions + .get(0) + .comparisonExpressions) + .hasSize(2); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .logicalExpressions + .get(0) + .comparisonExpressions + .get(0) + .getFilterOperations()) + .isEqualTo(expectedResult3.getFilterOperations()); + assertThat( + filterClause + .logicalExpression() + .logicalExpressions + .get(0) + .logicalExpressions + .get(0) + .comparisonExpressions + .get(1) + .getFilterOperations()) + .isEqualTo(expectedResult4.getFilterOperations()); + } + @Test public void mustHandleInArrayNonEmpty() throws Exception { String json = """ From f49acaf7d41ebbf2f2371fdcb60d0778ab46e007 Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Thu, 21 Dec 2023 14:18:15 -0500 Subject: [PATCH 05/19] Added a hack to demo --- .../FilterClauseDeserializer.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java index cd1eb31e9e..1c44faa414 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java @@ -47,9 +47,32 @@ public FilterClause deserialize( validate(implicitAnd); // push down not operator implicitAnd.dfs(null); + + implicitAnd = topDown(implicitAnd); + return new FilterClause(implicitAnd); } + private LogicalExpression topDown(LogicalExpression logicalExpression) { + if (logicalExpression == null) { + return null; + } + LogicalExpression newLogicalExpression; + if (logicalExpression.getLogicalRelation() == LogicalExpression.LogicalOperator.AND) { + newLogicalExpression = LogicalExpression.and(); + } else { + newLogicalExpression = LogicalExpression.or(); + } + for (ComparisonExpression comparisonExpression : logicalExpression.comparisonExpressions) { + newLogicalExpression.addComparisonExpression(comparisonExpression); + } + for (LogicalExpression innerLogicalExpression : logicalExpression.logicalExpressions) { + LogicalExpression newInnerLogicalExpression = topDown(innerLogicalExpression); + newLogicalExpression.addLogicalExpression(newInnerLogicalExpression); + } + return newLogicalExpression; + } + private void populateExpression(LogicalExpression logicalExpression, JsonNode node) { if (logicalExpression == null) { return; From 3d525d0be3e9801e27e2c78c13fd23ee48459131 Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Thu, 21 Dec 2023 14:36:05 -0500 Subject: [PATCH 06/19] Added a hack to demo --- .../api/model/command/clause/filter/LogicalExpression.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java index b7068b06d2..70e1548a96 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java @@ -31,10 +31,10 @@ public String getOperator() { private int totalIdComparisonExpressionCount; /** This method will flip the operators and operand if logical operator is not */ - public void dfs(LogicalExpression parent) { + public void traverseForNot(LogicalExpression parent) { List tempLogicalExpressions = new ArrayList<>(logicalExpressions); for (LogicalExpression logicalExpression : tempLogicalExpressions) { - logicalExpression.dfs(this); + logicalExpression.traverseForNot(this); } Iterator iterator = logicalExpressions.iterator(); From dc5d55cc0f73ac684db07df0e28dd8ec444b8510 Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Thu, 21 Dec 2023 14:38:58 -0500 Subject: [PATCH 07/19] Added a hack to demo --- .../model/command/deserializers/FilterClauseDeserializer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java index 1c44faa414..00d7bd87c3 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java @@ -46,7 +46,7 @@ public FilterClause deserialize( populateExpression(implicitAnd, filterNode); validate(implicitAnd); // push down not operator - implicitAnd.dfs(null); + implicitAnd.traverseForNot(null); implicitAnd = topDown(implicitAnd); From 768ab39731381095e3b754c703ad57693e137f03 Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Fri, 22 Dec 2023 13:58:36 -0500 Subject: [PATCH 08/19] Changed the range operator for not --- .../command/clause/filter/ValueComparisonOperator.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ValueComparisonOperator.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ValueComparisonOperator.java index 636804e1bd..1f563d59c3 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ValueComparisonOperator.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ValueComparisonOperator.java @@ -37,13 +37,13 @@ public FilterOperator flip() { case NIN: return IN; case GT: - return LT; - case GTE: return LTE; + case GTE: + return LT; case LT: - return GT; - case LTE: return GTE; + case LTE: + return GT; } return this; } From 27761d05281be1f55f0d0ce8507c0d6f3ea040d5 Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Wed, 3 Jan 2024 09:52:12 -0500 Subject: [PATCH 09/19] $not support for $all operator --- .../operation/model/impl/DBFilterBase.java | 30 +++++-------------- .../model/impl/ExpressionBuilder.java | 5 +++- .../impl/matcher/FilterableResolver.java | 10 ++----- .../model/impl/FindOperationTest.java | 8 ++--- 4 files changed, 19 insertions(+), 34 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java index cbc6a141e1..a11176dc88 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java @@ -595,10 +595,16 @@ boolean canAddField() { /** Filter for document where all values exists for an array */ public static class AllFilter extends DBFilterBase { private final List arrayValue; + private final boolean negation; - public AllFilter(String path, List arrayValue) { + public AllFilter(String path, List arrayValue, boolean negation) { super(path); this.arrayValue = arrayValue; + this.negation = negation; + } + + public boolean isNegation() { + return negation; } @Override @@ -617,7 +623,7 @@ public List getAll() { result.add( BuiltCondition.of( DATA_CONTAINS, - Predicate.CONTAINS, + negation ? Predicate.NOT_CONTAINS : Predicate.CONTAINS, new JsonTerm(getHashValue(new DocValueHasher(), getPath(), value)))); } return result; @@ -629,26 +635,6 @@ public BuiltCondition get() { } } - /** Filter for document where all values exists for an array */ - public static class NotAnyFilter extends SetFilterBase { - private final Object arrayValue; - - public NotAnyFilter(DocValueHasher hasher, String path, Object arrayValue) { - super("array_contains", path, getHashValue(hasher, path, arrayValue), Operator.NOT_CONTAINS); - this.arrayValue = arrayValue; - } - - @Override - JsonNode asJson(JsonNodeFactory nodeFactory) { - return DBFilterBase.getJsonNode(nodeFactory, arrayValue); - } - - @Override - boolean canAddField() { - return false; - } - } - /** Filter for document where array has specified number of elements */ public static class SizeFilter extends MapFilterBase { public SizeFilter(String path, Operator operator, Integer size) { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ExpressionBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ExpressionBuilder.java index 6bb8050ab4..525784d9f9 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ExpressionBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ExpressionBuilder.java @@ -112,7 +112,10 @@ private static Expression buildExpressionRecursive( List allFilterConditions = allFilter.getAll(); List> allFilterVariables = allFilterConditions.stream().map(Variable::of).toList(); - conditionExpressions.add(ExpressionUtils.andOf(allFilterVariables)); + conditionExpressions.add( + allFilter.isNegation() + ? ExpressionUtils.orOf(allFilterVariables) + : ExpressionUtils.andOf(allFilterVariables)); } else if (dbFilter instanceof DBFilterBase.InFilter inFilter) { if (inFilter.operator.equals(DBFilterBase.InFilter.Operator.IN)) { hasInFilterThisLevel = true; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterableResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterableResolver.java index 3485ba7da8..909a3943a0 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterableResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterableResolver.java @@ -295,16 +295,12 @@ public static List findDynamic(CaptureExpression captureExpression if (captureExpression.marker() == ALL_GROUP) { List arrayValue = (List) filterOperation.operand().value(); - filters.add(new DBFilterBase.AllFilter(captureExpression.path(), arrayValue)); + filters.add(new DBFilterBase.AllFilter(captureExpression.path(), arrayValue, false)); } if (captureExpression.marker() == NOT_ANY_GROUP) { - final DocValueHasher docValueHasher = new DocValueHasher(); - List objects = (List) filterOperation.operand().value(); - for (Object arrayValue : objects) { - filters.add( - new DBFilterBase.NotAnyFilter(docValueHasher, captureExpression.path(), arrayValue)); - } + List arrayValue = (List) filterOperation.operand().value(); + filters.add(new DBFilterBase.AllFilter(captureExpression.path(), arrayValue, true)); } if (captureExpression.marker() == SIZE_GROUP) { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java index f8573cf879..06300d2a7e 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java @@ -1091,7 +1091,7 @@ public void findWithAllFilter() throws Exception { LogicalExpression implicitAnd = LogicalExpression.and(); implicitAnd.comparisonExpressions.add(new ComparisonExpression(null, null, null)); List filters1 = - List.of(new DBFilterBase.AllFilter("tags", List.of("tag1", "tag2"))); + List.of(new DBFilterBase.AllFilter("tags", List.of("tag1", "tag2"), false)); implicitAnd.comparisonExpressions.get(0).setDBFilters(filters1); FindOperation operation = FindOperation.unsortedSingle( @@ -1161,7 +1161,7 @@ public void findOrWithAllFilter() throws Exception { "username", DBFilterBase.MapFilterBase.Operator.EQ, "user1")); explicitOr.comparisonExpressions.get(0).setDBFilters(filter1); List filters2 = - List.of(new DBFilterBase.AllFilter("tags", List.of("tag1", "tag2"))); + List.of(new DBFilterBase.AllFilter("tags", List.of("tag1", "tag2"), false)); explicitOr.comparisonExpressions.get(1).setDBFilters(filters2); FindOperation operation = @@ -2465,7 +2465,7 @@ public void expressionSort() { LogicalExpression implicitAnd1 = LogicalExpression.and(); implicitAnd1.comparisonExpressions.add(new ComparisonExpression(null, null, null)); List filters1 = - List.of(new DBFilterBase.AllFilter("tags", List.of("tag1", "tag2"))); + List.of(new DBFilterBase.AllFilter("tags", List.of("tag1", "tag2"), false)); implicitAnd1.comparisonExpressions.get(0).setDBFilters(filters1); FindOperation operation1 = FindOperation.unsortedSingle( @@ -2481,7 +2481,7 @@ public void expressionSort() { LogicalExpression implicitAnd2 = LogicalExpression.and(); implicitAnd2.comparisonExpressions.add(new ComparisonExpression(null, null, null)); List filters2 = - List.of(new DBFilterBase.AllFilter("tags", List.of("tag1", "tag2"))); + List.of(new DBFilterBase.AllFilter("tags", List.of("tag1", "tag2"), false)); implicitAnd2.comparisonExpressions.get(0).setDBFilters(filters2); FindOperation operation2 = From 60292fc03ff882b2824cd892edab0fd6301365ea Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Wed, 3 Jan 2024 09:59:42 -0500 Subject: [PATCH 10/19] Fixed range not test cases --- .../deserializers/FilterClauseDeserializerTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java index df77bc34cf..564cbed0fa 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java @@ -874,7 +874,7 @@ ValueComparisonOperator.EQ, new JsonLiteral("testName", JsonType.STRING))), "f5", List.of( new ValueComparisonOperation( - ValueComparisonOperator.GT, + ValueComparisonOperator.GTE, new JsonLiteral(new BigDecimal(5), JsonType.NUMBER))), null); @@ -883,7 +883,7 @@ ValueComparisonOperator.EQ, new JsonLiteral("testName", JsonType.STRING))), "f6", List.of( new ValueComparisonOperation( - ValueComparisonOperator.GTE, + ValueComparisonOperator.GT, new JsonLiteral(new BigDecimal(5), JsonType.NUMBER))), null); @@ -892,7 +892,7 @@ ValueComparisonOperator.EQ, new JsonLiteral("testName", JsonType.STRING))), "f7", List.of( new ValueComparisonOperation( - ValueComparisonOperator.LT, + ValueComparisonOperator.LTE, new JsonLiteral(new BigDecimal(5), JsonType.NUMBER))), null); final ComparisonExpression gte = @@ -900,7 +900,7 @@ ValueComparisonOperator.EQ, new JsonLiteral("testName", JsonType.STRING))), "f8", List.of( new ValueComparisonOperation( - ValueComparisonOperator.LTE, + ValueComparisonOperator.LT, new JsonLiteral(new BigDecimal(5), JsonType.NUMBER))), null); From 452853c9be265d575b4385090cd5df5c16b0a421 Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Wed, 3 Jan 2024 10:54:33 -0500 Subject: [PATCH 11/19] Fixed the count roll up --- .../clause/filter/LogicalExpression.java | 8 ++++++- .../FilterClauseDeserializer.java | 22 ------------------- 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java index 70e1548a96..c73e716a3c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/LogicalExpression.java @@ -42,6 +42,11 @@ public void traverseForNot(LogicalExpression parent) { LogicalExpression logicalExpression = iterator.next(); if (logicalExpression.logicalRelation == LogicalOperator.NOT) { iterator.remove(); + this.totalComparisonExpressionCount = + this.totalComparisonExpressionCount - logicalExpression.totalComparisonExpressionCount; + this.totalIdComparisonExpressionCount = + this.totalIdComparisonExpressionCount + - logicalExpression.totalIdComparisonExpressionCount; } } @@ -56,7 +61,8 @@ private void addToParent(LogicalExpression parent) { if (comparisonExpressions.size() > 1) { // Multiple conditions in not, after push down will become or final LogicalExpression orLogic = LogicalExpression.or(); - orLogic.comparisonExpressions.addAll(comparisonExpressions); + comparisonExpressions.stream() + .forEach(comparisonExpression -> orLogic.addComparisonExpression(comparisonExpression)); parent.addLogicalExpression(orLogic); } else { if (comparisonExpressions.size() == 1) { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java index 00d7bd87c3..69f4c39fc0 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java @@ -48,31 +48,9 @@ public FilterClause deserialize( // push down not operator implicitAnd.traverseForNot(null); - implicitAnd = topDown(implicitAnd); - return new FilterClause(implicitAnd); } - private LogicalExpression topDown(LogicalExpression logicalExpression) { - if (logicalExpression == null) { - return null; - } - LogicalExpression newLogicalExpression; - if (logicalExpression.getLogicalRelation() == LogicalExpression.LogicalOperator.AND) { - newLogicalExpression = LogicalExpression.and(); - } else { - newLogicalExpression = LogicalExpression.or(); - } - for (ComparisonExpression comparisonExpression : logicalExpression.comparisonExpressions) { - newLogicalExpression.addComparisonExpression(comparisonExpression); - } - for (LogicalExpression innerLogicalExpression : logicalExpression.logicalExpressions) { - LogicalExpression newInnerLogicalExpression = topDown(innerLogicalExpression); - newLogicalExpression.addLogicalExpression(newInnerLogicalExpression); - } - return newLogicalExpression; - } - private void populateExpression(LogicalExpression logicalExpression, JsonNode node) { if (logicalExpression == null) { return; From f3aa914ef6233071d59fe87b86d51459c782d849 Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Wed, 3 Jan 2024 11:04:18 -0500 Subject: [PATCH 12/19] Added unit test for new negation logic in all filter --- .../model/impl/FindOperationTest.java | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java index 06300d2a7e..4e627ee419 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperationTest.java @@ -1192,6 +1192,77 @@ public void findOrWithAllFilter() throws Exception { assertThat(result.errors()).isNullOrEmpty(); } + @Test + public void findOrWithAllFilterWithNegation() throws Exception { + String collectionReadCql = + "SELECT key, tx_id, doc_json FROM \"%s\".\"%s\" WHERE (array_contains CONTAINS ? OR (array_contains NOT CONTAINS ? OR array_contains NOT CONTAINS ?)) LIMIT 1" + .formatted(KEYSPACE_NAME, COLLECTION_NAME); + + String doc1 = + """ + { + "_id": "doc1", + "username": "user1", + "registration_active" : true, + "tags": ["tag1", "tag2"] + } + """; + + SimpleStatement stmt = + SimpleStatement.newInstance( + collectionReadCql, "username Suser1", "tags Stag1", "tags Stag2"); + List rows = Arrays.asList(resultRow(0, "doc1", UUID.randomUUID(), doc1)); + AsyncResultSet results = new MockAsyncResultSet(KEY_TXID_JSON_COLUMNS, rows, null); + final AtomicInteger callCount = new AtomicInteger(); + QueryExecutor queryExecutor = mock(QueryExecutor.class); + when(queryExecutor.executeRead(eq(stmt), any(), anyInt())) + .then( + invocation -> { + callCount.incrementAndGet(); + return Uni.createFrom().item(results); + }); + + LogicalExpression explicitOr = LogicalExpression.or(); + explicitOr.comparisonExpressions.add(new ComparisonExpression(null, null, null)); + explicitOr.comparisonExpressions.add(new ComparisonExpression(null, null, null)); + + List filter1 = + List.of( + new DBFilterBase.TextFilter( + "username", DBFilterBase.MapFilterBase.Operator.EQ, "user1")); + explicitOr.comparisonExpressions.get(0).setDBFilters(filter1); + List filters2 = + List.of(new DBFilterBase.AllFilter("tags", List.of("tag1", "tag2"), true)); + explicitOr.comparisonExpressions.get(1).setDBFilters(filters2); + + FindOperation operation = + FindOperation.unsortedSingle( + COMMAND_CONTEXT, + explicitOr, + DocumentProjector.identityProjector(), + ReadType.DOCUMENT, + objectMapper); + + Supplier execute = + operation + .execute(queryExecutor) + .subscribe() + .withSubscriber(UniAssertSubscriber.create()) + .awaitItem() + .getItem(); + + // assert query execution + assertThat(callCount.get()).isEqualTo(1); + + // then result + CommandResult result = execute.get(); + assertThat(result.data().getResponseDocuments()) + .hasSize(1) + .containsOnly(objectMapper.readTree(doc1)); + assertThat(result.status()).isNullOrEmpty(); + assertThat(result.errors()).isNullOrEmpty(); + } + @Test public void findWithSizeFilter() throws Exception { String collectionReadCql = From 11d6f69468383b4d5132d8d380952b730d650ac9 Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Wed, 3 Jan 2024 12:10:02 -0500 Subject: [PATCH 13/19] Added IT cases --- .../jsonapi/api/v1/FindIntegrationTest.java | 162 ++++++++++++++++++ 1 file changed, 162 insertions(+) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java index aee534ef33..bdf7eab4b9 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java @@ -1727,6 +1727,168 @@ public void NeSubdoc() { jsonEquals(expected5), jsonEquals(expected6))); } + + @Test + public void simpleNot() { + String json = + """ + { + "find": { + "filter" : { + "$not": [ + {"username": "user1"} + ] + } + } + } + """; + String expected2 = + """ + {"_id":"doc2", "username":"user2", "subdoc":{"id":"abc"},"array":["value1"]} + """; + String expected3 = + """ + {"_id": "doc3","username": "user3","tags" : ["tag1", "tag2", "tag1234567890123456789012345", null, 1, true], "nestedArray" : [["tag1", "tag2"], ["tag1234567890123456789012345", null]]} + """; + + String expected4 = + """ + { + "_id": "doc4", + "username":"user4", + "indexedObject" : { "0": "value_0", "1": "value_1" } + } + """; + String expected5 = + """ + { + "_id": "doc5", + "username": "user5", + "sub_doc" : { "a": 5, "b": { "c": "v1", "d": false } } + } + """; + String expected6 = + """ + { + "_id": {"$date": 6}, + "user-name": "user6" + } + """; + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body(json) + .when() + .post(CollectionResource.BASE_PATH, namespaceName, collectionName) + .then() + .statusCode(200) + .body("status", is(nullValue())) + .body("errors", is(nullValue())) + .body("data.documents", hasSize(5)) + .body( + "data.documents", + containsInAnyOrder( + jsonEquals(expected2), + jsonEquals(expected3), + jsonEquals(expected4), + jsonEquals(expected5), + jsonEquals(expected6))); + } + + @Test + public void nestedNotPushDown() { + String json = + """ + { + "find": { + "filter": { + "$not": [ + { + "$and": [ + { + "$or": [ + { + "username": "user3" + }, + { + "subdoc.id": { + "$eq": "abc" + } + } + ] + }, + { + "$or": [ + { + "username": "user2" + }, + { + "subdoc.id": { + "$eq": "xyz" + } + } + ] + } + ] + } + ] + } + } + } + """; + + String expected = + """ + {"_id":"doc2", "username":"user2", "subdoc":{"id":"abc"},"array":["value1"]} + """; + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body(json) + .when() + .post(CollectionResource.BASE_PATH, namespaceName, collectionName) + .then() + .statusCode(200) + .body("status", is(nullValue())) + .body("errors", is(nullValue())) + .body("data.documents", not(containsInAnyOrder(expected))) + .body("data.documents", hasSize(5)); + } + + @Test + public void multipleNotCancelling() { + String json = + """ + { + "find": { + "filter" : { + "$not":[ + { "$not" : [ + {"username" : "user1"} + ]} + ] + } + } + } + """; + + String expected = + """ + {"_id":"doc1", "username":"user1", "active_user":true, "date" : {"$date": 1672531200000}, "age" : 20, "null_column": null} + """; + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body(json) + .when() + .post(CollectionResource.BASE_PATH, namespaceName, collectionName) + .then() + .statusCode(200) + .body("status", is(nullValue())) + .body("errors", is(nullValue())) + .body("data.documents", hasSize(1)) + .body("data.documents", containsInAnyOrder(jsonEquals(expected))); + } } @Nested From 4dac951e22a269d547b8326660331cb41148d2a3 Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Wed, 3 Jan 2024 12:44:17 -0500 Subject: [PATCH 14/19] Added IT cases with range query --- .../jsonapi/api/v1/FindIntegrationTest.java | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java index bdf7eab4b9..413ac75e37 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java @@ -1889,6 +1889,67 @@ public void multipleNotCancelling() { .body("data.documents", hasSize(1)) .body("data.documents", containsInAnyOrder(jsonEquals(expected))); } + + @Test + public void withNotAllOperator() { + String json = + """ + { + "find": { + "filter" : {"$not" : [{"tags" : {"$all" : ["tag1", "tag2"]}}] } + } + } + """; + + String expected = + """ + {"_id": "doc3","username": "user3","tags" : ["tag1", "tag2", "tag1234567890123456789012345", null, 1, true], "nestedArray" : [["tag1", "tag2"], ["tag1234567890123456789012345", null]]} + """; + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body(json) + .when() + .post(CollectionResource.BASE_PATH, namespaceName, collectionName) + .then() + .statusCode(200) + .body("status", is(nullValue())) + .body("errors", is(nullValue())) + .body("data.documents", not(containsInAnyOrder(expected))) + .body("data.documents", hasSize(5)); + } + + @Test + public void rangeWithNot() { + String json = + """ + { + "find": { + "filter" : { + "$not": [ + {"age": {"$gte" : "21"}} + ] + } + } + } + """; + String expected = + """ + {"_id":"doc1", "username":"user1", "active_user":true, "date" : {"$date": 1672531200000}, "age" : 20, "null_column": null} + """; + given() + .header(HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME, getAuthToken()) + .contentType(ContentType.JSON) + .body(json) + .when() + .post(CollectionResource.BASE_PATH, namespaceName, collectionName) + .then() + .statusCode(200) + .body("status", is(nullValue())) + .body("errors", is(nullValue())) + .body("data.documents", hasSize(5)) + .body("data.documents", containsInAnyOrder(jsonEquals(expected))); + } } @Nested From b25aa3ca1c0236d7a8dcb8e423908c955b9c639d Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Wed, 3 Jan 2024 13:05:08 -0500 Subject: [PATCH 15/19] Added IT cases with range query fix --- .../io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java index 413ac75e37..5fb79014a3 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java @@ -1927,7 +1927,7 @@ public void rangeWithNot() { "find": { "filter" : { "$not": [ - {"age": {"$gte" : "21"}} + {"age": {"$gte" : 21}} ] } } From e4383e78bb9900f8880794a33fd195f943e4b286 Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Wed, 3 Jan 2024 13:53:08 -0500 Subject: [PATCH 16/19] Added IT cases with range query fix --- .../io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java index 5fb79014a3..aa3aaa3f1f 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java @@ -1947,7 +1947,7 @@ public void rangeWithNot() { .statusCode(200) .body("status", is(nullValue())) .body("errors", is(nullValue())) - .body("data.documents", hasSize(5)) + .body("data.documents", hasSize(1)) .body("data.documents", containsInAnyOrder(jsonEquals(expected))); } } From c2aca98293d3f8b6e7bbaeca73abff73891baae0 Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Wed, 3 Jan 2024 16:15:57 -0500 Subject: [PATCH 17/19] Use BigDecimal.negate() instead of multiplying by -1. --- .../api/model/command/clause/filter/ComparisonExpression.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ComparisonExpression.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ComparisonExpression.java index 566cf2db5b..55ae16b21f 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ComparisonExpression.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ComparisonExpression.java @@ -52,8 +52,7 @@ private JsonLiteral getFlippedOperandValue(FilterOperator operator, JsonLiter if (operator == ElementComparisonOperator.EXISTS) { return new JsonLiteral(!((Boolean) operand.value()), operand.type()); } else if (operator == ArrayComparisonOperator.SIZE) { - return new JsonLiteral( - ((BigDecimal) operand.value()).multiply(new BigDecimal(-1)), operand.type()); + return new JsonLiteral(((BigDecimal) operand.value()).negate(), operand.type()); } else { return operand; } From bde16daf7cb45a49cffdd8b617576ca78d6deced Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Wed, 3 Jan 2024 16:19:33 -0500 Subject: [PATCH 18/19] Moved the comment to java doc --- .../command/clause/filter/ArrayComparisonOperator.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ArrayComparisonOperator.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ArrayComparisonOperator.java index 9046dfb461..1b67bf6a84 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ArrayComparisonOperator.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ArrayComparisonOperator.java @@ -1,10 +1,13 @@ package io.stargate.sgv2.jsonapi.api.model.command.clause.filter; -/** List of element level operator that can be used in Filter clause */ +/** + * List of element level operator that can be used in Filter clause NOTANY operator is used + * internally to support $not operation with $all + */ public enum ArrayComparisonOperator implements FilterOperator { ALL("$all"), SIZE("$size"), - // Will not be supported from outside + /** Can not be used in filter clause operator in user api */ NOTANY("$notany"); private String operator; From 73320871275a2e4c36389d53e421507ebab124a0 Mon Sep 17 00:00:00 2001 From: maheshrajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Fri, 5 Jan 2024 09:05:40 -0500 Subject: [PATCH 19/19] Changed the not structure value type from array to object --- .../FilterClauseDeserializer.java | 11 +- .../FilterClauseDeserializerTest.java | 141 ++++-------------- .../jsonapi/api/v1/FindIntegrationTest.java | 75 ++++------ 3 files changed, 66 insertions(+), 161 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java index 69f4c39fc0..9eede13d55 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializer.java @@ -86,8 +86,14 @@ private void populateExpression(LogicalExpression logicalExpression, JsonNode no private void populateExpression( LogicalExpression logicalExpression, Map.Entry entry) { if (entry.getValue().isObject()) { + if (entry.getKey().equals("$not")) { + LogicalExpression innerLogicalExpression = LogicalExpression.not(); + populateExpression(innerLogicalExpression, entry.getValue()); + logicalExpression.addLogicalExpression(innerLogicalExpression); + } else { + logicalExpression.addComparisonExpression(createComparisonExpression(entry)); + } // inside of this entry, only implicit and, no explicit $and/$or - logicalExpression.addComparisonExpression(createComparisonExpression(entry)); } else if (entry.getValue().isArray()) { LogicalExpression innerLogicalExpression = null; switch (entry.getKey()) { @@ -97,9 +103,6 @@ private void populateExpression( case "$or": innerLogicalExpression = LogicalExpression.or(); break; - case "$not": - innerLogicalExpression = LogicalExpression.not(); - break; case DocumentConstants.Fields.VECTOR_EMBEDDING_FIELD: throw new JsonApiException( ErrorCode.INVALID_FILTER_EXPRESSION, diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java index 564cbed0fa..1ac32e7d3b 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/FilterClauseDeserializerTest.java @@ -772,10 +772,7 @@ public void simpleNot() throws Exception { String json = """ { - "$not":[ - {"name" : "testName"}, - {"age" : "testAge"} - ] + "$not": {"name" : "testName"} } """; final ComparisonExpression expectedResult1 = @@ -785,37 +782,12 @@ public void simpleNot() throws Exception { new ValueComparisonOperation( ValueComparisonOperator.NE, new JsonLiteral("testName", JsonType.STRING))), null); - final ComparisonExpression expectedResult2 = - new ComparisonExpression( - "age", - List.of( - new ValueComparisonOperation( - ValueComparisonOperator.NE, new JsonLiteral("testAge", JsonType.STRING))), - null); FilterClause filterClause = objectMapper.readValue(json, FilterClause.class); - assertThat(filterClause.logicalExpression().logicalExpressions).hasSize(1); - assertThat(filterClause.logicalExpression().logicalExpressions.get(0).getLogicalRelation()) - .isEqualTo(LogicalExpression.LogicalOperator.OR); - assertThat(filterClause.logicalExpression().logicalExpressions.get(0).comparisonExpressions) - .hasSize(2); + assertThat(filterClause.logicalExpression().logicalExpressions).hasSize(0); + assertThat(filterClause.logicalExpression().comparisonExpressions.size()).isEqualTo(1); assertThat( - filterClause - .logicalExpression() - .logicalExpressions - .get(0) - .comparisonExpressions - .get(0) - .getFilterOperations()) + filterClause.logicalExpression().comparisonExpressions.get(0).getFilterOperations()) .isEqualTo(expectedResult1.getFilterOperations()); - assertThat( - filterClause - .logicalExpression() - .logicalExpressions - .get(0) - .comparisonExpressions - .get(1) - .getFilterOperations()) - .isEqualTo(expectedResult2.getFilterOperations()); } @Test @@ -823,7 +795,8 @@ public void comparisonOperatorsWithNot() throws Exception { String json = """ { - "$not":[ + "$not": { + "$and" : [ {"f1" : {"$eq" : "testName"}}, {"f2" : {"$ne" : "testName"}}, {"f3" : {"$in" : ["testName1","testName2"]}}, @@ -836,6 +809,7 @@ public void comparisonOperatorsWithNot() throws Exception { {"f10" : {"$exists" : false}}, {"f11" : {"$size" : 1}} ] + } } """; final ComparisonExpression eq = @@ -1038,12 +1012,7 @@ public void twoLevelNot() throws Exception { String json = """ { - "$not":[ - { "$not" : [ - {"name" : "testName"}, - {"age" : "testAge"} - ]} - ] + "$not":{ "$not" : {"name" : "testName"} } } """; final ComparisonExpression expectedResult1 = @@ -1053,37 +1022,12 @@ public void twoLevelNot() throws Exception { new ValueComparisonOperation( ValueComparisonOperator.EQ, new JsonLiteral("testName", JsonType.STRING))), null); - final ComparisonExpression expectedResult2 = - new ComparisonExpression( - "age", - List.of( - new ValueComparisonOperation( - ValueComparisonOperator.EQ, new JsonLiteral("testAge", JsonType.STRING))), - null); FilterClause filterClause = objectMapper.readValue(json, FilterClause.class); - assertThat(filterClause.logicalExpression().logicalExpressions).hasSize(1); - assertThat(filterClause.logicalExpression().logicalExpressions.get(0).getLogicalRelation()) - .isEqualTo(LogicalExpression.LogicalOperator.AND); - assertThat(filterClause.logicalExpression().logicalExpressions.get(0).comparisonExpressions) - .hasSize(2); + assertThat(filterClause.logicalExpression().logicalExpressions).hasSize(0); + assertThat(filterClause.logicalExpression().comparisonExpressions.size()).isEqualTo(1); assertThat( - filterClause - .logicalExpression() - .logicalExpressions - .get(0) - .comparisonExpressions - .get(0) - .getFilterOperations()) + filterClause.logicalExpression().comparisonExpressions.get(0).getFilterOperations()) .isEqualTo(expectedResult1.getFilterOperations()); - assertThat( - filterClause - .logicalExpression() - .logicalExpressions - .get(0) - .comparisonExpressions - .get(1) - .getFilterOperations()) - .isEqualTo(expectedResult2.getFilterOperations()); } @Test @@ -1091,14 +1035,7 @@ public void multipleNot() throws Exception { String json = """ { - "$not":[ - { "$not" : [ - { "$not" : [ - {"name" : "testName"}, - {"age" : "testAge"} - ]} - ]} - ] + "$not": { "$not" : { "$not" : {"name" : "testName"} } } } """; final ComparisonExpression expectedResult1 = @@ -1108,37 +1045,12 @@ public void multipleNot() throws Exception { new ValueComparisonOperation( ValueComparisonOperator.NE, new JsonLiteral("testName", JsonType.STRING))), null); - final ComparisonExpression expectedResult2 = - new ComparisonExpression( - "age", - List.of( - new ValueComparisonOperation( - ValueComparisonOperator.NE, new JsonLiteral("testAge", JsonType.STRING))), - null); FilterClause filterClause = objectMapper.readValue(json, FilterClause.class); - assertThat(filterClause.logicalExpression().logicalExpressions).hasSize(1); - assertThat(filterClause.logicalExpression().logicalExpressions.get(0).getLogicalRelation()) - .isEqualTo(LogicalExpression.LogicalOperator.OR); - assertThat(filterClause.logicalExpression().logicalExpressions.get(0).comparisonExpressions) - .hasSize(2); + assertThat(filterClause.logicalExpression().logicalExpressions).hasSize(0); + assertThat(filterClause.logicalExpression().comparisonExpressions.size()).isEqualTo(1); assertThat( - filterClause - .logicalExpression() - .logicalExpressions - .get(0) - .comparisonExpressions - .get(0) - .getFilterOperations()) + filterClause.logicalExpression().comparisonExpressions.get(0).getFilterOperations()) .isEqualTo(expectedResult1.getFilterOperations()); - assertThat( - filterClause - .logicalExpression() - .logicalExpressions - .get(0) - .comparisonExpressions - .get(1) - .getFilterOperations()) - .isEqualTo(expectedResult2.getFilterOperations()); } @Test @@ -1265,18 +1177,17 @@ public void multipleLevel() throws Exception { "age": "testAge" }, { - "$not": [ - { - "$or": [ - { - "address": "testAddress" - }, - { - "tags": { "$size" : 1} - } - ] - } - ] + "$not": + { + "$or": [ + { + "address": "testAddress" + }, + { + "tags": { "$size" : 1} + } + ] + } } ] } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java index aa3aaa3f1f..ed60666397 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/FindIntegrationTest.java @@ -1735,9 +1735,7 @@ public void simpleNot() { { "find": { "filter" : { - "$not": [ - {"username": "user1"} - ] + "$not": {"username": "user1"} } } } @@ -1802,36 +1800,35 @@ public void nestedNotPushDown() { { "find": { "filter": { - "$not": [ - { - "$and": [ - { - "$or": [ - { - "username": "user3" - }, - { - "subdoc.id": { - "$eq": "abc" - } - } - ] - }, - { - "$or": [ - { - "username": "user2" - }, - { - "subdoc.id": { - "$eq": "xyz" - } - } - ] - } - ] - } - ] + "$not": + { + "$and": [ + { + "$or": [ + { + "username": "user3" + }, + { + "subdoc.id": { + "$eq": "abc" + } + } + ] + }, + { + "$or": [ + { + "username": "user2" + }, + { + "subdoc.id": { + "$eq": "xyz" + } + } + ] + } + ] + } } } } @@ -1862,11 +1859,7 @@ public void multipleNotCancelling() { { "find": { "filter" : { - "$not":[ - { "$not" : [ - {"username" : "user1"} - ]} - ] + "$not":{ "$not" : {"username" : "user1"} } } } } @@ -1896,7 +1889,7 @@ public void withNotAllOperator() { """ { "find": { - "filter" : {"$not" : [{"tags" : {"$all" : ["tag1", "tag2"]}}] } + "filter" : {"$not" : {"tags" : {"$all" : ["tag1", "tag2"]}} } } } """; @@ -1926,9 +1919,7 @@ public void rangeWithNot() { { "find": { "filter" : { - "$not": [ - {"age": {"$gte" : 21}} - ] + "$not": {"age": {"$gte" : 21}} } } }