Skip to content

[PoC] [BugFix] Fix BABEL isNonStrictGroupBy causing ANY_VALUE wrapping and NPE#15

Draft
dai-chen wants to merge 1 commit into
mainfrom
poc/babel-strict-groupby
Draft

[PoC] [BugFix] Fix BABEL isNonStrictGroupBy causing ANY_VALUE wrapping and NPE#15
dai-chen wants to merge 1 commit into
mainfrom
poc/babel-strict-groupby

Conversation

@dai-chen
Copy link
Copy Markdown
Owner

@dai-chen dai-chen commented May 5, 2026

Description

Replace Frameworks.getPlanner() with a custom validate+convert pipeline that uses BABEL conformance for parsing but disables isNonStrictGroupBy for validation. This fixes two ClickBench query failures:

  • q35: GROUP BY ordinal no longer wraps literals in ANY_VALUE
  • q40: CASE expressions in GROUP BY no longer trigger NPE in AggFinder

Also unifies the SQL path with the PPL path by having CalciteNativeStrategy consume pre-parsed SqlNode from CalciteSqlQueryParser (reusing the shared UnifiedQueryParser interface) instead of duplicating parsing via PlannerImpl.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • New functionality has javadoc added.
  • New functionality has a user manual doc added.
  • New PPL command checklist all confirmed.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff or -s.
  • Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Replace Frameworks.getPlanner() with a custom validate+convert pipeline
that uses BABEL conformance for parsing but disables isNonStrictGroupBy
for validation. This fixes two ClickBench query failures:

- q35: GROUP BY ordinal no longer wraps literals in ANY_VALUE
- q40: CASE expressions in GROUP BY no longer trigger NPE in AggFinder

Also unifies the SQL path with the PPL path by having CalciteNativeStrategy
consume pre-parsed SqlNode from CalciteSqlQueryParser (reusing the shared
UnifiedQueryParser interface) instead of duplicating parsing via PlannerImpl.

Signed-off-by: Chen Dai <daichen@amazon.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Chen Dai <daichen@amazon.com>
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.

1 participant