viirya commented on PR #56940:
URL: https://github.com/apache/spark/pull/56940#issuecomment-4860435028

   Closing this PR. @gaogaotiantian is right — the output fast path silently 
bypasses the string/binary conversion logic, and I verified it is a breaking 
change.
   
   The fast path (`pa.array(raw_results, type=...)`, falling back to the 
per-element converter only on `ArrowException`) only ever *activates* when 
`result_conv is not None`. Among the return types the gate treats as safe, that 
is exactly `StringType` and `BinaryType` — numeric/bool/date leaves have no 
converter, so the fast path is skipped for them entirely. And `pa.array` 
performs permissive cross-type coercion that diverges from the converters for 
precisely those two types:
   
   - `array<string>` (or top-level `string`) returning `bytes`: `pa.array` 
decodes to `"abc"`, while `convert_string` produces the Python repr `"b'abc'"`. 
Verified end-to-end — master (non-legacy) returns `["b'abc'", "b'de'"]`, this 
PR returns `["abc", "de"]`.
   - `binary` returning `str`: `pa.array` encodes to `b'abc'`, while 
`convert_binary` raises (it asserts `bytes`) — so an error is silently turned 
into a value.
   
   (The `bool` case in your snippet is actually safe by luck: `pa.array` 
rejects `bool`->string with `ArrowTypeError`, so it falls back to the converter 
and still lowercases. But the concern is correct — `bytes`/`str` are the cases 
that slip through.)
   
   The `_output_fast_path_safe` gate only excluded value-transforming types 
(timestamp/decimal); it missed `pa.array`'s cross-type coercion, and the only 
types where the fast path runs are the ones that diverge — so the approach is 
unsafe wherever it takes effect.
   
   The larger part of the pandas->Arrow nested-column regression is on the 
input side, handled independently in #56943 (SPARK-57864) via a different 
mechanism (`to_pandas()` + recursive listify, gated on columns needing *no* 
per-element conversion), which is not affected by this. Closing in favor of 
that; thanks for the catch.


-- 
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]

Reply via email to