[SPARK-59425][PYTHON] Introduce EvalTypeHandler pipeline and migrate scalar Arrow UDF - #58729
Yicong-Huang wants to merge 17 commits into
Conversation
… UDF Co-authored-by: Isaac <no-reply@databricks.com>
…andlers Co-authored-by: Isaac <no-reply@databricks.com>
…(arrow/pandas) Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
…ke registry public Co-authored-by: Isaac <no-reply@databricks.com>
…erializer() Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
… no ignores) Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
…onf base Co-authored-by: Isaac <no-reply@databricks.com>
…das._typing dep Co-authored-by: Isaac <no-reply@databricks.com>
… into _typing Co-authored-by: Isaac <no-reply@databricks.com>
…de-effect import Co-authored-by: Isaac <no-reply@databricks.com>
…rivate submodules Co-authored-by: Isaac <no-reply@databricks.com>
…r) runner contract Co-authored-by: Isaac <no-reply@databricks.com>
…ors lint Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
There was a problem hiding this comment.
I checked the migrated path against the removed code and it's identical:
- Serializer — the old
SQL_SCALAR_ARROW_UDFfell through to theelsebranch (ArrowStreamSerializer(write_start_stream=True)), which is exactly whatBatchEvalTypeHandler.serializerreturns. udfsconstruction — the dispatch prologue's list comprehension is identical to the shared one further down (read_single_udf(pickleSer, udf_info, eval_type, runner_conf, udf_index=...)).runbody — a faithful copy of the old inlinefunc, withcol_names/combined_arrow_schemastill hoisted into__init__(once perread_udfs, as before).
Import integrity of the moves also checks out: worker_util.py already imports Any/Optional/Union, and the PR adds import json + the pyspark.sql.types import that RunnerConf/EvalConf need; worker.py has no remaining bare Conf reference after the import swap; and to_arrow_schema is still used elsewhere in worker.py, so nothing becomes a dead import. unittest.TestCase is the right base for these handler-level tests (no SparkSession/SparkContext needed), and the new module is wired into modules.py + both setup.py files.
Two optional, non-blocking nits:
-
Import-cycle fragility in the package.
eval_handlers/__init__.pyimports_arrowat the bottom while_arrowimportsBatchEvalTypeHandlerback from the package. That's fine as long as the package is always imported first (which the worker and tests do), but importingpyspark.sql.eval_handlers._arrowdirectly first would raiseImportErrorbecauseArrowScalarUDFHandlerisn't defined yet when__init__re-enters it. Moving the base classes into a small leaf module (e.g._base.py) that both__init__and_arrowimport would remove the cycle entirely. Low priority since_arrowis private. -
assertfor duplicate-registration detection in__init_subclass__is stripped underpython -O. A plainif eval_type in EVAL_TYPE_HANDLERS: raise ...would keep the guard (andtest_duplicate_eval_type_rejected) working under optimized mode. Minor — it matches the existingassertstyle in the worker.
Nice foundation for migrating the remaining eval types one at a time.
What changes were proposed in this pull request?
This introduces an extensible execution model for the Arrow/Pandas UDF eval types in the PySpark worker and migrates the first eval type onto it. It is the foundation for the umbrella refactor SPARK-59415 (design: "1DD: Python UDF Eval Handler API Design").
read_udfsinpython/pyspark/worker.pyhas grown into one largeif/elifdispatcher over ~30 eval types. This PR adds apyspark.sql.eval_handlerspackage where each eval type is a self-containedEvalTypeHandlersubclass:EvalTypeHandler[InputBatch, OutputBatch]declares aserializerand implementsrun(split_index, data), which consumes the input stream and yields the output stream.BatchEvalTypeHandler(Iterator[pa.RecordBatch]),GroupedEvalTypeHandler(Iterator[GroupedBatch]), andCoGroupedEvalTypeHandler(Iterator[CoGroupedBatch]).eval_typeand is auto-registered inEVAL_TYPE_HANDLERSvia__init_subclass__;read_udfslooks it up and delegates, so a new eval type attaches without editing a central branch.ArrowScalarUDFHandlermigratesSQL_SCALAR_ARROW_UDFas the first handler.Two supporting moves keep the package self-contained (no behavior change): the shared result-verification helpers move to
pyspark.sql.eval_handlers.verification, andRunnerConf/EvalConfmove next to theirConfbase inpyspark.worker_util.The remaining Arrow/Pandas eval types stay on the existing
if/elifpath and will be migrated incrementally, each independently revertible.Why are the changes needed?
To make each eval type's execution explicit, self-contained, and unit-testable, and to remove the central
if/elifdispatch so subsequent eval types can be migrated one at a time.Does this PR introduce any user-facing change?
No. This is an internal worker refactor: no change to any UDF API, the on-the-wire format, or JVM-side code, and the migrated
SQL_SCALAR_ARROW_UDFpath is behavior-identical.How was this patch tested?
New
pyspark.sql.tests.test_eval_type_handlersunit suite (handler registration, per-category serializers and abstractness,run, and end-to-endArrowScalarUDFHandleroutput including schema coercion), plus the existing Arrow UDF suites that exercise the migrated path.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Isaac
This pull request and its description were written by Isaac.