Use RexList to carry aggregation pre arguments for intermediate stage

[xiangfu0]

This PR re-implement #11105, which use hints to carry literals due to the calcite limitation.
Since the recent upgrade of Calcite library, we can use rexList((https://issues.apache.org/jira/browse/CALCITE-4334 and apache/calcite@ddb4200)) to carry original aggregation functions arguments for literals and intermediate stage with placeholder.

• Use RexList to carry all aggregation arguments for literals and setting intermediate stage placeholders
• Removed existing hack of setting literals in v2 functions

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 90.78947% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 62.15%. Comparing base (59551e4) to head (9f35387).
Report is 499 commits behind head on master.

Files	Patch %	Lines
...inot/query/runtime/operator/AggregateOperator.java	78.78%	4 Missing and 3 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #13217      +/-   ##
============================================
+ Coverage     61.75%   62.15%   +0.40%     
+ Complexity      207      198       -9     
============================================
  Files          2436     2532      +96     
  Lines        133233   138976    +5743     
  Branches      20636    21531     +895     
============================================
+ Hits          82274    86378    +4104     
- Misses        44911    46141    +1230     
- Partials       6048     6457     +409     

Flag	Coverage Δ
custom-integration1	<0.01% <0.00%> (-0.01%)	⬇️
integration	<0.01% <0.00%> (-0.01%)	⬇️
integration1	<0.01% <0.00%> (-0.01%)	⬇️
integration2	0.00% <0.00%> (ø)
java-11	62.10% <90.78%> (+0.39%)	⬆️
java-21	62.03% <90.78%> (+0.41%)	⬆️
skip-bytebuffers-false	62.13% <90.78%> (+0.38%)	⬆️
skip-bytebuffers-true	62.02% <90.78%> (+34.29%)	⬆️
temurin	62.15% <90.78%> (+0.40%)	⬆️
unittests	62.14% <90.78%> (+0.40%)	⬆️
unittests1	46.67% <90.78%> (-0.23%)	⬇️
unittests2	27.82% <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.

[Jackie-Jiang]

Why do we need this? Are you trying to get boolean out of NULL? This won't be accurate because NULL is not equivalent to boolean false

[xiangfu0]

This issue is that boolean Literal is converted to 1 or 0 value over wire.
Ref: https://github.com/apache/pinot/blob/master/pinot-query-planner/src/main/java/org/apache/pinot/query/planner/logical/RexExpressionUtils.java#L70

[xiangfu0]

for null, it anyway won't work with type boolean.

[xiangfu0]

Changed to only handle the Integer conversion.

[Jackie-Jiang]

The bug is on the RexExpression conversion side, where boolean value is not properly preserved. We should change RexExpressionUtils to keep the value type aligned with the data type. Also, if I read it correctly, TIMESTAMP is also not properly handled as it is not supported within LiteralContext yet

[Jackie-Jiang]

Trying to understand it better, why can't we reuse _functionOperands here for literals?

[xiangfu0]

correct, it will break the query plan, e.g. intermediate stage result reference.
So need to apply _functionPreArgs to _functionOperands during the query execution phase.

[Jackie-Jiang]

Since it won't be backward compatible anyway, can we directly use this as operands? I think that is the general way to handle it, where we already support reading literal from operands

[xiangfu0]

This won't work for intermediate stage, which expects only one argument in the operand list with InputRef to the type of an intermediate object.

[Jackie-Jiang]

Should we use same way to process raw input and intermediate input since we already have all operands?
Right you we assume we don't need function for raw input which is not always true

[xiangfu0]

The only difference is that first argument type in intermediate stage function operands might be different than raw input stage.

For count(*), no argument in rawInput stage, but you need one InputRef in the intermediate stage.

[xiangfu0]

Close this since it's already implemented in #13282