srielau commented on code in PR #58549:
URL: https://github.com/apache/spark/pull/58549#discussion_r3951595145
##########
python/pyspark/sql/udf.py:
##########
@@ -314,9 +317,24 @@ def _conf_is_true(key: str, default: Optional[str] = None)
-> bool:
@staticmethod
def _check_return_type(returnType: DataType, evalType: int) -> None:
+ char_varchar_supported_eval_types = (
+ PythonEvalType.SQL_ARROW_BATCHED_UDF,
+ PythonEvalType.SQL_SCALAR_PANDAS_UDF,
+ PythonEvalType.SQL_SCALAR_PANDAS_ITER_UDF,
+ PythonEvalType.SQL_SCALAR_ARROW_UDF,
+ PythonEvalType.SQL_SCALAR_ARROW_ITER_UDF,
+ )
+
+ def check_arrow_type() -> None:
+ if evalType not in char_varchar_supported_eval_types and _has_type(
Review Comment:
Fixed in 157b0d247d1. Both `SQL_GROUPED_AGG_PANDAS_ITER_UDF` and
`SQL_GROUPED_AGG_ARROW_ITER_UDF` now go through the existing aggregate
return-type validation, which recursively rejects CHAR/VARCHAR. Added both
variants to the focused negative eval-type matrix.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/python/EvalPythonEvaluatorFactory.scala:
##########
@@ -36,6 +38,16 @@ abstract class EvalPythonEvaluatorFactory(
output: Seq[Attribute])
extends PartitionEvaluatorFactory[InternalRow, InternalRow] {
+ private val applyCharVarcharChecks =
+ CharVarcharUtils.shouldApplyWriteSideLengthCheck(SQLConf.get)
+ private val checkedOutput = if (applyCharVarcharChecks) {
+ childOutput ++ output.drop(childOutput.length).map { attr =>
Review Comment:
Fixed in 157b0d247d1. `ExtractPythonUDFs` now stops nested fusion when the
intermediate result recursively contains CHAR/VARCHAR and write-side checks are
active, so the inner result is materialized and checked before the outer UDF
consumes it. Added pickled and Arrow execution regressions for
padding/overflow, plus physical-plan tests proving standard mode splits both
executor types while legacy mode retains fusion.
##########
python/pyspark/sql/pandas/types.py:
##########
@@ -133,7 +135,7 @@ def to_arrow_type(
arrow_type = pa.float64()
elif isinstance(dt, DecimalType):
arrow_type = pa.decimal128(dt.precision, dt.scale)
- elif isinstance(dt, StringType):
+ elif isinstance(dt, (StringType, CharType, VarcharType)):
Review Comment:
Fixed in 157b0d247d1. Arrow-optimized and PyArrow-native UDTF return-type
validation now recursively rejects schemas containing CHAR/VARCHAR before
executor planning. Added direct CHAR and nested array-of-CHAR negative coverage.
##########
python/pyspark/sql/tests/arrow/test_arrow_python_udf.py:
##########
@@ -270,18 +271,45 @@ def f(v: float):
rounded = df.select(f("v").alias("d")).first().d
self.assertEqual(rounded, Decimal("1.233999999999999986"))
- def test_err_return_type(self):
- with self.assertRaises(PySparkNotImplementedError) as pe:
- udf(lambda x: x, VarcharType(10), useArrow=True)
-
- self.check_error(
- exception=pe.exception,
- errorClass="NOT_IMPLEMENTED",
- messageParameters={
- "feature": "Invalid return type with Arrow-optimized Python
UDF: VarcharType(10)"
- },
+ def test_char_varchar_results(self):
+ schema = StructType(
+ [
+ StructField("c", CharType(4)),
+ StructField("v", VarcharType(3)),
+ StructField("nested", ArrayType(CharType(2))),
+ StructField("m", MapType(CharType(2), VarcharType(3))),
+ ]
)
+ with self.sql_conf(
+ {
+ "spark.sql.charVarchar.standardSemantics.enabled": "true",
Review Comment:
Added in 157b0d247d1. The Arrow UDF legacy-mode regression uses
`useArrow=True` and verifies that under-length CHAR remains unpadded and
over-length VARCHAR remains accepted.
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/python/ArrowColumnarPythonUDFSuite.scala:
##########
@@ -103,6 +104,39 @@ class ArrowColumnarPythonUDFSuite extends
SharedSparkSession {
}
}
+ test("Arrow-backed source: CHAR/VARCHAR output checks") {
+ assume(shouldTestPandasUDFs)
+ withSQLConf(
+ SQLConf.ARROW_PYSPARK_EXECUTION_ENABLED.key -> "true",
+ SQLConf.ARROW_PYSPARK_UDF_COLUMNAR_INPUT_ENABLED.key -> "true",
+ SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
Review Comment:
Added in 157b0d247d1. The `readArrowSource` legacy-mode test verifies
unpadded CHAR, accepted over-length VARCHAR, and an Arrow-backed columnar child.
--
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]
