[GitHub] [incubator-druid] gianm commented on issue #7281: bump calcite version to 1.18.0 in druid-sql

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

gianm commented on issue #7281: bump calcite version to 1.18.0 in druid-sql
URL: https://github.com/apache/incubator-druid/pull/7281#issuecomment-484743556
 
 
   > It looks Calcite released 1.19.0 recently.. @gianm Do you think which way is make more sense, updating this pr to upgrade Calcite to 1.19.0 or first to 1.18.0 and then 1.19.0. Both version met the same issue discussed above..thanks
   
   IMO, updating this PR to go straight to 1.19.0 is best.
   
   > One way might be that modify DruidRexExecutor to keep the same policy as calcite as following (adding strong.isNull check)
   > ...
   > But this way will reduce =(null, null) and =(null, '')(useDefaultValueForNull) as false, so the test testEmptyStringEquality's result will be affected too and =(null, null) as false is a little counter-intuitive
   > What do you think, thanks!
   
   Hmm. I think in Druid's default mode (`NullHandling.replaceWithDefault()` true) then treating VARCHAR nulls as empty strings is generally the right thing to do. In SQL-compatible mode it's best to be SQL compatible of course. If there is some way we can accomplish that, it seems ideal.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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