xuzifu666 commented on code in PR #5118:
URL: https://github.com/apache/calcite/pull/5118#discussion_r3655646956
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -209,36 +208,28 @@ public class SqlFunctions {
private static final Function1<List<Object>, Enumerable<Object>>
LIST_AS_ENUMERABLE =
a0 -> a0 == null ? Linq4j.emptyEnumerable() : Linq4j.asEnumerable(a0);
+ /** Like {@link #LIST_AS_ENUMERABLE}, for a collection whose struct elements
+ * are kept whole: each element is converted from the collection's internal
+ * list representation to an Object[] struct value. */
+ @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());
Review 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.
##########
core/src/main/java/org/apache/calcite/sql/SqlUnnestOperator.java:
##########
@@ -108,9 +111,13 @@ public SqlUnnestOperator(boolean withOrdinality) {
builder.add(field.getName(), fieldType);
}
} else {
- RelDataType colType = padNullable
- ? typeFactory.enforceTypeWithNullability(componentType, true)
+ RelDataType elementType = componentType.isStruct()
+ ? typeFactory.builder().kind(componentType.getStructKind())
+ .addAll(componentType.getFieldList()).build()
Review 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.
##########
core/src/main/java/org/apache/calcite/rel/core/Uncollect.java:
##########
@@ -74,12 +85,30 @@ public Uncollect(RelOptCluster cluster, RelTraitSet
traitSet,
/** Creates an Uncollect.
*
* <p>Use {@link #create} unless you know what you're doing. */
- @SuppressWarnings("method.invocation.invalid")
public Uncollect(RelOptCluster cluster, RelTraitSet traitSet, RelNode input,
boolean withOrdinality, List<String> itemAliases) {
+ // Non-empty item aliases historically implied that struct elements
are not
Review Comment:
The comments here are not left-aligned.
--
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]
