andygrove commented on code in PR #5866:
URL: https://github.com/apache/datafusion-comet/pull/5866#discussion_r3997539295
##########
spark/src/main/scala/org/apache/spark/sql/comet/execution/shuffle/CometShuffleExchangeExec.scala:
##########
@@ -477,7 +477,10 @@ object CometShuffleExchangeExec
case dt if isTimeType(dt) =>
true
case StructType(fields) =>
- fields.nonEmpty && fields.forall(f =>
supportedSerializableDataType(f.dataType))
+ fields.nonEmpty && fields.forall(f =>
supportedSerializableDataType(f.dataType)) &&
+ // Java Arrow keys struct children by name, so the FFI import of a
decoded batch
+ // fails on duplicate field names
Review Comment:
The description says no plan reaches this clause today. I think that stops
being true as soon as any native operator can emit a duplicate-named struct,
which #5603 would do for `named_struct`. Might be worth rewording the comment
to say this is the backstop for native-produced structs rather than framing it
purely as parity with the columnar predicate, so nobody removes it as dead code
later.
--
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]
