[Enhancement] improve sql digest for massive compound predicates #53207
+128
−67
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
![starrocks-cr[bot]](https://avatars.githubusercontent.com/in/404334?s=60&v=4){kind=small}
satanson
reviewed
Nov 26, 2024
| exprs.add((SlotRef) x); | ||
| } | ||
| }); | ||
| return "$massive_compounds[" + exprs.stream().map(SlotRef::toSqlImpl) |
There was a problem hiding this comment.
SlotRefs are query-specific so unstable, suggest to use original SlotRefs(slotref references to table column) and convert original slotrefs into table columns then sort these columns lexicographically.
LiShuMing
reviewed
Nov 26, 2024
| if (pair.first + pair.second >= MASSIVE_COMPOUND_LIMIT) { | ||
| // Only record de-duplicated slots if there are too many compounds | ||
| Set<SlotRef> exprs = Sets.newHashSet(); | ||
| traverse(node, x -> { |
There was a problem hiding this comment.
why not use node.collectAllSlotRefs() directly?
There was a problem hiding this comment.
well, I should use it. but it seems I should improve the performance of that method first.
There was a problem hiding this comment.
it constructs a lot of intermediate list,, i'm afraid the performance can be terrible
| @Override | ||
| public String visitCompoundPredicate(CompoundPredicate node, Void context) { | ||
| Pair<Integer, Integer> pair = countCompound(node); | ||
| if (pair.first + pair.second >= MASSIVE_COMPOUND_LIMIT) { |
There was a problem hiding this comment.
How about Expr#flattenPredicate;
LiShuMing
approved these changes
Nov 26, 2024
murphyatwork
force-pushed
the
murphy_opt_massive_or_predicate_digest
branch
from
14527f8
to
7dbd265
Compare
Signed-off-by: Murphy <[email protected]>
Signed-off-by: Murphy <[email protected]>
murphyatwork
force-pushed
the
murphy_opt_massive_or_predicate_digest
branch
from
7dbd265
to
8693c26
Compare
|
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]✅ pass : 53 / 55 (96.36%) file detail
|
[BE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
LiShuMing
approved these changes
Nov 28, 2024
Seaven
approved these changes
Nov 28, 2024
|
@Mergifyio backport branch-3.4 |
|
@Mergifyio backport branch-3.3 |
|
@Mergifyio backport branch-3.2 |
✅ Backports have been created
|
✅ Backports have been created
|
✅ Backports have been created
|
mergify bot
pushed a commit
that referenced
this pull request
Nov 28, 2024
) Signed-off-by: Murphy <[email protected]> (cherry picked from commit c411ac5)
42 tasks
mergify bot
pushed a commit
that referenced
this pull request
Nov 28, 2024
) Signed-off-by: Murphy <[email protected]> (cherry picked from commit c411ac5) # Conflicts: # fe/fe-core/src/main/java/com/starrocks/analysis/Expr.java # fe/fe-core/src/main/java/com/starrocks/sql/optimizer/transformer/RelationTransformer.java
42 tasks
mergify bot
pushed a commit
that referenced
this pull request
Nov 28, 2024
) Signed-off-by: Murphy <[email protected]> (cherry picked from commit c411ac5) # Conflicts: # fe/fe-core/src/main/java/com/starrocks/analysis/Expr.java # fe/fe-core/src/main/java/com/starrocks/sql/analyzer/AnalyzerUtils.java # fe/fe-core/src/main/java/com/starrocks/sql/common/SqlDigestBuilder.java # fe/fe-core/src/main/java/com/starrocks/sql/optimizer/transformer/RelationTransformer.java # fe/fe-core/src/test/java/com/starrocks/sql/common/SqlDigestBuilderTest.java
42 tasks
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.
Why I'm doing:
What I'm doing:
In cases where an OR predicate is dynamically constructed with a fixed column, the SQL digest varies due to a differing number of predicates. To address this, we consolidate extensive compound predicates into a compact format, ensuring consistent SQL digests.
Fixes #issue
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: