kgyrtkirk commented on code in PR #18232:
URL: https://github.com/apache/druid/pull/18232#discussion_r2266616338
##########
sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java:
##########
@@ -3875,7 +3876,7 @@ public void testArrayAggGroupByArrayContainsSubquery()
}
- @NotYetSupported(Modes.UNNEST_INLINED)
+ @NotYetSupported({Modes.UNNEST_INLINED, Modes.DD_UNNEST_INLINED})
Review Comment:
these `DD_UNNEST_INLINED` cases are somewhat unrealistic; in this case the
`unnest` is not really there - as it just runs thru a direct array
with unrealistic I mean
```
SELECT * FROM (select ARRAY[1,2,3] v) t, UNNEST(t.v)
```
would work - which is actually more natural use of `unnest`
never wanted to rewrite these testcases; but might make sense to reject
`unnest(ARRAY<>)` expressions; or flatten it at compilation time...
by putting the `@NYS` on these cases I was just postponing to address
that...
`DD_UNNEST_RESULT_MISMATCH` is on the otherhand is a different kind of
problem; it detects that there are slight differences in semantics sometimes:
`MV_TO_ARRAY` maps muyltiple things to the same value
```sql
SELECT label,l_arr,mv_to_array(mv) from larry
new Object[]{"[1]", "[1]", "[\"1\"]"},
new Object[]{"[2,3]", "[2,3]", "[\"2\",\"3\"]"},
new Object[]{"[]", "[]", null},
new Object[]{"[null]", "[null]", null},
new Object[]{"null", null, null}
```
which makes it impossible to handle it correctly; however the old unnest
handling had some tricks which make it work differently:
* decoupled respects the `mv_to_array` call and retains it/executes it
* meanwhile
[DruidCorrelateUnnestRel#getRexNodeToUnnest](https://github.com/apache/druid/blob/d64a7d4545c01697360140c32eb0d9bb446db5a0/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidCorrelateUnnestRel.java#L533)
explicitly unwraps the `mv_to_array` from the expression...which makes it
look thru those nulls
I believe the decoupled aprroach is the right in this case; but changing the
exisitng engine would be a behavioural change; so it was left as-is for now.
added `testMvToArrayUnnest` to the end of `CalciteArrayQueryTest`
--
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]
