Skip to content

Fix REGR_COUNT arguments being dropped during SqlToRel conversion#4990

Open
chloe-zh wants to merge 1 commit into
apache:mainfrom
chloe-zh:main
Open

Fix REGR_COUNT arguments being dropped during SqlToRel conversion#4990
chloe-zh wants to merge 1 commit into
apache:mainfrom
chloe-zh:main

Conversation

@chloe-zh
Copy link
Copy Markdown

@chloe-zh chloe-zh commented Jun 4, 2026

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.

Jira Link

https://issues.apache.org/jira/browse/CALCITE-7579

Problem

When parses REGR_COUNT(y, x) the SqlKind is mistakenly set to SqlKind.COUNT instead of SqlKind.REGR_COUNT, the agg builder then drop the args y and x and 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:

REGR_COUNT(y, x) -> REGR_COUNT(*)

which is semantically incorrect

Changes Proposed

  • Correct the SqlKind to REGR_COUNT
  • At build agg call, check SqlKind instead of instanceof thus it always focuses on real count and bypass regr_count
  • Remove regr_count from AggregateReduceFunctionsRule given it is irreducible.

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.
@mihaibudiu
Copy link
Copy Markdown
Contributor

Before submitting a PR please file an issue using https://issues.apache.org/jira

@chloe-zh
Copy link
Copy Markdown
Author

chloe-zh commented Jun 4, 2026

don't have account yet, just signed it up, waiting for update from the apache maintainer @mihaibudiu

@xiedeyantu
Copy link
Copy Markdown
Member

I have approved your Jira account application. You can now create a Jira and assign it to yourself.

@chloe-zh
Copy link
Copy Markdown
Author

chloe-zh commented Jun 4, 2026

thanks team, updated

@xiedeyantu
Copy link
Copy Markdown
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".

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 4, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants