Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

$not operator #760

Merged
merged 25 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
757b4c9
First version of `$not` operator
maheshrajamani Dec 21, 2023
5620776
Checking in with bug of not handling OR logic for All and not(ALL)
maheshrajamani Dec 21, 2023
f416392
Added test case
maheshrajamani Dec 21, 2023
269537e
Added test cases
maheshrajamani Dec 21, 2023
f49acaf
Added a hack to demo
maheshrajamani Dec 21, 2023
3d525d0
Added a hack to demo
maheshrajamani Dec 21, 2023
dc5d55c
Added a hack to demo
maheshrajamani Dec 21, 2023
768ab39
Changed the range operator for not
maheshrajamani Dec 22, 2023
7fbb6c9
Merge branch 'main' of https://github.com/stargate/jsonapi
maheshrajamani Jan 2, 2024
d0fc305
Merge branch 'main' into not-operator
maheshrajamani Jan 3, 2024
27761d0
$not support for $all operator
maheshrajamani Jan 3, 2024
60292fc
Fixed range not test cases
maheshrajamani Jan 3, 2024
452853c
Fixed the count roll up
maheshrajamani Jan 3, 2024
f3aa914
Added unit test for new negation logic in all filter
maheshrajamani Jan 3, 2024
11d6f69
Added IT cases
maheshrajamani Jan 3, 2024
4dac951
Added IT cases with range query
maheshrajamani Jan 3, 2024
b25aa3c
Added IT cases with range query fix
maheshrajamani Jan 3, 2024
e4383e7
Added IT cases with range query fix
maheshrajamani Jan 3, 2024
c2aca98
Use BigDecimal.negate() instead of multiplying by -1.
maheshrajamani Jan 3, 2024
bde16da
Moved the comment to java doc
maheshrajamani Jan 3, 2024
7332087
Changed the not structure value type from array to object
maheshrajamani Jan 5, 2024
2c3e07b
Merge branch 'main' into not-operator
maheshrajamani Jan 8, 2024
d4e9d9f
Resolved merge conflict
maheshrajamani Jan 11, 2024
232df5d
Merge branch 'not-operator' of https://github.com/stargate/jsonapi in…
maheshrajamani Jan 11, 2024
730c3b3
Merge branch 'main' into not-operator
maheshrajamani Jan 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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");
tatu-at-datastax marked this conversation as resolved.
Show resolved Hide resolved

private String operator;

