srielau commented on code in PR #58549:
URL: https://github.com/apache/spark/pull/58549#discussion_r4100081697
##########
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)) {
Review Comment:
Fixed in 732caa3f788. The JVM UDF builder now rejects CHAR/VARCHAR hidden in
UDT storage before eval-type dispatch, using a dedicated recursive
`hasCharVarcharInUDT` check and a classed error. The focused builder regression
passes and covers the authoritative boundary used by Connect reconstruction.
##########
python/pyspark/sql/pandas/types.py:
##########
@@ -157,7 +159,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 732caa3f788. `SparkArrowCStreamer` now recursively rejects
CHAR/VARCHAR, including UDT storage, before the shared Arrow mapper runs. Added
direct nested-schema and UDT-storage C-stream regressions.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/python/ColumnarArrowEvalPythonEvaluatorFactory.scala:
##########
@@ -212,10 +252,11 @@ private[python] class
ColumnarArrowEvalPythonEvaluatorFactory(
val numRows = resultBatch.numRows()
val resultCols = (0 until resultBatch.numCols()).map(
i => resultBatch.column(i)).toArray
- val (passThruCols, passThruRows) = passThruQueue.poll()
+ val passThruRows = passThruQueue.peek()._2
Review Comment:
I checked this against the merge base: the one-input-batch/one-output-batch
assertion and iterator eligibility in this Arrow-columnar path both predate
SPARK-59275. The issue is valid, but it is independent of CHAR/VARCHAR and
changing iterator rebatching here would violate the PR scope fence. I am
leaving it for a separate non-SPARK-59275 issue/PR.
##########
python/pyspark/sql/pandas/types.py:
##########
@@ -959,7 +961,7 @@ def _to_corrected_pandas_type(dt: DataType) ->
Optional[Any]:
return np.dtype("timedelta64[ns]")
else:
return np.dtype("timedelta64[us]")
- elif isinstance(dt, StringType):
+ elif isinstance(dt, (StringType, CharType, VarcharType)):
Review Comment:
Fixed in 732caa3f788 by restoring `_to_corrected_pandas_type` to its pre-PR
StringType-only behavior. CHAR/VARCHAR Arrow transport remains supported, but
this PR no longer changes `toPandas()` dtype correction.
##########
python/pyspark/sql/pandas/types.py:
##########
@@ -988,7 +990,7 @@ def _to_corrected_pandas_ext_type(dt: DataType) ->
Optional[Any]:
return pd.Float64Dtype()
elif isinstance(dt, BooleanType):
return pd.BooleanDtype()
- elif isinstance(dt, StringType):
+ elif isinstance(dt, (StringType, CharType, VarcharType)):
Review Comment:
Fixed in 732caa3f788. The unreachable CHAR/VARCHAR extension-dtype branch
was removed by restoring `_to_corrected_pandas_ext_type` to StringType only.
--
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]
