fix(tesseract): Emit GROUP BY for non-windowed Aggregate stages in ungrouped queries#11328
Open
esrever10 wants to merge 1 commit into
Open
Conversation
…grouped queries An ungrouped SQL API query (bare SELECT without GROUP BY, as BI tools send when browsing a table) selecting a multi-stage measure with a group_by/reduce_by lock made the query-level ungrouped flag suppress the GROUP BY of the measure's Aggregate stage, while the stage still projected plain aggregate functions next to bare dimension columns. Databases enforcing SQL grouping rules reject the statement: ERROR: column "fk_aggregate_keys.orders__city" must appear in the GROUP BY clause or be used in an aggregate function A non-windowed Aggregate stage always renders plain aggregates, so it must be grouped by its projected dimensions regardless of the query's ungrouped flag; the enclosing FullKeyAggregate join broadcasts the stage output back to detail rows, preserving ungrouped semantics. Rank / Calculate / windowed stages are untouched — they render window functions or plain expressions and are valid without GROUP BY. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
the
pr:community
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Issue Reference this PR resolves
Fixes #11327
Description of Changes Made
An ungrouped SQL API query (bare
SELECTwithoutGROUP BY, the shape BI tools emit when browsing a table) selecting agroup_by/reduce_by-locked multi-stage measure plus any dimension produced invalid SQL: the measure's Aggregate stage projected plainsum(...)next to bare dimension columns with noGROUP BY, so databases enforcing SQL grouping rules (e.g. Postgres) rejected the statement. Full trace in #11327.The fix follows from what the stage renders: a non-windowed Aggregate stage projects plain aggregate functions by construction, so its select must be grouped by its projected dimensions regardless of the query-level
ungroupedflag. Ungrouped semantics are unaffected — the enclosingFullKeyAggregatejoin broadcasts the stage output back to detail rows. Rank / Calculate / windowed stages are untouched: they render window functions or plain expressions and are valid withoutGROUP BY.ORDER BYbehavior is unchanged.Changes:
physical_plan_builder/processors/multi_stage_measure_calculation.rs: emitGROUP BYwhen the stage is grouped or renders plain aggregates (calculation_type == Aggregate && window_function_to_use == None).test_fixtures/.../integration_multi_stage.yaml: addunique_customers_group_by_status— agroup_by-lockedsumovercount_distinct, i.e. a non-sum-rollable input that is not window-path eligible and therefore hits the plain-aggregate JOIN model (existing ungrouped tests only covered Calculate-type / window-path measures, which are valid withoutGROUP BY).tests/integration/multi_stage/ungrouped.rs: regression test — fails on master (assert sql.contains("GROUP BY"); the whole statement previously had none), passes with the fix; also executes against Postgres under theintegration-postgresfeature.Verification:
cargo test -p cubesqlplanner— 1076 passed, 0 failed (new test red on master, green with the fix).