-
Notifications
You must be signed in to change notification settings - Fork 8
fix: support for attribute expressions in selections #129
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
Merged
aaron-steinfeld
merged 27 commits into
main
from
attribute-expression-selections-refactor
Dec 29, 2021
Merged
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
74de0f2
added sup for attr expr in execution context
61f141c
added sup for attr expr in projection transformation
d88110b
Merge remote-tracking branch 'origin/main' into feat_api_support_for_…
8bf4b47
added sup for attr expr in pinotbasedrequesthandler
0eedfb7
refactored
78b71d0
added unit test for execution context
5ed48e4
added unit test in pinot based rqst handler
0233401
changed function signature
8f25119
resolved comments
a613792
nit
4603ef8
fixed unit tests
a1b9cfe
added template for new tests
46132ca
added integ test with attr expr for existing queries
75aad05
added functions for integ test
0a57dd9
added new integ tests
929e3c9
nit
d51263a
added contains key filter for selection
b4878a5
fix: support for attribute expressioons in selections
aaron-steinfeld 3a35f1c
test: update tests
aaron-steinfeld cb76232
style: fix nit
aaron-steinfeld 21a7aa6
Merge remote-tracking branch 'origin/main' into attribute-expression-…
aaron-steinfeld 52bc346
Apply suggestions from code review
aaron-steinfeld d13c3d7
added unit test for contains_key_value op
e250e45
Merge branch 'attribute-expression-selections-refactor' of github.com…
a52fa5e
chore: update dependencies
aaron-steinfeld fa6bc54
chore: fix integration tests
aaron-steinfeld 9f23278
Merge branch 'main' into attribute-expression-selections-refactor
kotharironak File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
174 changes: 174 additions & 0 deletions
174
...ice-impl/src/main/java/org/hypertrace/core/query/service/AbstractQueryTransformation.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| package org.hypertrace.core.query.service; | ||
|
|
||
| import static io.reactivex.rxjava3.core.Single.zip; | ||
|
|
||
| import io.reactivex.rxjava3.core.Observable; | ||
| import io.reactivex.rxjava3.core.Single; | ||
| import java.util.List; | ||
| import org.hypertrace.core.query.service.api.AttributeExpression; | ||
| import org.hypertrace.core.query.service.api.ColumnIdentifier; | ||
| import org.hypertrace.core.query.service.api.Expression; | ||
| import org.hypertrace.core.query.service.api.Filter; | ||
| import org.hypertrace.core.query.service.api.Function; | ||
| import org.hypertrace.core.query.service.api.LiteralConstant; | ||
| import org.hypertrace.core.query.service.api.OrderByExpression; | ||
| import org.hypertrace.core.query.service.api.QueryRequest; | ||
| import org.slf4j.Logger; | ||
|
|
||
| public abstract class AbstractQueryTransformation implements QueryTransformation { | ||
|
|
||
| protected abstract Logger getLogger(); | ||
|
|
||
| @Override | ||
| public Single<QueryRequest> transform( | ||
| QueryRequest queryRequest, QueryTransformationContext transformationContext) { | ||
| return zip( | ||
| this.transformExpressionList(queryRequest.getSelectionList()), | ||
| this.transformExpressionList(queryRequest.getAggregationList()), | ||
| this.transformFilter(queryRequest.getFilter()), | ||
| this.transformExpressionList(queryRequest.getGroupByList()), | ||
| this.transformOrderByList(queryRequest.getOrderByList()), | ||
| (selections, aggregations, filter, groupBys, orderBys) -> | ||
| this.rebuildRequest( | ||
| queryRequest, selections, aggregations, filter, groupBys, orderBys)) | ||
| .doOnSuccess(transformed -> this.debugLogIfRequestTransformed(queryRequest, transformed)); | ||
| } | ||
|
|
||
| protected QueryRequest rebuildRequest( | ||
| QueryRequest original, | ||
| List<Expression> selections, | ||
| List<Expression> aggregations, | ||
| Filter filter, | ||
| List<Expression> groupBys, | ||
| List<OrderByExpression> orderBys) { | ||
|
|
||
| QueryRequest.Builder builder = original.toBuilder(); | ||
|
|
||
| if (Filter.getDefaultInstance().equals(filter)) { | ||
| builder.clearFilter(); | ||
| } else { | ||
| builder.setFilter(filter); | ||
| } | ||
|
|
||
| return builder | ||
| .clearSelection() | ||
| .addAllSelection(selections) | ||
| .clearAggregation() | ||
| .addAllAggregation(aggregations) | ||
| .clearGroupBy() | ||
| .addAllGroupBy(groupBys) | ||
| .clearOrderBy() | ||
| .addAllOrderBy(orderBys) | ||
| .build(); | ||
| } | ||
|
|
||
| protected Single<Expression> transformExpression(Expression expression) { | ||
| switch (expression.getValueCase()) { | ||
| case COLUMNIDENTIFIER: | ||
| return this.transformColumnIdentifier(expression.getColumnIdentifier()); | ||
| case ATTRIBUTE_EXPRESSION: | ||
| return this.transformAttributeExpression(expression.getAttributeExpression()); | ||
| case FUNCTION: | ||
| return this.transformFunction(expression.getFunction()); | ||
| case ORDERBY: | ||
| return this.transformOrderBy(expression.getOrderBy()) | ||
| .map(expression.toBuilder()::setOrderBy) | ||
| .map(Expression.Builder::build); | ||
| case LITERAL: | ||
| return this.transformLiteral(expression.getLiteral()); | ||
| case VALUE_NOT_SET: | ||
| default: | ||
| return Single.just(expression); | ||
| } | ||
| } | ||
|
|
||
| protected Single<Expression> transformColumnIdentifier(ColumnIdentifier columnIdentifier) { | ||
| return Single.just(Expression.newBuilder().setColumnIdentifier(columnIdentifier).build()); | ||
| } | ||
|
|
||
| protected Single<Expression> transformAttributeExpression( | ||
| AttributeExpression attributeExpression) { | ||
| return Single.just(Expression.newBuilder().setAttributeExpression(attributeExpression).build()); | ||
| } | ||
|
|
||
| protected Single<Expression> transformLiteral(LiteralConstant literalConstant) { | ||
| return Single.just(Expression.newBuilder().setLiteral(literalConstant).build()); | ||
| } | ||
|
|
||
| protected Single<Expression> transformFunction(Function function) { | ||
| return this.transformExpressionList(function.getArgumentsList()) | ||
| .map(expressions -> function.toBuilder().clearArguments().addAllArguments(expressions)) | ||
| .map(Function.Builder::build) | ||
| .map(Expression.newBuilder()::setFunction) | ||
| .map(Expression.Builder::build); | ||
| } | ||
|
|
||
| protected Single<OrderByExpression> transformOrderBy(OrderByExpression orderBy) { | ||
| return this.transformExpression(orderBy.getExpression()) | ||
| .map(orderBy.toBuilder()::setExpression) | ||
| .map(OrderByExpression.Builder::build); | ||
| } | ||
|
|
||
| protected Single<Filter> transformFilter(Filter filter) { | ||
| if (filter.equals(Filter.getDefaultInstance())) { | ||
| return Single.just(filter); | ||
| } | ||
|
|
||
| Single<Expression> lhsSingle = this.transformExpression(filter.getLhs()); | ||
| Single<Expression> rhsSingle = this.transformExpression(filter.getRhs()); | ||
| Single<List<Filter>> childFilterListSingle = | ||
| Observable.fromIterable(filter.getChildFilterList()) | ||
| .concatMapSingle(this::transformFilter) | ||
| .toList(); | ||
| return zip( | ||
| lhsSingle, | ||
| rhsSingle, | ||
| childFilterListSingle, | ||
| (lhs, rhs, childFilterList) -> | ||
| this.rebuildFilterOmittingDefaults(filter, lhs, rhs, childFilterList)); | ||
| } | ||
|
|
||
| private Single<List<Expression>> transformExpressionList(List<Expression> expressionList) { | ||
| return Observable.fromIterable(expressionList) | ||
| .concatMapSingle(this::transformExpression) | ||
| .toList(); | ||
| } | ||
|
|
||
| private Single<List<OrderByExpression>> transformOrderByList( | ||
| List<OrderByExpression> orderByList) { | ||
| return Observable.fromIterable(orderByList).concatMapSingle(this::transformOrderBy).toList(); | ||
| } | ||
|
|
||
| /** | ||
| * This doesn't change any functional behavior, but omits fields that aren't needed, shrinking the | ||
| * object and keeping it equivalent to the source object for equality checks. | ||
| */ | ||
| private Filter rebuildFilterOmittingDefaults( | ||
| Filter original, Expression lhs, Expression rhs, List<Filter> childFilters) { | ||
| Filter.Builder builder = original.toBuilder(); | ||
|
|
||
| if (Expression.getDefaultInstance().equals(lhs)) { | ||
| builder.clearLhs(); | ||
| } else { | ||
| builder.setLhs(lhs); | ||
| } | ||
|
|
||
| if (Expression.getDefaultInstance().equals(rhs)) { | ||
| builder.clearRhs(); | ||
| } else { | ||
| builder.setRhs(rhs); | ||
| } | ||
|
|
||
| return builder.clearChildFilter().addAllChildFilter(childFilters).build(); | ||
| } | ||
|
|
||
| private void debugLogIfRequestTransformed(QueryRequest original, QueryRequest transformed) { | ||
| if (!original.equals(transformed)) { | ||
| getLogger() | ||
| .debug( | ||
| "Request transformation occurred. Original request: {} Transformed Request: {}", | ||
| original, | ||
| transformed); | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
18 changes: 18 additions & 0 deletions
18
...ava/org/hypertrace/core/query/service/attribubteexpression/AttributeExpressionModule.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| package org.hypertrace.core.query.service.attribubteexpression; | ||
|
|
||
| import com.google.inject.AbstractModule; | ||
| import com.google.inject.multibindings.Multibinder; | ||
| import org.hypertrace.core.query.service.QueryTransformation; | ||
|
|
||
| public class AttributeExpressionModule extends AbstractModule { | ||
|
|
||
| @Override | ||
| protected void configure() { | ||
| Multibinder<QueryTransformation> transformationMultibinder = | ||
| Multibinder.newSetBinder(binder(), QueryTransformation.class); | ||
| transformationMultibinder | ||
| .addBinding() | ||
| .to(AttributeExpressionSubpathExistsFilteringTransformation.class); | ||
| transformationMultibinder.addBinding().to(AttributeExpressionNormalizationTransformation.class); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
why is this needed?
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.
As part of this PR, we put in the concept of ordered transformations, so this ensures their order (since mutli injection does not).