viirya commented on PR #57137: URL: https://github.com/apache/spark/pull/57137#issuecomment-4920071728
Thanks for the honest feedback — I agree with the principle, and I've scoped this PR down accordingly. On "this should be done in arrow": fully agreed, and that's where the real fix is happening — apache/arrow#50327 (reworked per Antoine's review) moves the scalar-free conversion into PyArrow itself, with apache/arrow#50429 covering the map→dict form Spark consumes. The Spark-side `_to_pylist` shims from SPARK-58019/58023/58024 are explicitly interim, each with a removal note for when the minimum PyArrow version includes the fix. I've now **dropped the input-side converter fusion from this PR** — you're right that it stacked more machinery on the interim layer, and once the PyArrow fix ships it reduces to a few lines anyway, with no NumPy involved at all. Deferring it until then both answers the maintainability concern and makes it simpler. What remains is the **output side only**, which I'd argue is a different category from "taking over arrow's work": it removes *Spark's own* per-row result converters (defensive list copies, dict→entry-list, `Row`→dict) that run before `pa.array`. Arrow can never do this for us — it doesn't know what a `Row` is — and this half uses no NumPy and doesn't depend on the PyArrow version. It's a permanent simplification of Spark's own hot loop, guarded so that anything unexpected falls back to the existing per-row path with identical error behavior. On "we don't know what happens on other data types": we measured — scalar types are already *faster* on the arrow path than pickle (string 2.2x, date 2.4x, timestamp_ntz 2.9x, decimal 2.2x at 6.4M rows). The regression is specific to nested types, which is exactly what this line of work targets, and where the remaining gap closes when the PyArrow fix lands. -- 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]
