HyukjinKwon commented on PR #57899:
URL: https://github.com/apache/spark/pull/57899#issuecomment-5247312982

   Thanks again, @dongjoon-hyun — item 1 was a real silent-correctness bug. 
Fixed in 50ff335559d3.
   
   ### 1. Iterator path dropping rows on an all-empty/null partition
   
   Confirmed the analysis: the JVM join is driven by the output iterator, so 
any unmatched input row is dropped silently. When a whole partition is 
empty/null arrays and a skip-empty iterator UDF yields nothing, `buffered` 
stayed `None` and the final `emit_ready` broke on `buffered is None` even for 
rows needing zero elements.
   
   Fix: replaced the single growing buffer with a list of chunks + running 
length. A row needing zero elements (empty **or** null array) is now emitted 
immediately without waiting for a chunk, using the first-seen chunk's element 
type (falling back to the UTC-typed `arrow_element_type`; a partition that only 
ever emits zero-length rows never mixes the two, so the stream schema stays 
consistent). Added `test_scalar_iter_udf_over_all_empty_and_null_partition` 
over a single `coalesce(1)` partition of only `[]`/`NULL`, covering both the 
pandas and Arrow flavors with a skip-empty UDF. Also fixed the comment — 
residual shapes can be empty (non-null) arrays too.
   
   ### Minor (all taken)
   
   - **Repeated `pa.concat_arrays`**: the chunk-list buffer also resolves this 
— chunks are concatenated only when a shape spans more than one, so a UDF that 
yields once per input batch never re-copies the buffer.
   - **`_elementwise_renest` second return value**: dropped; both callers now 
take just the nested array, and the docstring no longer claims a 
length-verification role no caller used.
   - **Test message nit**: now prints the eval-type name via 
`PythonEvalType.toString(expectedEvalType)` instead of the function object.
   - **Struct-element return test**: added 
`test_scalar_pandas_udf_struct_element_return_type` (a pandas UDF returning a 
`pd.DataFrame`/struct element inside a lambda).
   - **Timestamp test**: strengthened to also assert against a native 
`timestamp_add(DAY, x, TIMESTAMP '2020-01-01')` expression, so a timezone bug 
common to both UDF paths would be caught, while going through identical 
driver-collection semantics.
   
   Agreed the batch-size amplification is inherent to the 102 design, so left 
as-is.
   
   Full suite green: 52 classic + 52 Connect parity (+1 skip) + 24 
`ExtractPythonUDFFromLambdaSuite`, plus `ruff format`/`ruff check`.
   


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