HyukjinKwon commented on PR #59424: URL: https://github.com/apache/spark/pull/59424#issuecomment-6225142534
@sarutak Thank you for the thorough review, with a probe library. All the inline comments are addressed in 74aa0dffc534, with replies there. On Arrow UDFs: I agree that the root cause is in the columnar input path, and thanks for showing it with `ArrowCachedBatchSerializer`. I opened SPARK-60294 with #59551. It copies the pass-through columns of each batch into vectors of the task with `VectorAppender`, and releases them once the output batch that holds them is no longer used. Its test reproduces both failures without native data sources, with a test source that refills the same vectors for every batch. On your two questions: - This PR waits for #59551, rather than sending its batches to the row queue path: with the fix, the batches of a native data source take the Arrow columnar path like the batches of other sources, and the Arrow cache is fixed too. - `Arrow UDFs on the columns of a native data source` runs a UDF over one batch, two and 2,000, and checks the pass-through columns of every row against the result of the UDF. Locally on Java 26, it fails without #59551 and passes with it. The CI of pull requests skips it, since the job that runs the native data source suites has no pandas. @zhztheplayer Thanks for the review. - Native library infrastructure: I agree that discovery, platform selection, distribution, checksums and loading are not specific to data sources. I would keep them internal in this PR, because a general facility needs its own API: what a plugin declares in a manifest, how it looks up its libraries, and how the libraries of the sessions are isolated. They are already in classes separate from the data source integration (`NativeDataSourcePackage`, `NativeLibraries` and `NativeDataSourceRegistry`, apart from `NativeDataSource`), so a follow-up can extract them without changing the package format, once there is a second user such as Gluten. I'd be glad to work on that with you. - Columnar writes: yes, a desirable follow-up, which @zhouyuan asked for too. The writers of the adapter already take Arrow-backed `ColumnarBatch`es, so `V2TableWriteExec` could pass the batches of a columnar child to them directly. - SPARK-57468 and #59154: as @sarutak said, they are about the conventions of the batches and the transitions between operators, so the lifetime of the input batches of Python UDFs needs its own fix, #59551. -- 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]
