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

make inclause behaviour consistent between mongo and postgres #186

Merged
merged 10 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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 @@ -1251,34 +1251,47 @@ public void testQueryV1AggregationFilter(String dataStoreName) throws IOExceptio

@ParameterizedTest
@ArgumentsSource(AllProvider.class)
public void testQueryV1AggregationWithInFilterWithPrimitiveLhs(final String dataStoreName) throws IOException {
public void testQueryV1AggregationWithInFilterWithPrimitiveLhs(final String dataStoreName)
throws IOException {
final Collection collection = getCollection(dataStoreName);
final Query query = Query.builder()
final Query query =
Query.builder()
.addSelection(IdentifierExpression.of("item"))
.addSelection(IdentifierExpression.of("quantity"))
.setFilter(
RelationalExpression.of(IdentifierExpression.of("item"), IN, ConstantExpression.ofStrings(List.of("Comb", "Shampoo")))
)
RelationalExpression.of(
IdentifierExpression.of("item"),
IN,
ConstantExpression.ofStrings(List.of("Comb", "Shampoo"))))
.build();

final Iterator<Document> resultDocs = collection.aggregate(query);
assertDocsAndSizeEqualWithoutOrder(dataStoreName, resultDocs, "query/test_primitive_lhs_in_filter_aggr_response.json", 4);
assertDocsAndSizeEqualWithoutOrder(
dataStoreName, resultDocs, "query/test_primitive_lhs_in_filter_aggr_response.json", 4);
}

@ParameterizedTest
@ArgumentsSource(AllProvider.class)
public void testQueryV1AggregationWithInFilterWithArrayLhs(final String dataStoreName) throws IOException {
public void testQueryV1AggregationWithInFilterWithArrayLhs(final String dataStoreName)
throws IOException {
final Collection collection = getCollection(dataStoreName);
final Query query = Query.builder()
final Query query =
Query.builder()
.addSelection(IdentifierExpression.of("item"))
.addSelection(IdentifierExpression.of("price"))
.setFilter(
RelationalExpression.of(IdentifierExpression.of("props.colors"), IN, ConstantExpression.ofStrings(List.of("Orange")))
)
RelationalExpression.of(
IdentifierExpression.of("props.colors"),
IN,
ConstantExpression.ofStrings(List.of("Orange"))))
.build();

final Iterator<Document> resultDocs = collection.aggregate(query);
assertDocsAndSizeEqualWithoutOrder(dataStoreName, resultDocs, "query/test_json_column_array_lhs_in_filter_aggr_response.json", 1);
assertDocsAndSizeEqualWithoutOrder(
dataStoreName,
resultDocs,
"query/test_json_column_array_lhs_in_filter_aggr_response.json",
1);
}

@ParameterizedTest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@
"item":"Shampoo",
"quantity": 10
}
]
]
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,13 @@
.map(
val -> {
paramsBuilder.addObjectParam(val).addObjectParam(val);
return "((jsonb_typeof(to_jsonb("+parsedExpression+")) = 'array' AND to_jsonb("+parsedExpression+") @> jsonb_build_array(?)) OR (jsonb_build_array(" + parsedExpression + ") @> jsonb_build_array(?)))";
return "((jsonb_typeof(to_jsonb("
SrikarMannepalli marked this conversation as resolved.
Show resolved Hide resolved
+ parsedExpression
+ ")) = 'array' AND to_jsonb("
+ parsedExpression
+ ") @> jsonb_build_array(?)) OR (jsonb_build_array("
+ parsedExpression
+ ") @> jsonb_build_array(?)))";
})
.collect(Collectors.joining(" OR "));
filterStringBuilder.append(collect);
Expand Down Expand Up @@ -418,7 +424,6 @@
String sqlOperator;
boolean isMultiValued = false;
boolean isContainsOp = false;
boolean isInOP = false;
switch (op) {
case "EQ":
case "=":
Expand Down Expand Up @@ -450,25 +455,31 @@
case "NOT_IN":
case "NOT IN":
// NOTE: Pl. refer this in non-parsed expression for limitation of this filter
// In order to make the behaviour same as for mongo, the not in operator would match only if
// the lhs
// and rhs have no intersection at all
// NOTE: This doesn't work in case the lhs is a function
SrikarMannepalli marked this conversation as resolved.
Show resolved Hide resolved
sqlOperator = " NOT IN ";
isMultiValued = true;
isInOP = true;
filterString
.append(" IS NULL OR")
.append(" NOT ")
.append(" NOT (")
.append(
prepareFilterStringForInOperator(

Check warning on line 468 in document-store/src/main/java/org/hypertrace/core/documentstore/postgres/utils/PostgresUtils.java

View check run for this annotation

Codecov / codecov/patch

document-store/src/main/java/org/hypertrace/core/documentstore/postgres/utils/PostgresUtils.java#L464-L468

Added lines #L464 - L468 were not covered by tests
preparedExpression, (Iterable<Object>) value, paramsBuilder));
break;
preparedExpression, (Iterable<Object>) value, paramsBuilder))
.append(")");
return filterString.toString();

