10415: Type match between resultType and function's dataType

[abhioncbr]

This PR will check the type of the functionCall.DataType and _resultType and if it's a mismatch, it will see set the correct one

This fixes #10415.

[codecov-commenter]

Codecov Report

Merging #10472 (3510c14) into master (0c6c54e) will decrease coverage by 1.93%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master   #10472      +/-   ##
============================================
- Coverage     70.21%   68.29%   -1.93%     
+ Complexity     6116     6106      -10     
============================================
  Files          2090     2090              
  Lines        112207   112211       +4     
  Branches      17078    17079       +1     
============================================
- Hits          78784    76629    -2155     
- Misses        27857    30093    +2236     
+ Partials       5566     5489      -77     

Flag	Coverage Δ
integration1	24.65% <0.00%> (+0.24%)	⬆️
integration2	?
unittests1	67.86% <100.00%> (-0.04%)	⬇️
unittests2	13.96% <0.00%> (-0.01%)	⬇️

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

Impacted Files	Coverage Δ
...ery/runtime/operator/operands/FunctionOperand.java	100.00% <100.00%> (ø)

... and 185 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[walterddr]

Pinot's FunctionInvoker largely depending on the next operator's FunctionInvoker parameter type casting
(see:

pinot/pinot-common/src/main/java/org/apache/pinot/common/function/FunctionInvoker.java

Line 115 in d1227e4

arguments[i] = parameterType.convert(argument, argumentType);

)
however we are not guaranteed to have another function invoker chained after.

For this i think we can use functionCall.getDataType() and try to use it to convert _resultType similar to the link above. (it would have performance issues but a good starting point)

please comment on the issue and ask for some specific repo examples so we can add to the test cases

[abhioncbr]

Thanks for the help. I have a couple of questions.

• For conversion(parameterType.convert), do I have to convert the _resultType(fieldSpec.DataType) to PinotDataType and perform the conversion or use the conversion functionFieldSpec.convertInternal directly.
• Second, what should be the outcome of the failing caswhen query? It should fail, or with conversion, it should execute without error.

Thanks.

[walterddr]

from my perspective

Thanks for the help. I have a couple of questions.

• For conversion(parameterType.convert), do I have to convert the _resultType(fieldSpec.DataType) to PinotDataType and perform the conversion or use the conversion functionFieldSpec.convertInternal directly.

Either should work. but would go with the safest, which is what FunctionInvoker handles parameter type

• Second, what should be the outcome of the failing caswhen query? It should fail, or with conversion, it should execute without error.

it should not fail at all at this stage --> calcite should ensure the resulting data type is consistent. otherwise it will fail in planner before reaching runtime. If the data is not what it said it is (from input) then the failure handling is the same as all other operators

[abhioncbr]

@walterddr Can you please take a look? Thanks

[walterddr]

thank you will do. i also asked for more examples from @ankitsultana in the original issue. let's wait for a bit and add more test cases

[ankitsultana]

I think it might be worthwhile to add a couple or so queries which do comparisons of double/float columns with double/float literals, and likewise for int/long columns.

Stuff like:

case longCol > 10
...
case doubleCol < 1.3
case floatCol < 10
..
where longCol < 10 and doubleCol < 1.3 and floatCol < 10...

..
with tmp as (select sum(floatCol) as float_sum from ... group by col1) select * from tmp where float_sum < 10

[abhioncbr]

I added a couple of test cases, as suggested. Please let me know if I have to add more.

[walterddr]

looking