Unnecessary cartesian explosion if multi-value column is reused in expression

[vogievetsky]

Affected Version

Druid 0.16, 0.15

Description

With this dataset (as fs):

{"time":"2019-08-14T00:00:00.000Z","srcGroups":["x","y","z"],"dstGroups":["a","b","c","d"]}
{"time":"2019-08-14T00:00:00.000Z","srcGroups":["x","y","z"],"dstGroups":["a","c","d"]}
{"time":"2019-08-14T00:00:00.000Z","srcGroups":["x","y","z"],"dstGroups":["a","g"]}

Doing this query:

SELECT
  CASE "dstGroups"
    WHEN 'b' THEN 'b'
    WHEN 'g' THEN 'g'
    ELSE 'Other'
  END AS "dst",
  COUNT(*) AS "Count"
FROM "fs"
GROUP BY 1
ORDER BY "Count" DESC

Yields an unexpected cartesian explosion:

This is due to this expression in the underlying plan:

"case_searched((\"dstGroups\" == 'b'),'b',(\"dstGroups\" == 'g'),'g','Other')"

Which triggers a cartesian product on the same column dstGroups which it should not do.

[clintropolis]

This is my bad, it is treating conditional functions the same as something like concat(x, x) which is not correct.

[adilev26]

so this should yield a result of [other, 1], [b, 1], [g, 1] ?

[adilev26]

also, what will happen if we add a case at the top "WHEN 'a' THEN 'a'" ?
will we just get [a, 3]?

[clintropolis]

#8019 is the PR that changed the behavior to treat repeated uses of the same multi-value input as cartesian_map rather than map, though I can't find reason for why we decided to make this change. Based on this behavior definitely not making sense for conditional functions, it's probably worth revisiting and having a discussion if the map behavior was better, as it's relatively easy with just a few lines of code to change back (though a fair bit of tests to fix up).

[vogievetsky]

I believe that the logical behavior would be for an expression to be a cartesian map over all unique multi-value dimensions present in it. Hence "case_searched((\"dstGroups\" == 'b'),'b',(\"dstGroups\" == 'g'),'g','Other')" and concat(x, x) would work the same way by virtue of the fact that they both only refer to one variable.

[clintropolis]

I believe that the logical behavior would be for an expression to be a cartesian map over all unique multi-value dimensions present in it.

The behavior you suggest seems fine to me, your example presents a good case for why it should be like that, and as previously mentioned, this was the behavior until #8019 (still drawing a blank as to why we decided to make the change earlier). I have opened #8957 which essentially reverts this behavior (without removing the improvements of that PR), should we all agree the old behavior is more useful.

[clintropolis]

so this should yield a result of [other, 1], [b, 1], [g, 1] ?

With the adjusted behavior in #8957, the expression would be translated into something like

map(dstGroups -> case_searched((dstGroups == 'b'),'b',(dstGroups == 'g'),'g','Other'), dstGroups)

which would yield the per row results:

[other, b, other, other]
[other, other, other]
[other, g]

Druid does an implicit "un-nest" when grouping on a multi-value string dimension, so the final results would be I think:

other, 7
b, 1
g, 1

if I can add correctly