You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "Yicong-Huang (via GitHub)" <gi...@apache.org> on 2026/08/05 22:18:44 UTC
[PR] [SPARK-58601][PYTHON] Tighten mapInPandas return-value contract to require a strict Iterator [spark]
Yicong-Huang opened a new pull request, #57800:
URL: https://github.com/apache/spark/pull/57800
### What changes were proposed in this pull request?
Tighten the `mapInPandas` (`SQL_MAP_PANDAS_ITER_UDF`) return-value contract in `worker.py` to require a strict `Iterator`, matching `mapInArrow` (`SQL_MAP_ARROW_ITER_UDF`).
Previously the runtime check was `isinstance(result, Iterator) or hasattr(result, "__iter__")`, which accepted any iterable (e.g. a returned `list`). It is now `isinstance(result, Iterator)`; a non-`Iterator` iterable is rejected with `UDF_RETURN_TYPE` ("iterator of pandas.DataFrame"/"pandas.Series"). The comment explaining why `verify_return_type` was not reused is removed accordingly.
### Why are the changes needed?
The declared signature has always been `PandasMapIterFunction = Callable[[Iterator[DataFrameLike]], Iterator[DataFrameLike]]`, and the `DataFrame.mapInPandas` docstring states the function "outputs an iterator of pandas.DataFrames". The runtime, however, leniently accepted any iterable, diverging from the documented contract. This aligns the runtime with the declared `Iterator[...]` signature, mirroring the same tightening already done for `mapInArrow` in SPARK-56612. It also unblocks reusing `verify_return_type` at the `mapInPandas` site with `Iterator[...]` (SPARK-58598).
### Does this PR introduce _any_ user-facing change?
No. Minor tightening: a UDF returning a non-`Iterator` iterable (e.g. `list`) is now rejected with `UDF_RETURN_TYPE`, aligning the runtime with the documented `Iterator[...]` signature. This mirrors the `mapInArrow` tightening in SPARK-56612.
### How was this patch tested?
Updated `test_pandas_map.py`: removed the "returning list of DataFrames" positive case from `test_map_in_pandas`, and added a `list_not_iter` negative case to `check_other_than_dataframe_iter` asserting a returned `list` is rejected, mirroring `test_arrow_map.py::test_other_than_recordbatch_iter`. Ran `pyspark.sql.tests.pandas.test_pandas_map` and the Connect parity suite.
### Was this patch authored or co-authored using generative AI tooling?
No
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58601][PYTHON] Tighten mapInPandas return-value contract to require a strict Iterator [spark]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #57800:
URL: https://github.com/apache/spark/pull/57800#discussion_r3770461543
##########
python/pyspark/worker.py:
##########
@@ -1923,6 +1933,16 @@ def func(split_index: int, data: Iterator[pa.RecordBatch]) -> Iterator[pa.Record
# invoke the UDF
output_batches = udf_func(input_batches)
+ # The declared signature is Iterator[...], so a strict iterator is required. The
+ # legacy flag restores the pre-4.3.0 behavior of accepting any iterable (e.g. list)
+ # by adapting it into an iterator before the shared element-type verification.
+ if (
+ runner_conf.map_in_batch_legacy_accept_any_iterable
+ and not isinstance(output_batches, Iterator)
+ and hasattr(output_batches, "__iter__")
Review Comment:
the `__iter__` check was a legacy behavior and we should pin it to what we have today. the goal is to remove this legacy behavior, I think we should avoid making it more wide by accepting more types that implement `__getitem__`.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58601][PYTHON] Tighten mapInPandas return-value contract to require a strict Iterator [spark]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #57800:
URL: https://github.com/apache/spark/pull/57800#issuecomment-5209659005
I changed it to describe as behavior change. Also added a flag to revert back to the old behavior. the same flag will guard mapInArrow as well. A migration guide from 4.2 to 4.3 is added.
Note that I kept the flag `spark.sql.execution.pythonUDF.mapInBatch.legacy.acceptAnyIterable.enabled` to be `false` by default. meaning that the change will take effect and choose to use the old behavior by explcitily setting the flag to be `true`.
@uros-b @HyukjinKwon @cloud-fan could you please check again?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58601][PYTHON] Tighten mapInPandas return-value contract to require a strict Iterator [spark]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #57800:
URL: https://github.com/apache/spark/pull/57800#issuecomment-5198449468
> Should we update the migration guide? To explain mapInPandas UDF returning a plain list of DataFrames (e.g. lambda it: [pdf for pdf in it]) previously succeeded and now raises UDF_RETURN_TYPE with "but is list"
I am on the fence here. The public doc declares `Callable[[Iterator[DataFrameLike]]` and accepting `list` which is an iterable not iterator has always been a silently allowed side effect. This PR tightens the allowed return type to its declared type, so not sure how to mention it in migration guide. Maybe @HyukjinKwon @cloud-fan can suggest action here?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58601][PYTHON] Tighten mapInPandas return-value contract to require a strict Iterator [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #57800:
URL: https://github.com/apache/spark/pull/57800#discussion_r3754694217
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -5390,6 +5390,16 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val PYTHON_UDF_MAP_IN_BATCH_LEGACY_ACCEPT_ANY_ITERABLE_ENABLED =
+ buildConf("spark.sql.execution.pythonUDF.mapInBatch.legacy.acceptAnyIterable.enabled")
+ .internal()
+ .doc("When true, mapInPandas and mapInArrow UDFs may return any iterable (e.g. a list) " +
+ "rather than a strict iterator, matching the behavior before 4.3.0. When false, the " +
+ "returned value must be an iterator, matching the declared Iterator[...] signatures.")
+ .version("4.3.0")
Review Comment:
ah ok. let's land this to 4.3. synced w @Yicong-Huang
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58601][PYTHON] Tighten mapInPandas return-value contract to require a strict Iterator [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #57800:
URL: https://github.com/apache/spark/pull/57800#discussion_r3789672870
##########
python/pyspark/worker.py:
##########
@@ -1923,6 +1933,16 @@ def func(split_index: int, data: Iterator[pa.RecordBatch]) -> Iterator[pa.Record
# invoke the UDF
output_batches = udf_func(input_batches)
+ # The declared signature is Iterator[...], so a strict iterator is required. The
+ # legacy flag restores the pre-4.3.0 behavior of accepting any iterable (e.g. list)
+ # by adapting it into an iterator before the shared element-type verification.
+ if (
+ runner_conf.map_in_batch_legacy_accept_any_iterable
+ and not isinstance(output_batches, Iterator)
+ and hasattr(output_batches, "__iter__")
Review Comment:
Python's iterable protocol includes the sequence protocol: objects implementing `__getitem__` can be consumed by `iter(...)` even without `__iter__`. The current config name and migration note promise acceptance of any iterable, so please either implement that advertised contract by attempting `iter(...)` in both branches (with a sequence-protocol regression test), or narrow the config name and documentation consistently to describe the historical `__iter__`-only behavior.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58601][PYTHON] Tighten mapInPandas return-value contract to require a strict Iterator [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #57800:
URL: https://github.com/apache/spark/pull/57800#discussion_r3740195776
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -5390,6 +5390,16 @@ object SQLConf {
.booleanConf
.createWithDefault(false)
+ val PYTHON_UDF_MAP_IN_BATCH_LEGACY_ACCEPT_ANY_ITERABLE_ENABLED =
+ buildConf("spark.sql.execution.pythonUDF.mapInBatch.legacy.acceptAnyIterable.enabled")
+ .internal()
+ .doc("When true, mapInPandas and mapInArrow UDFs may return any iterable (e.g. a list) " +
+ "rather than a strict iterator, matching the behavior before 4.3.0. When false, the " +
+ "returned value must be an iterator, matching the declared Iterator[...] signatures.")
+ .version("4.3.0")
Review Comment:
4.4 since branch-4.3 is already cut out
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58601][PYTHON] Tighten mapInPandas return-value contract to require a strict Iterator [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #57800:
URL: https://github.com/apache/spark/pull/57800#discussion_r3755586837
##########
python/pyspark/worker.py:
##########
@@ -1923,6 +1933,16 @@ def func(split_index: int, data: Iterator[pa.RecordBatch]) -> Iterator[pa.Record
# invoke the UDF
output_batches = udf_func(input_batches)
+ # The declared signature is Iterator[...], so a strict iterator is required. The
+ # legacy flag restores the pre-4.3.0 behavior of accepting any iterable (e.g. list)
+ # by adapting it into an iterator before the shared element-type verification.
+ if (
+ runner_conf.map_in_batch_legacy_accept_any_iterable
+ and not isinstance(output_batches, Iterator)
+ and hasattr(output_batches, "__iter__")
Review Comment:
**Blocking:**
The compatibility mode should accept every object Python can iterate, but this guard excludes sequence-protocol objects that implement `__getitem__` without `__iter__`; the pandas branch at `worker.py:2726` does the same. Please attempt `iter(...)` and catch `TypeError` in both branches, with a sequence-protocol regression test.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58601][PYTHON] Tighten mapInPandas return-value contract to require a strict Iterator [spark]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #57800:
URL: https://github.com/apache/spark/pull/57800#issuecomment-5421705590
**Merge Summary:**
- merged into master https://github.com/apache/spark/commit/69a108ba8adfef816f0652104c64062947c0df8c
- merged into branch-4.x https://github.com/apache/spark/commit/b456177af27bea14f5088cd23aca3ae019451052
- merged into branch-4.3 https://github.com/apache/spark/commit/decb7346459a4da5b9e61b298fbaeff2fc5c7b4d
*Posted by `merge_spark_pr.py`*
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58601][PYTHON] Tighten mapInPandas return-value contract to require a strict Iterator [spark]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang closed pull request #57800: [SPARK-58601][PYTHON] Tighten mapInPandas return-value contract to require a strict Iterator
URL: https://github.com/apache/spark/pull/57800
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58601][PYTHON] Tighten mapInPandas return-value contract to require a strict Iterator [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #57800:
URL: https://github.com/apache/spark/pull/57800#discussion_r3736429392
##########
python/docs/source/migration_guide/pyspark_upgrade.rst:
##########
@@ -19,6 +19,10 @@
Upgrading PySpark
==================
+Upgrading from PySpark 4.2 to 4.3
+---------------------------------
+* In Spark 4.3, a ``mapInPandas`` UDF must return an iterator of ``pandas.DataFrame``\s; returning any other iterable such as a ``list`` now raises ``UDF_RETURN_TYPE``, matching the existing ``mapInArrow`` behavior and the declared ``Iterator[...]`` signature. To restore the previous behavior of accepting any iterable for both ``mapInPandas`` and ``mapInArrow``, set ``spark.sql.execution.pythonUDF.mapInBatch.legacy.acceptAnyIterable.enabled`` to ``true``.
Review Comment:
Please update the Spark Connect callback aliases to require `Iterator` as well. `connect/_typing.py` still declares both `PandasMapIterFunction` and `ArrowMapIterFunction` with `Iterable` returns, but Connect dispatches these same strict evaluation types, so its public annotations currently tell users that returning a list is valid when the worker rejects it.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58601][PYTHON] Tighten mapInPandas return-value contract to require a strict Iterator [spark]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #57800:
URL: https://github.com/apache/spark/pull/57800#issuecomment-5419160164
The only CI failure is on old client test, https://github.com/Yicong-Huang/spark/actions/runs/32895462731/job/98005794753. I will proceed on merging this, and fix the old client CI with #58286
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org