Check warning on line 471 in document-store/src/main/java/org/hypertrace/core/documentstore/postgres/utils/PostgresUtils.java

View check run for this annotation

Codecov / codecov/patch

document-store/src/main/java/org/hypertrace/core/documentstore/postgres/utils/PostgresUtils.java#L470-L471

Added lines #L470 - L471 were not covered by tests
case "IN":
// In order to make the behaviour same as for mongo, the in operator would match if the lhs
// and rhs have any intersection at all
// NOTE: Pl. refer this in non-parsed expression for limitation of this filter
// NOTE: This doesn't work in case the lhs is a function
SrikarMannepalli marked this conversation as resolved.
Show resolved Hide resolved
sqlOperator = " IN ";
isMultiValued = true;
isInOP = true;
filterString =
prepareFilterStringForInOperator(
preparedExpression, (Iterable<Object>) value, paramsBuilder);
break;
return filterString.toString();
case "NOT_EXISTS":
case "NOT EXISTS":
sqlOperator = " IS NULL ";
Expand Down Expand Up @@ -505,10 +516,6 @@
throw new UnsupportedOperationException(UNSUPPORTED_QUERY_OPERATION);
}

if (isInOP) {
return filterString.toString();
}

filterString.append(sqlOperator);
if (value != null) {
if (isMultiValued) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -648,19 +648,19 @@ void testFindWithSortingAndPagination() {
+ "document->'quantity' AS \"quantity\", "
+ "document->'date' AS \"date\" "
+ "FROM \"testCollection\" "
+ "WHERE ((jsonb_build_array(document->>'item') @> jsonb_build_array(?)) OR (jsonb_build_array(document->>'item') @> jsonb_build_array(?)) OR (jsonb_build_array(document->>'item') @> jsonb_build_array(?)) OR (jsonb_build_array(document->>'item') @> jsonb_build_array(?))) "
+ "WHERE (((jsonb_typeof(to_jsonb(document->'item')) = 'array' AND to_jsonb(document->'item') @> jsonb_build_array(?)) OR (jsonb_build_array(document->'item') @> jsonb_build_array(?))) OR ((jsonb_typeof(to_jsonb(document->'item')) = 'array' AND to_jsonb(document->'item') @> jsonb_build_array(?)) OR (jsonb_build_array(document->'item') @> jsonb_build_array(?))) OR ((jsonb_typeof(to_jsonb(document->'item')) = 'array' AND to_jsonb(document->'item') @> jsonb_build_array(?)) OR (jsonb_build_array(document->'item') @> jsonb_build_array(?))) OR ((jsonb_typeof(to_jsonb(document->'item')) = 'array' AND to_jsonb(document->'item') @> jsonb_build_array(?)) OR (jsonb_build_array(document->'item') @> jsonb_build_array(?)))) "
+ "ORDER BY document->'quantity' DESC NULLS LAST,document->'item' ASC NULLS FIRST "
+ "OFFSET ? LIMIT ?",
sql);

Params params = postgresQueryParser.getParamsBuilder().build();
assertEquals(6, params.getObjectParams().size());
assertEquals(10, params.getObjectParams().size());
assertEquals("Mirror", params.getObjectParams().get(1));
assertEquals("Comb", params.getObjectParams().get(2));
assertEquals("Shampoo", params.getObjectParams().get(3));
assertEquals("Bottle", params.getObjectParams().get(4));
assertEquals(1, params.getObjectParams().get(5));
assertEquals(3, params.getObjectParams().get(6));
assertEquals("Comb", params.getObjectParams().get(3));
assertEquals("Shampoo", params.getObjectParams().get(5));
assertEquals("Bottle", params.getObjectParams().get(7));
assertEquals(1, params.getObjectParams().get(9));
assertEquals(3, params.getObjectParams().get(10));
}

