[bugfix] Fix SUM arithmetic rewrite under null handling

[xiangfu0]

Summary

• Change SUM arithmetic rewrites to always use count(column) instead of count(1) so null-handling semantics stay correct without special-casing.
• Restore/maintain AVG/MIN/MAX rewrites (including negative-multiplier flips) and accept the times alias.
• Ensure Calcite parser applies query options before rewrites, preserving null-handling flags.
• Expand AggregationOptimizer tests to cover null-aware SUM semantics, times alias, pre/post operator checks, and parser-driven query shapes.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 63.24%. Comparing base (d03eda3) to head (ddc3147).
⚠️ Report is 8 commits behind head on master.

Files with missing lines	Patch %	Lines
...not/sql/parsers/rewriter/AggregationOptimizer.java	83.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #17338      +/-   ##
============================================
- Coverage     63.28%   63.24%   -0.04%     
  Complexity     1474     1474              
============================================
  Files          3135     3139       +4     
  Lines        186477   187184     +707     
  Branches      28495    28652     +157     
============================================
+ Hits         118006   118390     +384     
- Misses        59357    59635     +278     
- Partials       9114     9159      +45     

Flag	Coverage Δ
custom-integration1	100.00% <ø> (ø)
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-11	63.22% <83.33%> (-0.05%)	⬇️
java-21	63.22% <83.33%> (+<0.01%)	⬆️
temurin	63.24% <83.33%> (-0.04%)	⬇️
unittests	63.24% <83.33%> (-0.04%)	⬇️
unittests1	55.67% <83.33%> (+0.01%)	⬆️
unittests2	33.87% <0.00%> (-0.08%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Jackie-Jiang]

Can we always rewrite it to count(column)? When null handling is not enabled, IIRC this will be processed as count(*)

[Copilot]

Pull request overview

This PR fixes a bug in the AggregationOptimizer where SUM aggregation rewrites were using count(1) instead of count(column), causing incorrect results when null handling is enabled. The fix ensures that when null handling is enabled, the optimizer correctly uses count(column) to preserve semantics for nullable columns.

Key Changes:

• Modified AggregationOptimizer to use count(column) instead of count(1) in SUM rewrites
• Updated all helper methods to pass column expressions to count operations
• Added comprehensive test coverage for null handling scenarios

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
AggregationOptimizer.java	Changed SUM rewrite logic to use count(column) instead of count(1), removed createCountOneExpression() method
AggregationOptimizerTest.java	Added test setup/teardown, new null handling tests, and updated all verification helpers to check for count(column)

[xiangfu0]

Can we always rewrite it to count(column)? When null handling is not enabled, IIRC this will be processed as count(*)

done

[Jackie-Jiang]

Please check if Calcite has rule with similar optimization. We need to ensure the rewrite is semantically identical

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[Copilot]

The test is checking for a lowercase version of the operator name, but other tests in this file use exact string matching without case normalization. This introduces inconsistency. Consider using the exact operator name as returned by the parser or normalizing all operator comparisons consistently throughout the test class.

Suggested change

	assertEquals(multiplicationFunction.getOperator().toLowerCase(), "times");
	assertEquals(multiplicationFunction.getOperator(), "times");