sql: preserve filters during distinct preprocessing - #174721
Open
Alignyx wants to merge 1 commit into
Open
Conversation
When every aggregate is distinct, physical planning adds an early distinct stage keyed by the union of aggregate argument columns. For filtered aggregates, this can discard rows that have equal arguments but different filter results before each aggregate applies its own filter. Include every aggregate filter column in the early distinct key. Add direct and prepared SQL regressions with multiple filtered distinct aggregates. Fixes cockroachdb#158770 Release note (bug fix): Fixed incorrect results from multiple filtered DISTINCT aggregates over the same argument.
|
Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
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
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.
Fixes #158770.
Root cause
When every aggregate in a stage is
DISTINCT, physical planning adds an early distinct processor keyed by the union of the aggregate argument columns. Filter columns were not included in that key. Rows with the same aggregate argument but different filter results could therefore be collapsed before each aggregate evaluated its own filter, causing multiple filtered DISTINCT aggregates over the same argument to observe the wrong subset of rows. Placeholders made the issue visible by preventing optimizer deduplication, but they were not the source of the error; two different literal filters failed in the same way.How I tracked it down
I compared the reported prepared query with direct literal filters, reordered aggregates, a single filtered DISTINCT aggregate, non-DISTINCT controls, identical filters, different aggregate arguments, and
COUNTplusSUMover the same DISTINCT argument. Row/vectorized and local/forced-distributed execution agreed on the failure, which placed it before either aggregation implementation. The physical plan contained both aggregate specs and both Boolean filter projections, but the shared all-DISTINCT preprocessing stage deduplicated only on the argument column. For a repeated argument, that stage could retain a false-filter row and discard the true-filter row before either aggregate ran.Fix
The early distinct key now includes every non-nil aggregate filter column in addition to the existing union of argument columns. This preserves rows that are distinguishable by any later filter while retaining the shared preprocessing optimization and leaving individual aggregate implementations unchanged.
Test coverage
2,1.2,2) and different thresholds (2,1).