@Test
Expand Down Expand Up @@ -1121,16 +1121,16 @@ void testQueryQ1AggregationFilterWithStringInFilterAlongWithNonAliasFields() {
"SELECT COUNT(DISTINCT document->>'quantity' ) AS \"qty_count\", "
+ "document->'item' AS \"item\", document->'price' AS \"price\" "
+ "FROM \"testCollection\" GROUP BY document->'item',document->'price' "
+ "HAVING (COUNT(DISTINCT document->>'quantity' ) <= ?) AND (((jsonb_build_array(CAST (document->'item' AS TEXT)) @> jsonb_build_array(?)) OR (jsonb_build_array(CAST (document->'item' AS TEXT)) @> jsonb_build_array(?)) OR (jsonb_build_array(CAST (document->'item' AS TEXT)) @> jsonb_build_array(?)) OR (jsonb_build_array(CAST (document->'item' AS TEXT)) @> jsonb_build_array(?))))",
+ "HAVING (COUNT(DISTINCT document->>'quantity' ) <= ?) AND ((((jsonb_typeof(to_jsonb(CAST (document->'item' AS TEXT))) = 'array' AND to_jsonb(CAST (document->'item' AS TEXT)) @> jsonb_build_array(?)) OR (jsonb_build_array(CAST (document->'item' AS TEXT)) @> jsonb_build_array(?))) OR ((jsonb_typeof(to_jsonb(CAST (document->'item' AS TEXT))) = 'array' AND to_jsonb(CAST (document->'item' AS TEXT)) @> jsonb_build_array(?)) OR (jsonb_build_array(CAST (document->'item' AS TEXT)) @> jsonb_build_array(?))) OR ((jsonb_typeof(to_jsonb(CAST (document->'item' AS TEXT))) = 'array' AND to_jsonb(CAST (document->'item' AS TEXT)) @> jsonb_build_array(?)) OR (jsonb_build_array(CAST (document->'item' AS TEXT)) @> jsonb_build_array(?))) OR ((jsonb_typeof(to_jsonb(CAST (document->'item' AS TEXT))) = 'array' AND to_jsonb(CAST (document->'item' AS TEXT)) @> jsonb_build_array(?)) OR (jsonb_build_array(CAST (document->'item' AS TEXT)) @> jsonb_build_array(?)))))",
sql);

Params params = postgresQueryParser.getParamsBuilder().build();
assertEquals(5, params.getObjectParams().size());
assertEquals(9, params.getObjectParams().size());
assertEquals(10, params.getObjectParams().get(1));
assertEquals("\"Mirror\"", params.getObjectParams().get(2));
assertEquals("\"Comb\"", params.getObjectParams().get(3));
assertEquals("\"Shampoo\"", params.getObjectParams().get(4));
assertEquals("\"Bottle\"", params.getObjectParams().get(5));
assertEquals("\"Comb\"", params.getObjectParams().get(4));
assertEquals("\"Shampoo\"", params.getObjectParams().get(6));
assertEquals("\"Bottle\"", params.getObjectParams().get(8));
}

@Test
Expand Down Expand Up @@ -1164,16 +1164,16 @@ void testQueryQ1DistinctCountAggregationWithOnlyFilter() {
assertEquals(
"SELECT COUNT(DISTINCT document->>'quantity' ) AS \"qty_count\" "
+ "FROM \"testCollection\" "
+ "WHERE (CAST (document->>'price' AS NUMERIC) <= ?) AND (((jsonb_build_array(document->>'item') @> jsonb_build_array(?)) OR (jsonb_build_array(document->>'item') @> jsonb_build_array(?)) OR (jsonb_build_array(document->>'item') @> jsonb_build_array(?)) OR (jsonb_build_array(document->>'item') @> jsonb_build_array(?))))",
+ "WHERE (CAST (document->>'price' AS NUMERIC) <= ?) AND ((((jsonb_typeof(to_jsonb(document->'item')) = 'array' AND to_jsonb(document->'item') @> jsonb_build_array(?)) OR (jsonb_build_array(document->'item') @> jsonb_build_array(?))) OR ((jsonb_typeof(to_jsonb(document->'item')) = 'array' AND to_jsonb(document->'item') @> jsonb_build_array(?)) OR (jsonb_build_array(document->'item') @> jsonb_build_array(?))) OR ((jsonb_typeof(to_jsonb(document->'item')) = 'array' AND to_jsonb(document->'item') @> jsonb_build_array(?)) OR (jsonb_build_array(document->'item') @> jsonb_build_array(?))) OR ((jsonb_typeof(to_jsonb(document->'item')) = 'array' AND to_jsonb(document->'item') @> jsonb_build_array(?)) OR (jsonb_build_array(document->'item') @> jsonb_build_array(?)))))",
sql);

Params params = postgresQueryParser.getParamsBuilder().build();
assertEquals(5, params.getObjectParams().size());
assertEquals(9, params.getObjectParams().size());
assertEquals(10, params.getObjectParams().get(1));
assertEquals("Mirror", params.getObjectParams().get(2));
assertEquals("Comb", params.getObjectParams().get(3));
assertEquals("Shampoo", params.getObjectParams().get(4));
assertEquals("Bottle", params.getObjectParams().get(5));
assertEquals("Comb", params.getObjectParams().get(4));
assertEquals("Shampoo", params.getObjectParams().get(6));
assertEquals("Bottle", params.getObjectParams().get(8));
}

@Test
Expand Down