-
Notifications
You must be signed in to change notification settings - Fork 102
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
resolves #1671: match index hints in a query #1638
Conversation
pengpeng-lu
commented
May 3, 2022
•
edited
edited
- This PR is to support index hint in a query
- Add index info to FullUnorderedScanExpression, then when match candidates, the query can be subsumed by a candidate only when the index is included in the hint indexes.
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
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.
Hi,
the major code path is exactly as discussed. Nice! Please do work on these two bullets:
- setting the name of the index into the fuse of the
MatchCandidate
s when they get created. That name is available but needs to be threaded into the right place(s). - test cases! Please add (at least) a query test case that demonstrates the hinting mechanism, i.e. even though a better index is available the worse one wins because it is hinted. Look into
FDBSimpleQueryGraphTest
for inspiration on how to do that.
@@ -325,7 +326,9 @@ static Optional<MatchCandidate> fromPrimaryDefinition(@Nonnull final RecordMetaD | |||
@Nonnull | |||
static GroupExpressionRef<RelationalExpression> createBaseRef(@Nonnull RecordMetaData metaData, @Nonnull final Set<String> allAvailableRecordTypes, @Nonnull final Set<String> recordTypesForIndex) { | |||
final var quantifier = | |||
Quantifier.forEach(GroupExpressionRef.of(new FullUnorderedScanExpression(allAvailableRecordTypes))); | |||
Quantifier.forEach(GroupExpressionRef.of(new FullUnorderedScanExpression(allAvailableRecordTypes, metaData.getAllIndexes().stream() |
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.
This should be a singleton set containing just the name of the index this particular MatchCandidate
represents.
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.
You will need to pass the name of the index this expansion is for to the expansion visitors that create the MatchCandidate
. That name should be passed in here.
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.
I'm a little confused here: we can getRecordTypes
from both MetaDataPlanContext
and RecordMetaData
, and the result seems a bit different for these two. Which one should we use in new FullUnorderedScanExpression
? Also we can getIndexNames
for both, which one should we use?
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.
The ones in MetaDataPlanContext
are derived from the query being planned. So, the record types being queried ("FROM
clause") and the allowed indexes ("INDEX
hints"). The ones in RecordMetaData
are the ones in the schema.
...re/src/main/java/com/apple/foundationdb/record/query/plan/cascades/RelationalExpression.java
Outdated
Show resolved
Hide resolved
...m/apple/foundationdb/record/query/plan/cascades/expressions/FullUnorderedScanExpression.java
Outdated
Show resolved
Hide resolved
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
...yer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/MatchCandidate.java
Outdated
Show resolved
Hide resolved
.../java/com/apple/foundationdb/record/provider/foundationdb/query/FDBSimpleQueryGraphTest.java
Outdated
Show resolved
Hide resolved
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
.../java/com/apple/foundationdb/record/provider/foundationdb/query/FDBSimpleQueryGraphTest.java
Outdated
Show resolved
Hide resolved
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
...d-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/AccessHint.java
Outdated
Show resolved
Hide resolved
...-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/AccessHints.java
Show resolved
Hide resolved
...-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/PrimaryAccessHint.java
Outdated
Show resolved
Hide resolved
...m/apple/foundationdb/record/query/plan/cascades/expressions/FullUnorderedScanExpression.java
Outdated
Show resolved
Hide resolved
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
...-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/AccessHints.java
Show resolved
Hide resolved
...-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/PrimaryAccessHint.java
Outdated
Show resolved
Hide resolved
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|
SonarCloud Quality Gate failed. |
AWS CodeBuild CI Report for Linux CentOS 7
|
AWS CodeBuild CI Report for Linux CentOS 7
|