Expand All @@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public class ComparisonExpression {

private final String path;

@Valid @NotEmpty private final List<FilterOperation<?>> filterOperations;
@Valid @NotEmpty private List<FilterOperation<?>> filterOperations;

private List<DBFilterBase> dbFilters;

Expand All @@ -33,6 +33,32 @@ public List<FilterOperation<?>> getFilterOperations() {
return filterOperations;
}

/** This method will be called when not operation is pushed down */
public void flip() {
List<FilterOperation<?>> 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>(!((Boolean) operand.value()), operand.type());
} else if (operator == ArrayComparisonOperator.SIZE) {
return new JsonLiteral<BigDecimal>(
((BigDecimal) operand.value()).multiply(new BigDecimal(-1)), operand.type());
Copy link
Contributor

@tatu-at-datastax tatu-at-datastax Jan 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is done to flip between 1 and -1, more efficient way would be to use BigDecimal.negate() (https://docs.oracle.com/javase/8/docs/api/java/math/BigDecimal.html#negate-- )

But I guess I am not sure what the logic here really is? Negative values can't match anything, I guess that is the idea?

EDIT: this ^^^ makes sense when at later point negative value is converted to "$size NOT equals X"

} else {
return operand;
}
}

public ComparisonExpression(
@NotBlank(message = "json node path can not be null in filter") String path,
List<FilterOperation<?>> filterOperations,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,9 @@ public enum ElementComparisonOperator implements FilterOperator {
public String getOperator() {
return operator;
}

@Override
public FilterOperator flip() {
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, FilterOperator> operatorMap = new HashMap<>();

Expand All @@ -22,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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ public class LogicalExpression {

public enum LogicalOperator {
AND("$and"),
OR("$or");
OR("$or"),
NOT("$not");
private String operator;

LogicalOperator(String operator) {
Expand All @@ -25,10 +26,67 @@ public String getOperator() {
}
}

private final LogicalOperator logicalRelation;
private LogicalOperator logicalRelation;
private int totalComparisonExpressionCount;
private int totalIdComparisonExpressionCount;

/** This method will flip the operators and operand if logical operator is not */
public void traverseForNot(LogicalExpression parent) {
List<LogicalExpression> tempLogicalExpressions = new ArrayList<>(logicalExpressions);
for (LogicalExpression logicalExpression : tempLogicalExpressions) {
logicalExpression.traverseForNot(this);
}

Iterator<LogicalExpression> iterator = logicalExpressions.iterator();
while (iterator.hasNext()) {
LogicalExpression logicalExpression = iterator.next();
if (logicalExpression.logicalRelation == LogicalOperator.NOT) {
iterator.remove();
this.totalComparisonExpressionCount =
this.totalComparisonExpressionCount - logicalExpression.totalComparisonExpressionCount;
this.totalIdComparisonExpressionCount =
this.totalIdComparisonExpressionCount
- logicalExpression.totalIdComparisonExpressionCount;
}
}

if (logicalRelation == LogicalOperator.NOT) {
flip();
addToParent(parent);
}
}

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();
comparisonExpressions.stream()
.forEach(comparisonExpression -> orLogic.addComparisonExpression(comparisonExpression));
parent.addLogicalExpression(orLogic);
} else {
if (comparisonExpressions.size() == 1) {
// Unary not, after push down will become additional condition
parent.addComparisonExpression(comparisonExpressions.get(0));
}
}
comparisonExpressions.clear();
}

private void flip() {
if (logicalRelation == LogicalOperator.AND) {
logicalRelation = LogicalOperator.OR;
} else if (logicalRelation == LogicalOperator.OR) {
logicalRelation = LogicalOperator.AND;
}
tatu-at-datastax marked this conversation as resolved.
Show resolved Hide resolved
for (LogicalExpression logicalExpression : logicalExpressions) {
logicalExpression.flip();
}
for (ComparisonExpression comparisonExpression : comparisonExpressions) {
comparisonExpression.flip();
}
}

public List<LogicalExpression> logicalExpressions;
public List<ComparisonExpression> comparisonExpressions;

Expand All @@ -51,6 +109,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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 LTE;
case GTE:
return LT;
case LT:
return GTE;
case LTE:
return GT;
}
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ public FilterClause deserialize(
LogicalExpression implicitAnd = LogicalExpression.and();
populateExpression(implicitAnd, filterNode);
validate(implicitAnd);
// push down not operator
implicitAnd.traverseForNot(null);

return new FilterClause(implicitAnd);
}

Expand Down Expand Up @@ -94,6 +97,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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Object> arrayValue;
private final boolean negation;

public AllFilter(String path, List<Object> arrayValue) {
public AllFilter(String path, List<Object> arrayValue, boolean negation) {
super(path);
this.arrayValue = arrayValue;
this.negation = negation;
}

public boolean isNegation() {
return negation;
}

@Override
Expand All @@ -617,7 +623,7 @@ public List<BuiltCondition> getAll() {
result.add(
BuiltCondition.of(
DATA_CONTAINS,
Predicate.CONTAINS,
negation ? Predicate.NOT_CONTAINS : Predicate.CONTAINS,
new JsonTerm(getHashValue(new DocValueHasher(), getPath(), value))));
}
return result;
Expand All @@ -631,8 +637,8 @@ public BuiltCondition get() {

/** Filter for document where array has specified number of elements */
public static class SizeFilter extends MapFilterBase<Integer> {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ private static Expression<BuiltCondition> buildExpressionRecursive(
List<BuiltCondition> allFilterConditions = allFilter.getAll();
List<Variable<BuiltCondition>> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public abstract class FilterableResolver<T extends Command & Filterable> {
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();
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -292,12 +295,27 @@ public static List<DBFilterBase> findDynamic(CaptureExpression captureExpression

if (captureExpression.marker() == ALL_GROUP) {
List<Object> arrayValue = (List<Object>) 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) {
List<Object> arrayValue = (List<Object>) filterOperation.operand().value();
filters.add(new DBFilterBase.AllFilter(captureExpression.path(), arrayValue, true));
}

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 {
tatu-at-datastax marked this conversation as resolved.
Show resolved Hide resolved
operator = DBFilterBase.MapFilterBase.Operator.MAP_NOT_EQUALS;
}
filters.add(
new DBFilterBase.SizeFilter(captureExpression.path(), operator, Math.abs(size)));
}

if (captureExpression.marker() == ARRAY_EQUALS) {
Expand Down
Loading