[GitHub] [druid] gianm edited a comment on pull request #11327: fix a bugs related to SQL type inference return type nullability

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

gianm edited a comment on pull request #11327:
URL: https://github.com/apache/druid/pull/11327#issuecomment-860951542


   > Generally LGTM. Could you take a look at ConcatOperatorConversion too? I think it should also be "cascade nullable".
   
   Btw, surveying what else is out there:
   
   - PostgreSQL CONCAT ignores nulls, and `||` returns null if any argument is null: https://www.postgresql.org/docs/13/functions-string.html, http://sqlfiddle.com/#!17/5821c5/2
   - MySQL CONCAT returns null if any argument is null, and doesn't seem to support the `||` operator: https://dev.mysql.com/doc/refman/8.0/en/string-functions.html#function_concat, http://sqlfiddle.com/#!9/180f8e/1
   - BigQuery CONCAT returns null if any argument is null; it mentions support for `||` but doesn't say how it will handle nulls: https://cloud.google.com/bigquery/docs/reference/standard-sql/string_functions
   
   Today, our ConcatFunc is implemented to return null if any argument is null, and we use that ConcatFunc for both CONCAT and `||`. I think in light of what other DBs do, this is fine, so I'd just suggest updating the ConcatOperatorConversion and the docs appropriately.


-- 
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



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