[CALCITE-7669] Uncollect should support the Trino semantics of UNNEST - #5118
[CALCITE-7669] Uncollect should support the Trino semantics of UNNEST#5118mihaibudiu wants to merge 2 commits into
Conversation
Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
|
xuzifu666
left a comment
xuzifu666
left a comment
There was a problem hiding this comment.
I think the overall implementation is fine; I've left some comments for reference.
| ? typeFactory.enforceTypeWithNullability(componentType, true) | ||
| RelDataType elementType = componentType.isStruct() | ||
| ? typeFactory.builder().kind(componentType.getStructKind()) | ||
| .addAll(componentType.getFieldList()).build() |
There was a problem hiding this comment.
The return type here is NOT NULL, so the isNullable mentioned above(line 101) isn't actually used in this branch; it might be worth verifying whether this is appropriate.
In Trino, the output columns of UNNEST(array of nullable row) are nullable.
| @SuppressWarnings({"rawtypes", "unchecked"}) | ||
| private static final Function1<List<Object>, Enumerable<Object>> STRUCT_LIST_AS_ENUMERABLE = | ||
| a0 -> a0 == null ? Linq4j.emptyEnumerable() | ||
| : Linq4j.asEnumerable(a0).select(e -> (Object) ((List) e).toArray()); |
There was a problem hiding this comment.
Arrays containing NULL elements, such as UNNEST(ARRAY[ROW(1,'x'), CAST(NULL AS ROW(...))]), trigger a direct NPE (the SCALAR path avoids this issue as it performs no conversion).
Trino currently produces a row of NULLs for this case.
It is recommended to change the logic to e == null ? null : ... and add a test case that includes NULL elements.
There was a problem hiding this comment.
An attempt to fix this bug has revealed some independent problems in RelToLixTranslator, which need to be fixed fix. I will file a separate issue and mark this as draft until the other one is fixed.
2 participants
Jira Link
CALCITE-7669
Changes Proposed
There are two separate commits, if independent interest. The first commit only strengthens the validator for rejecting UNNEST on the right side of some JOIN operators.
The semantics of the UNNEST operator changes depending on conformance: for Presto/Trino UNNEST(a) where a is a ROW ARRAY produces a single column; for other conformances it produces as many columns as there are fields in ROW.
This PR makes the choice of semantics explicit at all levels of the representation: the Uncollect operator, and the Enumerable representation by adding a new boolean flag to control the behavior. The flag clearly affects type inference too.
This PR is a stand-alone fragment extracted from #5031