Druid: COALESCE does not work with empty strings as documentation states it would

[sascha-coenen]

Affected Version

0.16.0 and 0.17.0

Description

COALESCE does NOT work with empty strings

   COALESCE('something', 'fallback') AS tst1,   
   COALESCE(null, 'fallback') AS tst2,
   COALESCE('', 'fallback') AS tst3

this yields
"something", "fallback", ""

According to the documentation, the third projection should also yield the output "fallback"

COALESCE(value1, value2, ...) | Returns the first value that is neither NULL nor empty string.

I would petition to keep the documentation as is and modify the function behaviour to be conformant to the documentation as it would be quite useful if COALESCE could also deal with empty strings.

[gianm]

In default null-handling mode (https://druid.apache.org/docs/latest/querying/sql.html#null-handling-modes) there are some inconsistencies with nulls vs empty string handling that we are unable to repair while maintaining SQL compliance.

I think probably the most likely thing we'll do here is change the docs to be SQL compliant, i.e. "returns the first non-NULL value". I suppose if you wanted to treat nulls and empty strings equivalently you could do COALESCE(NULLIF(expr1, ''), NULLIF(expr2, ''), ...) or something like that.

[sascha-coenen]

would it be possible to change the behaviour of LOOKUPS such that a key miss evaluates to NULL rather than an empty string? In that case we would not have a need for dealing with empty strings. Formulating fallbacks for lookups is our only usecase in which we need to check for empty strings.

[gianm]

would it be possible to change the behaviour of LOOKUPS such that a key miss evaluates to NULL rather than an empty string?

That's a good point and seems pretty reasonable. I think it's actually already like that in SQL-compliant null handling mode. So maybe in that mode things already work like you expect. It would be good to exercise this case in any unit tests we add while working on this issue.