fix(schema-compiler): Use join paths from pre-aggregation declaration instead of building join tree from scratch #9471
Conversation
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
mcheshkov
force-pushed
the
pre-agg-join-path-take-2
branch
from
b38d3b6 to
fd7e96a
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9471 +/- ##
===========================================
- Coverage 80.54% 59.07% -21.47%
===========================================
Files 382 153 -229
Lines 96521 13017 -83504
Branches 2192 2202 +10
===========================================
- Hits 77739 7690 -70049
+ Misses 18471 5014 -13457
- Partials 311 313 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
waralexrom
approved these changes
Apr 22, 2025
mcheshkov
force-pushed
the
pre-agg-join-path-take-2
branch
from
fd7e96a to
9698ca6
Compare
… instead of building join tree from scratch (#9431) Without this pre-aggregatoin declaration would build a list of members, and build join tree from scratch. New join tree could be different, and pre-aggregation would contain incorrect data. Add join hints to member adapters, to build join tree with proper members. This is more of a hack than proper solution: join tree would still disallow to have same cube twice with different join paths. Just adding extra join hints on query level does not work because of FKQA and multiplication check in subqueries: we have to know join path for every member, so it should be either explicit or unambigous _for each member_. Join tree from query is still not included in pre-aggregation matching, so user would have to manually ensure that views and pre-aggregations both match each other and does not produce false matches (cherry picked from commit aea3a6c)
…e join paths tests
…ncy between CubeSymbols and BaseQuery
mcheshkov
force-pushed
the
pre-agg-join-path-take-2
branch
from
9698ca6 to
2bb6df8
Compare
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.
Check List
Description of Changes Made (if issue reference is not provided)
Take 2 for #9431, now with more tests and fixes in pre-aggregations matching
Without this pre-aggregatoin declaration would build a list of members, and build join tree from scratch. New join tree could be different, and pre-aggregation would contain incorrect data.
Add join hints to member adapters, to build join tree with proper members. This is more of a hack than proper solution: join tree would still disallow to have same cube twice with different join paths.
Just adding extra join hints on query level does not work because of FKQA and multiplication check in subqueries: we have to know join path for every member, so it should be either explicit or unambigous for each member.
Join tree from query is still not included in pre-aggregation matching, so user would have to manually ensure that views and pre-aggregations both match each other and does not produce false matches
For now, only pre-aggregation itself respects join paths, but pre-aggregation matching does same as before: strips join path from pre-aggregation references and compares members directly.