HyukjinKwon commented on PR #58142: URL: https://github.com/apache/spark/pull/58142#issuecomment-5349570652
Thanks for the thorough review, @dongjoon-hyun! Addressed in the latest commit. Point by point: 1. **`_test()` doctest guard** — fixed: `del pyspark.sql.functions.builtin.udaf.__doc__` is now added alongside `udf`/`arrow_udtf` when PyArrow/pandas are missing, so the new `udaf` doctest doesn't fail in a no-PyArrow doctest run. 2. **Mixed incremental + grouped-agg over one window → `AssertionError`** — fixed: `ArrowWindowPythonExec.apply` now raises a clear `AnalysisException` via `QueryCompilationErrors.foundDifferentWindowFunctionTypeError` when the grouped Python window functions have different eval types (they share `WindowFunctionType.Python` so they pass `PhysicalWindow`, but can't run in one operator). This also gives the pre-existing pandas-vs-arrow mix a clean error instead of an assertion. Added `test_window_mixed_python_udf_rejected`. 3. **O(n^2) bounded-frame refold** — added a `TODO(SPARK-58890)` at the bounded branch describing the growing-frame O(n^2) and the running-buffer (O(n)) optimization when `begin` doesn't advance. Left as a follow-up as you suggested. 4. **`PythonEvalType.toString`** — fixed: `SQL_WINDOW_AGG_ARROW_INCREMENTAL_UDF` is now in the `match`. Nits: - **`ArrowWindowPythonExec` scaladoc** — updated to list the incremental eval type. - **`test_functions` stale `udaf` exclusion** — dropped from `jvm_excluded_fn` (udaf is a real PySpark function now, so the JVM `functions.udaf` is matched by it). - **Unbounded path materializes the whole partition via `to_pylist()`** — agreed, this is the intentional trade-off (no shuffle on the window path), so the bounded-memory property doesn't carry over; noted, no change. - **Only `DoubleType` output covered** — added `test_window_decimal_output` running `DecimalSum` through a window to exercise the explicit `pa.array(..., type=result_type)` typing. Verified locally: classic 28/28 and Connect parity 26/26 pass. -- 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]
