andygrove commented on code in PR #7251:
URL: https://github.com/apache/datafusion-comet/pull/7251#discussion_r4455935983
##########
spark/src/main/scala/org/apache/spark/sql/comet/operators.scala:
##########
@@ -1961,18 +1966,34 @@ object CometExplodeExec extends
CometOperatorSerde[GenerateExec] {
return None
}
- val isPosExplode = op.generator.nodeName.toLowerCase(Locale.ROOT) ==
"posexplode"
-
val explodeBuilder = OperatorOuterClass.Explode
.newBuilder()
.setChild(childExprProto.get)
.setOuter(op.outer)
.setPosition(isPosExplode)
+ .setInline(isInline)
.addAllProjectList(projectExprs.map(_.get).asJava)
Some(builder.setExplode(explodeBuilder).build())
}
+ /**
+ * `stack(n, v1, ..., vk)` emits `n` rows of `ceil(k / n)` columns, filled
row by row and padded
+ * with nulls. That is `inline` over an array holding one struct per output
row.
+ */
+ private def stackAsStructArray(stack: Stack): Expression = {
+ val numRows = stack.children.head.eval().asInstanceOf[Int]
+ val values = stack.children.tail
+ val fields = stack.elementSchema.fields.toSeq
+ CreateArray(Seq.tabulate(numRows) { row =>
Review Comment:
`getSupportLevel` now returns `Unsupported` for a `stack` whose
`supportCodegen` is false (more than 50 rows), so the struct rewrite is never
built for those. The check runs before `convert`, so EXPLAIN no longer
allocates the per-row structs either. Added a test that `stack(51, ...)` falls
back with that reason and one that `stack(50, ...)` still runs natively. Pushed
in a7a754e02.
##########
native/core/src/execution/planner/explode.rs:
##########
@@ -162,11 +163,30 @@ impl OperatorBuilder for ExplodeBuilder {
}
};
- output_fields.push(Field::new(
- array_field.name(),
- element_type,
- true, // Element is nullable after unnesting
- ));
+ // `inline` flattens each struct element into one column per field.
Every field is
+ // nullable after unnesting: a null struct element, or the row
`inline_outer` emits for a
+ // NULL or empty array, yields a row of nulls.
+ let struct_column_indices = if explode.inline {
+ let DataType::Struct(struct_fields) = &element_type else {
+ return Err(ExecutionError::GeneralError(format!(
+ "Expected List of Struct for inline, got {:?}",
+ array_field.data_type()
+ )));
+ };
+ output_fields.extend(
Review Comment:
Took the fallback route. `getSupportLevel` returns `Unsupported` for
`inline` / `inline_outer` when the element struct has no fields, so those
queries keep Spark behavior and the zero-column batch is never built. `stack`
always has at least one column, so it cannot reach this. I did not check
whether the JVM export path accepts a zero-column batch. Added a
`CometGenerateExecSuite` test over `array(struct(), struct())` for both
variants. Pushed in a7a754e02.
--
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]
