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
Aggregate index support in Cascades #1864
Merged
Merged
Conversation
This file contains 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
- treat aggregate index as a normal value index (on grouping columns). - basic matching on lower levels with Leaf and Intermediate rules seems to work, however the index is still not chosen ...
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
normen662
reviewed
Nov 4, 2022
...-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/CountValue.java
Outdated
Show resolved
Hide resolved
normen662
reviewed
Nov 4, 2022
...n/java/com/apple/foundationdb/record/query/plan/cascades/values/IndexOnlyAggregateValue.java
Outdated
Show resolved
Hide resolved
normen662
reviewed
Nov 4, 2022
...n/java/com/apple/foundationdb/record/query/plan/cascades/values/IndexOnlyAggregateValue.java
Outdated
Show resolved
Hide resolved
normen662
reviewed
Nov 4, 2022
...n/java/com/apple/foundationdb/record/query/plan/cascades/values/IndexableAggregateValue.java
Outdated
Show resolved
Hide resolved
normen662
reviewed
Nov 4, 2022
...ore/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/MessageValue.java
Outdated
Show resolved
Hide resolved
normen662
reviewed
Nov 4, 2022
.../java/com/apple/foundationdb/record/query/plan/cascades/values/StreamableAggregateValue.java
Outdated
Show resolved
Hide resolved
normen662
reviewed
Nov 4, 2022
...layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/Value.java
Outdated
Show resolved
Hide resolved
normen662
reviewed
Nov 4, 2022
.../main/java/com/apple/foundationdb/record/query/plan/plans/RecordQueryAggregateIndexPlan.java
Outdated
Show resolved
Hide resolved
normen662
reviewed
Nov 4, 2022
.../main/java/com/apple/foundationdb/record/query/plan/plans/RecordQueryAggregateIndexPlan.java
Outdated
Show resolved
Hide resolved
normen662
reviewed
Nov 4, 2022
.../main/java/com/apple/foundationdb/record/query/plan/plans/RecordQueryAggregateIndexPlan.java
Outdated
Show resolved
Hide resolved
normen662
reviewed
Nov 4, 2022
.../main/java/com/apple/foundationdb/record/query/plan/plans/RecordQueryAggregateIndexPlan.java
Outdated
Show resolved
Hide resolved
Result of fdb-record-layer-pr-proto3 on Linux CentOS 7
|
Result of fdb-record-layer-pr-proto2 on Linux CentOS 7
|
Result of fdb-record-layer-pr-proto2 on Linux CentOS 7
|
Result of fdb-record-layer-pr-proto3 on Linux CentOS 7
|
normen662
previously approved these changes
Nov 7, 2022
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.
Great stuff!
Result of fdb-record-layer-pr-proto2 on Linux CentOS 7
|
Result of fdb-record-layer-pr-proto3 on Linux CentOS 7
|
normen662
approved these changes
Nov 7, 2022
Result of fdb-record-layer-pr-proto2 on Linux CentOS 7
|
SonarCloud Quality Gate failed. |
Result of fdb-record-layer-pr-proto3 on Linux CentOS 7
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This resolves #1885 and adds initial support to planning aggregate indexes (representing the
GroupingKeyExpression
key expression) to Cascades.The support requires a query to have almost identical matching of the key expression.
Implementation notes:
RelationalExpression::subsumedBy
, asubsumedBy
method is added toValue
indicating a subsumption relationship betweenValue
instances, which could be useful as it provides more flexibility in calculatingValue
-compensation.FieldValue
employs a list of integers indicating the ordinal of each field in its field path, this list is used to check whether aFieldValue
subsumes anotherFieldValue
.NumericAggregationValue
such assum
andavg
into their own internal classes so we can annotate them with eitherStreamableAggregateValue
(a value that can be constructed using streaming aggregation) and / orIndexableAggregateValue
(a value that can be backed by an aggregate index). This helps with pattern matching, constructing plans, and choosing the right match candidate. Similar work done toCountValue
.Matcher
s forGroupBy
constituents.Uber-record
is adapted slightly to align it with the addition of field ordinals inFieldValue
. It is just a temporary solution though until we have a proper fix, as explained in Uber-record type for FullUnorderedScanExpression is brittle #1884. Had to adapt a number of now-broken plan hash codes in tests because of this.GroupByExpression
by a match candidate is, for now, exact. This means, for a match to happen, the query must match as-is the match candidate of the respective aggregate index. We will improve on this in the future and make it more robust.