[GitHub] [druid] LakshSingla commented on pull request #11923: Fix incorrect type conversion in DruidLogicalValueRule

[GitBox <gi...@apache.org>]

LakshSingla commented on pull request #11923:
URL: https://github.com/apache/druid/pull/11923#issuecomment-987574208


   Some analysis into the approaches that can be used to fix this:
   1. Cast the output type of the node with the output type of the function, i.e. floor(int) should return a double.
   PROS: 
   (None that I can distinctly think of over the others)
   CONS:
   Could introduce many new corner cases.
   I am of the idea that Calcite already casts the nodes appropriately, i.e. strlen("abc") is of type int and not string (not verified this). Therefore there is no issue with how the literal conversion is happening in DruidRexExecutor
   Is not in line with other SQL dialects as well.
   
   2. Functions or ExprEval can accept an input type, and try to cast the results to that type.
   PROS:
   Might make future errors less likely
   We would be able to cast the values in the corresponding function/expr eval itself rather than relying on the DruidRexExecutor to do the same in future.
   CONS:
   Huge amount of code change
   I am not sure if this is the correct way to go about it, since other rules are working fine without this additional complexity.
   
   3. Improve the extraction in the DruidLogicalValuesRule (The change that is introduced in this PR)
   PROS:
   Localized code change, would be able to identify the cause of the regressions if this causes.
   Since this is the only rule that is giving an error, I think this is where we should solve it.
   CONS:
   Can be dependent on the implementation of the Numeric type of the RexLiteral. (See the top level doc [here](https://calcite.apache.org/javadocAggregate/org/apache/calcite/rex/RexLiteral.html) ).
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org