NestDream commented on PR #59038: URL: https://github.com/apache/spark/pull/59038#issuecomment-5864756293
> Thanks, this is a clean fix. Both guards sit exactly where `zip` could drop data: the `convert_fields` branch of `convert_struct`, and the `_needSerializeAnyField` tuple/list branch of `StructType.toInternal`. The no-conversion branches stay unchanged, and the JVM already rejects mismatches there. `Row` also takes the positional tuple branch, so the `Row(c=3, a="y", b=2)` case is covered. Reusing `FIELD_STRUCT_LENGTH_MISMATCH` keeps the error consistent with the `verifySchema=True` path. The local-list test covers both sites: the string column goes through the converter, and `"y long, d date"` skips the converter and reaches `toInternal`. > > Two non-blocking suggestions: > > 1. **Migration guide entry.** Because `StructType.toInternal` is shared, jobs that currently finish can start failing. That includes non-Arrow Python UDFs returning a longer struct, and `applyInPandasWithState` / `transformWithState` state tuples longer than the declared state schema. They were getting truncated values, but they didn't fail. `pyspark_upgrade.rst` usually records this kind of change (e.g. the 4.2 `DATA_SOURCE_RETURN_SCHEMA_MISMATCH` entry). A short item under a new "Upgrading from PySpark 4.3 to 4.4" section would help users who hit it. > 2. **A test for the shared `toInternal` paths.** The description says non-Arrow UDF struct results and state tuples change too, but the new tests only exercise `createDataFrame`. A small case like a `useArrow=False` UDF with return type `"a string, d date"` returning a 3-tuple, asserting `FIELD_STRUCT_LENGTH_MISMATCH`, would pin the behavior for those callers. Thanks for the review @HyukjinKwon . Both done in 72009ab0461: 1. New "Upgrading from PySpark 4.3 to 4.4" section in pyspark_upgrade.rst with one entry covering the three paths that now raise instead of truncating (createDataFrame, non-Arrow UDF struct results, the state value of applyInPandasWithState / transformWithState). 2. test_udf_struct_result_length_mismatch in BaseUDFTestsMixin: a useArrow=False UDF with return type "a string, d date" returning a 3-tuple, asserting FIELD_STRUCT_LENGTH_MISMATCH. It runs in UDFTests, the Connect parity class, the six Arrow UDF classes (plain, legacy, non-legacy, Classic and Connect) and the transpile parity class. Without the types.py change it fails with "PythonException not raised" on Classic and on Connect. On the heading: master is 5.0.0-SNAPSHOT and branch-4.x is 4.4.0, so I went with 4.3 to 4.4 as you suggested, assuming this gets picked to branch-4.x. If it should stay master only I will rename 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: [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]
