sunchao commented on code in PR #5414:
URL: https://github.com/apache/datafusion-comet/pull/5414#discussion_r3836225436
##########
spark/src/main/scala/org/apache/comet/serde/structs.scala:
##########
@@ -250,7 +250,9 @@ object CometJsonToStructs extends
CometCodegenDispatch[JsonToStructs] with Nativ
private def isSupportedSchema(dt: DataType): Boolean = dt match {
case StructType(fields) =>
- fields.nonEmpty && fields.forall(f => isSupportedSchema(f.dataType))
+ // A struct's `fields` can be empty -- e.g. `from_json(col,
'struct<>')`'s target schema.
+ // With no fields to check, this holds vacuously.
+ fields.forall(f => isSupportedSchema(f.dataType))
Review Comment:
[P2] Keep nested empty structs off the native path until they are
constructed safely
Because this predicate is recursive, it now admits `struct<outer:struct<>>`,
not just the top-level `struct<>` covered by the new test. The top-level Rust
branch does not cover that shape: `finish_builder` still calls
`StructArray::new` for every nested `FieldBuilder::Struct`; for an empty nested
struct, `builders` yields zero child arrays. Arrow 58.4.0's `StructArray::new`
unwraps `try_new`, which rejects no child arrays, so the query panics once this
PR routes it native. Please add the same empty-fields construction in
`finish_builder` (using its null-buffer length) and a nested-empty regression
test, or keep nested empties unsupported here.
--
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]
