Fix REGR_COUNT arguments being dropped during SqlToRel conversion#4990
Open
chloe-zh wants to merge 1 commit into
Open
Fix REGR_COUNT arguments being dropped during SqlToRel conversion#4990chloe-zh wants to merge 1 commit into
chloe-zh wants to merge 1 commit into
Conversation
REGR_COUNT(y, x) was incorrectly converted to REGR_COUNT(*) because SqlRegrCountAggFunction extends SqlCountAggFunction, which: 1. Hardcodes SqlKind.COUNT in its constructor, causing REGR_COUNT to report the wrong SqlKind. 2. Causes RexBuilder.addAggCall() to match REGR_COUNT via SqlKind.COUNT and strip non-nullable arguments (an optimization valid only for COUNT). 3. Masked a missing case handler in AggregateReduceFunctionsRule, since SqlKind.COUNT is not in functionsToReduce. Fix: - Add a protected constructor to SqlCountAggFunction that accepts SqlKind, allowing subclasses to specify their own kind. - Update SqlRegrCountAggFunction to pass SqlKind.REGR_COUNT. - Guard the nullable-args optimization in RexBuilder.addAggCall() to check SqlKind == COUNT instead of instanceof. - Add case REGR_COUNT in AggregateReduceFunctionsRule to preserve it as-is, since it is irreducible.
Contributor
|
Before submitting a PR please file an issue using https://issues.apache.org/jira |
Author
|
don't have account yet, just signed it up, waiting for update from the apache maintainer @mihaibudiu |
Member
|
I have approved your Jira account application. You can now create a Jira and assign it to yourself. |
Author
|
thanks team, updated |
Member
|
The following three items(Jira Title, PR Title, Commit Message) MUST match exactly in wording and meaning, the title cannot contain the word "fix". |
|
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.



REGR_COUNT(y, x) was incorrectly converted to REGR_COUNT(*) because SqlRegrCountAggFunction extends SqlCountAggFunction, which:
Fix:
Jira Link
https://issues.apache.org/jira/browse/CALCITE-7579
Problem
When parses
REGR_COUNT(y, x)the SqlKind is mistakenly set toSqlKind.COUNTinstead ofSqlKind.REGR_COUNT, the agg builder then drop the argsyandxand replace with*, leading to empty arg list of the agg call at RelNode stage.A way to repro it is, parse and plan it, then convert it back to SQL, it would be:
which is semantically incorrect
Changes Proposed
instanceofthus it always focuses on real count and bypass regr_countAggregateReduceFunctionsRulegiven it is irreducible.