-
Notifications
You must be signed in to change notification settings - Fork 10
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
Array filtering support (Part #3): Postgres relational filter refactoring #189
Changes from all commits
e8f06db
cd439f9
44e6690
a348ca8
c6ba6cd
157f26e
37058ad
0960315
cc4347c
369daa4
7ac51bd
0021c9a
c074d5c
ad3c865
3ffa433
dba24d4
6c8a7c5
879c87e
ca0c7c0
73f7125
7d273f5
d157442
79c44e2
4fabba5
6defee8
57a10f8
bfde2f4
19cbb1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package org.hypertrace.core.documentstore.postgres.query.v1.parser.builder; | ||
|
||
import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; | ||
import org.hypertrace.core.documentstore.postgres.query.v1.vistors.PostgresSelectTypeExpressionVisitor; | ||
|
||
public interface PostgresSelectExpressionParserBuilder { | ||
PostgresSelectTypeExpressionVisitor build(final RelationalExpression expression); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
package org.hypertrace.core.documentstore.postgres.query.v1.parser.builder; | ||
|
||
import static org.hypertrace.core.documentstore.postgres.utils.PostgresUtils.getType; | ||
|
||
import lombok.AllArgsConstructor; | ||
import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; | ||
import org.hypertrace.core.documentstore.postgres.query.v1.PostgresQueryParser; | ||
import org.hypertrace.core.documentstore.postgres.query.v1.vistors.PostgresConstantExpressionVisitor; | ||
import org.hypertrace.core.documentstore.postgres.query.v1.vistors.PostgresDataAccessorIdentifierExpressionVisitor; | ||
import org.hypertrace.core.documentstore.postgres.query.v1.vistors.PostgresFieldIdentifierExpressionVisitor; | ||
import org.hypertrace.core.documentstore.postgres.query.v1.vistors.PostgresFunctionExpressionVisitor; | ||
import org.hypertrace.core.documentstore.postgres.query.v1.vistors.PostgresSelectTypeExpressionVisitor; | ||
|
||
@AllArgsConstructor | ||
public class PostgresSelectExpressionParserBuilderImpl | ||
implements PostgresSelectExpressionParserBuilder { | ||
|
||
private final PostgresQueryParser postgresQueryParser; | ||
|
||
@Override | ||
public PostgresSelectTypeExpressionVisitor build(final RelationalExpression expression) { | ||
switch (expression.getOperator()) { | ||
case CONTAINS: | ||
case NOT_CONTAINS: | ||
case EXISTS: | ||
case NOT_EXISTS: | ||
case IN: | ||
case NOT_IN: | ||
return new PostgresFunctionExpressionVisitor( | ||
new PostgresFieldIdentifierExpressionVisitor(this.postgresQueryParser)); | ||
|
||
default: | ||
return new PostgresFunctionExpressionVisitor( | ||
new PostgresDataAccessorIdentifierExpressionVisitor( | ||
this.postgresQueryParser, | ||
getType(expression.getRhs().accept(new PostgresConstantExpressionVisitor())))); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; | ||
|
||
import com.fasterxml.jackson.core.JsonProcessingException; | ||
import com.fasterxml.jackson.databind.JsonNode; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import org.hypertrace.core.documentstore.Document; | ||
import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; | ||
|
||
class PostgresContainsRelationalFilterParser implements PostgresRelationalFilterParser { | ||
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); | ||
|
||
@Override | ||
public String parse( | ||
final RelationalExpression expression, final PostgresRelationalFilterContext context) { | ||
final String parsedLhs = expression.getLhs().accept(context.lhsParser()); | ||
final Object parsedRhs = expression.getRhs().accept(context.rhsParser()); | ||
|
||
final Object convertedRhs = prepareJsonValueForContainsOp(parsedRhs); | ||
context.getParamsBuilder().addObjectParam(convertedRhs); | ||
|
||
return String.format("%s @> ?::jsonb", parsedLhs); | ||
} | ||
|
||
static String prepareJsonValueForContainsOp(final Object value) { | ||
if (value instanceof Document) { | ||
return prepareDocumentValueForContainsOp((Document) value); | ||
} else { | ||
return prepareValueForContainsOp(value); | ||
} | ||
} | ||
|
||
private static String prepareDocumentValueForContainsOp(final Document document) { | ||
try { | ||
final JsonNode node = OBJECT_MAPPER.readTree(document.toJson()); | ||
if (node.isArray()) { | ||
return document.toJson(); | ||
Check warning on line 36 in document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresContainsRelationalFilterParser.java
|
||
} else { | ||
return "[" + document.toJson() + "]"; | ||
} | ||
} catch (JsonProcessingException e) { | ||
throw new RuntimeException(e); | ||
Check warning on line 41 in document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresContainsRelationalFilterParser.java
|
||
} | ||
} | ||
|
||
private static String prepareValueForContainsOp(final Object value) { | ||
try { | ||
if (value instanceof Iterable<?>) { | ||
return OBJECT_MAPPER.writeValueAsString(value); | ||
Check warning on line 48 in document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresContainsRelationalFilterParser.java
|
||
} else { | ||
return "[" + OBJECT_MAPPER.writeValueAsString(value) + "]"; | ||
} | ||
} catch (JsonProcessingException ex) { | ||
throw new RuntimeException(ex); | ||
Check warning on line 53 in document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresContainsRelationalFilterParser.java
|
||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; | ||
|
||
import org.hypertrace.core.documentstore.expression.impl.ConstantExpression; | ||
import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; | ||
|
||
class PostgresExistsRelationalFilterParser implements PostgresRelationalFilterParser { | ||
@Override | ||
public String parse( | ||
final RelationalExpression expression, final PostgresRelationalFilterContext context) { | ||
final String parsedLhs = expression.getLhs().accept(context.lhsParser()); | ||
final boolean parsedRhs = !ConstantExpression.of(false).equals(expression.getRhs()); | ||
return parsedRhs | ||
? String.format("%s IS NOT NULL", parsedLhs) | ||
: String.format("%s IS NULL", parsedLhs); | ||
Check warning on line 14 in document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresExistsRelationalFilterParser.java
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; | ||
|
||
import java.util.stream.Collectors; | ||
import java.util.stream.StreamSupport; | ||
import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; | ||
import org.hypertrace.core.documentstore.postgres.Params; | ||
|
||
class PostgresInRelationalFilterParser implements PostgresRelationalFilterParser { | ||
|
||
@Override | ||
public String parse( | ||
final RelationalExpression expression, final PostgresRelationalFilterContext context) { | ||
final String parsedLhs = expression.getLhs().accept(context.lhsParser()); | ||
final Iterable<Object> parsedRhs = expression.getRhs().accept(context.rhsParser()); | ||
|
||
return prepareFilterStringForInOperator(parsedLhs, parsedRhs, context.getParamsBuilder()); | ||
} | ||
|
||
private String prepareFilterStringForInOperator( | ||
final String parsedLhs, | ||
final Iterable<Object> parsedRhs, | ||
final Params.Builder paramsBuilder) { | ||
// In order to make the behaviour same as for Mongo, the "NOT_IN" operator would match if the | ||
// LHS and RHS have any intersection (i.e. non-empty intersection) | ||
return StreamSupport.stream(parsedRhs.spliterator(), false) | ||
.map( | ||
value -> { | ||
paramsBuilder.addObjectParam(value).addObjectParam(value); | ||
return String.format( | ||
"((jsonb_typeof(to_jsonb(%s)) = 'array' AND to_jsonb(%s) @> jsonb_build_array(?)) OR (jsonb_build_array(%s) @> jsonb_build_array(?)))", | ||
parsedLhs, parsedLhs, parsedLhs); | ||
}) | ||
.collect(Collectors.joining(" OR ", "(", ")")); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; | ||
|
||
import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; | ||
|
||
class PostgresLikeRelationalFilterParser implements PostgresRelationalFilterParser { | ||
|
||
@Override | ||
public String parse( | ||
final RelationalExpression expression, final PostgresRelationalFilterContext context) { | ||
final Object parsedLhs = expression.getLhs().accept(context.lhsParser()); | ||
final Object parsedRhs = expression.getRhs().accept(context.rhsParser()); | ||
Check warning on line 11 in document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresLikeRelationalFilterParser.java
|
||
|
||
// Append ".*" at beginning and end of value to do a regex | ||
// Like operator is only applicable for string RHS. Hence, convert the RHS to string | ||
final String rhsValue = ".*" + parsedRhs + ".*"; | ||
context.getParamsBuilder().addObjectParam(rhsValue); | ||
Check warning on line 16 in document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresLikeRelationalFilterParser.java
|
||
|
||
// Case-insensitive regex search | ||
return String.format("%s ~* ?", parsedLhs); | ||
Check warning on line 19 in document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresLikeRelationalFilterParser.java
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; | ||
|
||
import static org.hypertrace.core.documentstore.postgres.query.v1.parser.filter.PostgresContainsRelationalFilterParser.prepareJsonValueForContainsOp; | ||
|
||
import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; | ||
|
||
class PostgresNotContainsRelationalFilterParser implements PostgresRelationalFilterParser { | ||
@Override | ||
public String parse( | ||
final RelationalExpression expression, final PostgresRelationalFilterContext context) { | ||
final String parsedLhs = expression.getLhs().accept(context.lhsParser()); | ||
final Object parsedRhs = expression.getRhs().accept(context.rhsParser()); | ||
|
||
final Object convertedRhs = prepareJsonValueForContainsOp(parsedRhs); | ||
context.getParamsBuilder().addObjectParam(convertedRhs); | ||
|
||
return String.format("%s IS NULL OR NOT %s @> ?::jsonb", parsedLhs, parsedLhs); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; | ||
|
||
import org.hypertrace.core.documentstore.expression.impl.ConstantExpression; | ||
import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; | ||
|
||
class PostgresNotExistsRelationalFilterParser implements PostgresRelationalFilterParser { | ||
@Override | ||
public String parse( | ||
final RelationalExpression expression, final PostgresRelationalFilterContext context) { | ||
final String parsedLhs = expression.getLhs().accept(context.lhsParser()); | ||
final boolean parsedRhs = ConstantExpression.of(false).equals(expression.getRhs()); | ||
return parsedRhs | ||
? String.format("%s IS NOT NULL", parsedLhs) | ||
Check warning on line 13 in document-store/src/main/java/org/hypertrace/core/documentstore/postgres/query/v1/parser/filter/PostgresNotExistsRelationalFilterParser.java
|
||
: String.format("%s IS NULL", parsedLhs); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; | ||
|
||
import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; | ||
|
||
class PostgresNotInRelationalFilterParser implements PostgresRelationalFilterParser { | ||
private static final PostgresInRelationalFilterParser inRelationalFilterParser = | ||
new PostgresInRelationalFilterParser(); | ||
|
||
@Override | ||
public String parse( | ||
final RelationalExpression expression, final PostgresRelationalFilterContext context) { | ||
final String parsedLhs = expression.getLhs().accept(context.lhsParser()); | ||
final String parsedInExpression = inRelationalFilterParser.parse(expression, context); | ||
return String.format("%s IS NULL OR NOT (%s)", parsedLhs, parsedInExpression); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; | ||
|
||
import lombok.Builder; | ||
import lombok.Builder.Default; | ||
import lombok.Value; | ||
import lombok.experimental.Accessors; | ||
import lombok.experimental.Delegate; | ||
import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; | ||
import org.hypertrace.core.documentstore.postgres.query.v1.PostgresQueryParser; | ||
import org.hypertrace.core.documentstore.postgres.query.v1.vistors.PostgresConstantExpressionVisitor; | ||
import org.hypertrace.core.documentstore.postgres.query.v1.vistors.PostgresSelectTypeExpressionVisitor; | ||
|
||
public interface PostgresRelationalFilterParser { | ||
String parse( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will every parse request have a different context? If not, shall we consider moving it as part of the Parser constructor? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes. That's the idea in the continuation PR @ #191 |
||
final RelationalExpression expression, final PostgresRelationalFilterContext context); | ||
|
||
@Value | ||
@Builder | ||
@Accessors(fluent = true) | ||
class PostgresRelationalFilterContext { | ||
PostgresSelectTypeExpressionVisitor lhsParser; | ||
|
||
@Default | ||
PostgresSelectTypeExpressionVisitor rhsParser = new PostgresConstantExpressionVisitor(); | ||
|
||
@Delegate PostgresQueryParser postgresQueryParser; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; | ||
|
||
import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; | ||
|
||
public interface PostgresRelationalFilterParserFactory { | ||
PostgresRelationalFilterParser parser(final RelationalExpression expression); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; | ||
|
||
import static java.util.Map.entry; | ||
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.CONTAINS; | ||
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.EXISTS; | ||
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.IN; | ||
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.LIKE; | ||
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.NOT_CONTAINS; | ||
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.NOT_EXISTS; | ||
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.NOT_IN; | ||
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.STARTS_WITH; | ||
|
||
import com.google.common.collect.Maps; | ||
import java.util.Map; | ||
import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; | ||
import org.hypertrace.core.documentstore.expression.operators.RelationalOperator; | ||
|
||
public class PostgresRelationalFilterParserFactoryImpl | ||
implements PostgresRelationalFilterParserFactory { | ||
private static final Map<RelationalOperator, PostgresRelationalFilterParser> parserMap = | ||
Maps.immutableEnumMap( | ||
Map.ofEntries( | ||
entry(CONTAINS, new PostgresContainsRelationalFilterParser()), | ||
entry(NOT_CONTAINS, new PostgresNotContainsRelationalFilterParser()), | ||
entry(EXISTS, new PostgresExistsRelationalFilterParser()), | ||
entry(NOT_EXISTS, new PostgresNotExistsRelationalFilterParser()), | ||
entry(IN, new PostgresInRelationalFilterParser()), | ||
entry(NOT_IN, new PostgresNotInRelationalFilterParser()), | ||
entry(LIKE, new PostgresLikeRelationalFilterParser()), | ||
entry(STARTS_WITH, new PostgresStartsWithRelationalFilterParser()))); | ||
private static final PostgresStandardRelationalFilterParser | ||
postgresStandardRelationalFilterParser = new PostgresStandardRelationalFilterParser(); | ||
|
||
@Override | ||
public PostgresRelationalFilterParser parser(final RelationalExpression expression) { | ||
return parserMap.getOrDefault(expression.getOperator(), postgresStandardRelationalFilterParser); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; | ||
|
||
import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; | ||
|
||
class PostgresStandardRelationalFilterParser implements PostgresRelationalFilterParser { | ||
private static final PostgresStandardRelationalOperatorMapper mapper = | ||
new PostgresStandardRelationalOperatorMapper(); | ||
|
||
@Override | ||
public String parse( | ||
final RelationalExpression expression, final PostgresRelationalFilterContext context) { | ||
final Object parsedLhs = expression.getLhs().accept(context.lhsParser()); | ||
final String operator = mapper.getMapping(expression.getOperator()); | ||
final Object parsedRhs = expression.getRhs().accept(context.rhsParser()); | ||
|
||
context.getParamsBuilder().addObjectParam(parsedRhs); | ||
return String.format("%s %s ?", parsedLhs, operator); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; | ||
|
||
import static java.util.Map.entry; | ||
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.EQ; | ||
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.GT; | ||
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.GTE; | ||
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.LT; | ||
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.LTE; | ||
import static org.hypertrace.core.documentstore.expression.operators.RelationalOperator.NEQ; | ||
|
||
import com.google.common.collect.Maps; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
import org.hypertrace.core.documentstore.expression.operators.RelationalOperator; | ||
|
||
class PostgresStandardRelationalOperatorMapper { | ||
private static final Map<RelationalOperator, String> mapping = | ||
Maps.immutableEnumMap( | ||
Map.ofEntries( | ||
entry(EQ, "="), | ||
entry(NEQ, "!="), | ||
entry(GT, ">"), | ||
entry(LT, "<"), | ||
entry(GTE, ">="), | ||
entry(LTE, "<="))); | ||
|
||
String getMapping(final RelationalOperator operator) { | ||
return Optional.ofNullable(mapping.get(operator)) | ||
.orElseThrow(() -> new UnsupportedOperationException("Unsupported operator: " + operator)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package org.hypertrace.core.documentstore.postgres.query.v1.parser.filter; | ||
|
||
import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; | ||
|
||
class PostgresStartsWithRelationalFilterParser implements PostgresRelationalFilterParser { | ||
@Override | ||
public String parse( | ||
final RelationalExpression expression, final PostgresRelationalFilterContext context) { | ||
final String parsedLhs = expression.getLhs().accept(context.lhsParser()); | ||
final Object parsedRhs = expression.getRhs().accept(context.rhsParser()); | ||
|
||
context.getParamsBuilder().addObjectParam(parsedRhs); | ||
return String.format("%s::text ^@ ?", parsedLhs); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IN
request look like?N
RHS values? For each of them, it will apply thejson_build_array
function.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically, the definition of
IN
is very loose when the LHS is an array.MongoDB treats,
IN
as a non-empty intersection between the LHS array and the RHS array. The Postgres behaviour was inconsistent with this definition breaking the parity. This PR https://github.com/hypertrace/document-store/pull/186/files addressed that by considering non-empty intersection between the LHS array and the RHS array forIN
for Postgres also. So, in this refactoring PR, I didn't change the behavior.