clintropolis commented on code in PR #15974:
URL: https://github.com/apache/druid/pull/15974#discussion_r1517161120
##########
sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/MultiValueStringOperatorConversions.java:
##########
@@ -458,6 +578,28 @@ boolean isAllowList()
}
}
+
+ private static List<DruidExpression>
harmonizeNullsMvdArg0OperandList(List<DruidExpression> druidExpressions)
+ {
+ final List<DruidExpression> newArgs;
+ if (druidExpressions.get(0).isDirectColumnAccess()) {
Review Comment:
hmm, good question. I guess I was trying to avoid weird stuff happening in
cases where someone was using one of the other MV_ functions which do not
coerce arguments like this like `MV_APPEND` and such. For scalar function
arguments like this, we actually _do_ automatically do this harmonization in
the native layer because we do it whenever we need to 'apply' a function across
an MVD, of which this is a case of that happening.
However, I guess that does create a difference in behavior if there is a mix
segments which have single value strings and multi-value strings, since the
single value strings would not need applied on the native layer... (I need to
double check this actually, its possible we always apply in this case) I hate
mvds so so much :sob:
Side note, I hope we can get the higher order function stuff done, because
it makes me sad everytime i see a query like this when it really should be
`MV_OVERLAP(MAP(x -> LOOKUP(x, 'lookupTable'), mvd), ARRAY['stuff',
'things'])`. Granted, that the native layer will automatically apply this
transformation (and when it maps like this it does automatically do this
harmonization), but i still find it confusing to look at, and if it _was_
written like this, then I think that it would be strange to harmonize the arg0
of `MV_OVERLAP` instead of harmonizing the `mvd` variable, e.g.
`MV_OVERLAP(MAP(x -> LOOKUP(x, 'lookupTable'), MV_HAROMONIZE_NULLS(mvd)),
ARRAY['stuff', 'things'])`.
So... I guess we can apply it all the time, i'll need to update some other
tests, especially some of the tests i have with array columns which are
wrapping the array columns with `ARRAY_TO_MV` and then testing the `MV_`
functions, who do correctly distinguish between `null`, `[]` and `[null]` but
now would no longer at least when using with these functions.
--
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]
