branch-4.1: [fix](aggregate) Normalize projected count slots before null safety checks #67732 - #67767
Closed
github-actions[bot] wants to merge 1 commit into
Closed
branch-4.1: [fix](aggregate) Normalize projected count slots before null safety checks #67732#67767github-actions[bot] wants to merge 1 commit into
github-actions[bot] wants to merge 1 commit into
Conversation
…hecks (#67732) ## Problem Counting a projected alias over a nullable indexed column can return an incorrect non-zero result when the filter retains only null rows. Mixing the alias count with `COUNT(*)` or another count exposes the problem: ```sql SELECT COUNT(x), COUNT(*) FROM (SELECT k AS x FROM t WHERE k IS NULL) q; ``` For two matching null rows, the correct result is `(0, 2)`, but the storage-layer index-count path can return `(2, 2)`. ## Root cause The FE implementation rule validates `IS NULL` and OR predicates before pushing count aggregation to the storage layer. In the Project variant, this validation used the aggregate-side alias slot, while the filter below the Project refers to the source slot. Their expression IDs differ, so the null-safety guard did not recognize that the filter and `COUNT` referenced the same nullable value. The rule normalized the aggregate argument to the source slot only later, after the safety decision had already been made. ## Reproduction ```sql CREATE TABLE t ( id INT NOT NULL, k INT NULL, INDEX idx_k (k) USING INVERTED ) DUPLICATE KEY(id) DISTRIBUTED BY HASH(id) BUCKETS 1 PROPERTIES ("replication_num" = "1"); INSERT INTO t VALUES (1, NULL), (2, NULL), (3, 1); SELECT COUNT(x), COUNT(*) FROM (SELECT k AS x FROM t WHERE k IS NULL) q; SELECT COUNT(x), COUNT(id) FROM (SELECT k AS x, id FROM t WHERE k IS NULL) q; ``` Before this change, both queries return `(2, 2)` and the plan contains `pushAggOp=COUNT_ON_INDEX`. Both queries should return `(0, 2)`. ## Fix Normalize aggregate arguments through the Project before collecting the slots used by the predicate safety checks. The count slots and filter slots are now compared in the same source expression-ID domain. If `IS NULL` targets a counted source slot, the FE rejects the index-count pushdown and preserves the column's null values. This change is limited to the FE planner. ## Tests - Added a FE plan test for `COUNT(projected_alias) + COUNT(*)` above an `IS NULL` filter, verifying that the count-on-index implementation rule is rejected. - Ran `PhysicalStorageLayerAggregateTest`: 7 tests passed. - Deployed the FE to a local sandbox and reran both SQL reproductions. They return `(0, 2)`, and the scan plan reports `pushAggOp=NONE`.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
|
run buildall |
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
2 participants
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.
Cherry-picked from #67732