srielau commented on code in PR #58549:
URL: https://github.com/apache/spark/pull/58549#discussion_r4086850996


##########
python/pyspark/sql/types.py:
##########
@@ -3051,8 +3051,22 @@ def _has_type(dt: DataType, dts: Union[type, Tuple[type, 
...]]) -> bool:
         return _has_type(dt.elementType, dts)
     elif isinstance(dt, MapType):
         return _has_type(dt.keyType, dts) or _has_type(dt.valueType, dts)
+    else:

Review Comment:
   Fixed in 69ef259f915. `_has_type` again descends through UDT storage, 
preserving the nanosecond map-key guard and the existing year-month/pandas 
preflight contracts. Logical CHAR/VARCHAR capability checks now use a separate 
`_has_logical_type` helper, with focused helper coverage.



##########
python/pyspark/sql/tests/arrow/test_arrow.py:
##########
@@ -1935,7 +1944,143 @@ def 
test_toPandas_nested_array_with_map_empty_outer(self):
     pandas_requirement_message or pyarrow_requirement_message,
 )
 class ArrowTests(ArrowTestsMixin, ReusedSQLTestCase):
-    pass
+    # These CHAR/VARCHAR cases are Classic-only: they exercise 
standard-semantics
+    # createDataFrame/toArrow and assign a UDT-backed `_schema` directly, 
neither of which is

Review Comment:
   Fixed in 69ef259f915. The class comment now accurately says that the 
Classic-only test overrides the cached `schema` attribute; it no longer refers 
to Connect `_schema`.



##########
sql/core/src/main/scala/org/apache/spark/sql/execution/python/ColumnarArrowEvalPythonEvaluatorFactory.scala:
##########
@@ -27,27 +27,43 @@ import org.apache.spark.api.python.ChainedPythonFunctions
 import org.apache.spark.internal.config.Python.PYTHON_UDF_PIPELINED_EXECUTION
 import org.apache.spark.sql.catalyst.InternalRow
 import org.apache.spark.sql.catalyst.expressions._
+import org.apache.spark.sql.catalyst.util.CharVarcharUtils
 import org.apache.spark.sql.errors.QueryExecutionErrors
 import org.apache.spark.sql.execution.RowToColumnConverter
 import org.apache.spark.sql.execution.metric.SQLMetric
 import org.apache.spark.sql.execution.python.EvalPythonExec.ArgumentMetadata
 import org.apache.spark.sql.execution.vectorized.OnHeapColumnVector
-import org.apache.spark.sql.types.{DataType, StructField, StructType, 
UserDefinedType}
+import org.apache.spark.sql.types.{DataType, StructField, StructType}
 import org.apache.spark.sql.types.DataType.equalsIgnoreCompatibleCollation
+import org.apache.spark.sql.util.ArrowUtils
 import org.apache.spark.sql.vectorized.{ArrowColumnVector, ColumnarBatch, 
ColumnVector}
 import org.apache.spark.util.Utils
 
+private[python] object ColumnarArrowEvalPythonEvaluatorFactory {
+  def toArrowPhysicalType(dataType: DataType): DataType =
+    CharVarcharUtils.replaceCharVarcharWithStringForPhysicalType(dataType)
+
+  def canUseArrowColumnar(
+      inputColumnIndices: Option[Array[Int]],
+      isArrow: Boolean,
+      udfs: Seq[PythonUDF]): Boolean = {
+    inputColumnIndices.isDefined &&
+      isArrow &&
+      !udfs.exists(_.hasCharVarcharResult)
+  }
+}
+
 /**
  * Evaluator factory for Arrow Python UDFs: ColumnarBatch in, ColumnarBatch 
out.
  *
  * Three execution paths based on input characteristics:
  *
  * 1. '''Arrow columnar path''' (UDF inputs are simple column refs AND
  *    columns are [[ArrowColumnVector]]): Arrow FieldVectors are extracted
- *    directly and serialized to IPC. Pass-through columns are kept as
- *    [[ColumnVector]] references (safe because Arrow vectors are
- *    independently allocated per batch). Output is produced by columnar
- *    combining: passThruCols ++ resultCols -> ColumnarBatch.
+ *    directly and serialized to IPC. Pass-through columns are transferred

Review Comment:
   Fixed in 69ef259f915. The Path 1 documentation now includes the checked 
CHAR/VARCHAR result exclusion alongside the simple-column and Arrow-vector 
requirements.



##########
sql/core/src/main/scala/org/apache/spark/sql/execution/python/UserDefinedPythonFunction.scala:
##########
@@ -108,7 +108,27 @@ case class UserDefinedPythonFunction(
       }
       PythonAggregate(name, func, dataType, e, udfDeterministic, bufferStruct)
     } else {
-      PythonUDF(name, func, dataType, e, pythonEvalType, udfDeterministic)
+      val conf = SQLConf.get
+      val resolvedDataType = if (conf.charVarcharFirstClassTypes) {
+        dataType
+      } else {
+        CharVarcharUtils.replaceCharVarcharWithString(dataType)
+      }
+      val charVarcharCheckedResultType =
+        if (CharVarcharUtils.shouldApplyWriteSideLengthCheck(conf) &&
+            CharVarcharUtils.hasCharVarchar(dataType)) {
+          Some(dataType)
+        } else {
+          None
+        }
+      PythonUDF(
+        name,
+        func,
+        resolvedDataType,
+        e,
+        pythonEvalType,
+        udfDeterministic,
+        charVarcharCheckedResultType = charVarcharCheckedResultType)

Review Comment:
   Fixed in 69ef259f915. `ConvertToCatalyst` now retains the original Python 
expression when its `PythonUDF` owns checked CHAR/VARCHAR semantics, so 
transpilation cannot bypass padding or overflow validation. Added focused 
optimizer coverage.



-- 
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]

Reply via email to