andygrove commented on code in PR #6607:
URL: https://github.com/apache/datafusion-comet/pull/6607#discussion_r4186685038
##########
spark/src/main/scala/org/apache/spark/sql/comet/execution/shuffle/CometShuffleExchangeExec.scala:
##########
@@ -144,9 +146,16 @@ case class CometShuffleExchangeExec(
ctx.perPartitionByKey,
positionalRoundRobin.isDefined)
case None =>
- // Non-native child (e.g. CometSparkToColumnarExec): no subtree to
inline. The dep gets
- // built via the convenience overload below; we just need a real RDD
of batches.
- child.executeColumnar()
+ child match {
+ // Native reads the source's Arrow stream, as it does for a native
operator's input. The
+ // batches from the source's `executeColumnar` share vectors that it
reuses for the next
+ // batch, and the convenience overload below closes each batch once
native has it.
+ // Closing a struct vector drops its children, so the next batch
would lose them.
+ case source: CometNativeArrowSource =>
source.doExecuteAsArrowStream()
Review Comment:
Agreed, it's in #6689 now. The leaf conversion half of the struct test moved
to `CometNativeShuffleSuite` there, and it also covers a local table scan,
which fails the same way (#6685). This PR keeps the same change until #6689
lands, and then I'll merge main. For the backport call: the code path dates
from #4572, so 1.0.0 has the bug as well, and every conversion that reaches it
is off by default, so it isn't a 1.1.0 regression. I've left it without a
backport label for now.
--
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]
