cloud-fan commented on code in PR #58549:
URL: https://github.com/apache/spark/pull/58549#discussion_r4186053146
##########
python/pyspark/sql/tests/test_types.py:
##########
@@ -659,6 +660,48 @@ def test_string_type_simple_string(self):
self.assertEqual(StringType("UTF8_LCASE").simpleString(), "string
collate UTF8_LCASE")
self.assertEqual(StringType("UNICODE").simpleString(), "string collate
UNICODE")
+ def test_check_no_char_varchar(self):
+ from pyspark.testing.objects import ExamplePointUDT
+
+ _check_no_char_varchar(StringType(), "Python UDF return types")
+ _check_no_char_varchar(ArrayType(StringType()), "Python UDF return
types")
+
+ cases = [
+ CharType(3),
+ VarcharType(8),
+ ArrayType(CharType(5)),
+ MapType(VarcharType(4), StringType()),
Review Comment:
**Non-blocking (P2):** Every changed map fixture constrains the key, so
key-first short-circuiting masks whether valueType recursion still works.
Removing value traversal would leave these tests green while a map with a
supported key and CHAR/VARCHAR value bypasses the new rejection.
**Verification:**
- **Regression:** Exercise a map whose key contains no constrained string
and whose value contains direct or nested CHAR/VARCHAR, asserting the classed
error and offending full map type.
##########
python/pyspark/sql/udf.py:
##########
@@ -213,12 +214,16 @@ def __init__(
)
self.evalType = evalType
self.deterministic = deterministic
+ if isinstance(returnType, DataType):
+ UserDefinedFunction._check_return_type(returnType, evalType)
Review Comment:
**Non-blocking (P2):** Calling the full eval-mode checker during
construction moves unrelated Arrow and Pandas return-type failures earlier and
repeats the same schema conversion when returnType is consumed. This affects
both Classic and Connect wrappers even when the declaration contains no
CHAR/VARCHAR.
**Verification:**
- **Behavior:** Verify materialized direct and nested CHAR/VARCHAR types are
rejected at construction in both Classic and Connect.
- **Regression:** Use non-CHAR/VARCHAR schemas that fail mode-specific Arrow
or StructType validation to prove construction remains successful and the
established failure occurs only when returnType is consumed.
- **Inspection:** Confirm constructors invoke only the recursive
constrained-string predicate and leave full eval-type dispatch at the lazy
property boundary.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/python/UserDefinedPythonFunction.scala:
##########
@@ -63,6 +64,11 @@ case class UserDefinedPythonFunction(
bufferType: DataType = null) {
def builder(e: Seq[Expression]): Expression = {
+ if (CharVarcharUtils.hasCharVarchar(dataType) ||
+ (bufferType != null && CharVarcharUtils.hasCharVarchar(bufferType))) {
Review Comment:
The pinned tree still combines the return and buffer checks and reports
`Python UDF return types` with `dataType`. Please split the branches so a
buffer-only failure reports `Python UDAF buffer schemas` and `bufferType`,
while preserving return-before-buffer precedence.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4139319570","thread_id":"inline:4139319570","verdict_sha256":"78154da676c40b545b9176547ed9da445c8861ff82a6f8fd9c155f88ffc13fad"}
-->
--
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]
