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


   Thanks everyone for the review. 
   I think there's no functional difference in b/w `RexLiteral.value(...)`, 
`(BigDecimal) literal.getValue(...)` and 
`literal.getValueAs(BigDecimal.class)`, so unless the internal specification of 
Calcite changes, we should be good to go with any of the three mentioned above. 
To keep uniformity with the codebase, I have converted all thecases to instance 
of `RexLiteral.value(...), RexLiteral.stringValue(...)`.
   Apart from that, I have also returned a `null` if `RexLiteral.value(...)` 
returns `null`, since in the original version of the code that was implicitly 
handled by `Literal.getValueAs(Long.class)`, whereas now we also have to cast 
and then extract using a method. 
   That was causing `testGroupByNothingWithLiterallyFalseFilter` to fail in 
`CalciteQueryTest`


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to