andygrove commented on PR #6564: URL: https://github.com/apache/datafusion-comet/pull/6564#issuecomment-5998880198
I reproduced the struct failure, but it isn't new in this PR, so I filed #6685 for it. Main fails the same way today with `spark.comet.convert.rdd.enabled` or `spark.comet.exec.localTableScan.enabled`, a struct column and a repartition, and the same code is in every release since 1.0.0. Native shuffle wraps the batches of any child that isn't a native operator in `ColumnarBatchArrowReader`, which closes the vectors the conversion reuses. It also fails at the default batch size once a partition has more than one batch. #6607 reads a `CometNativeArrowSource` child as an Arrow stream instead. With just that change applied on top of this branch, your repro, the orderBy and join variants, and the two leaf conversions all pass. Since this is already queued and the config is off by default, I'd rather let it land and add your struct-under-shuffle test to #6607, which will close #6685, when I merge main into it. A struct decline here would only come out again in #6607. On the profiles, I ran `CometTypedDatasetSuite` against main plus this branch on 3.4, 4.0 and 4.2, and all 15 tests pass. With the earlier 3.5 and 4.1 runs, it's green on every profile. You're right about the top-k arm: `SerializeFromObjectExec` doesn't report an output ordering in any supported version, so I'll remove it. I'll do that, the #6005 TODO, the shared benchmark helper and the `datasources.md` bullet in #6607, since it touches the same code and lands next. -- 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]
