You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "srielau (via GitHub)" <gi...@apache.org> on 2026/09/07 23:44:12 UTC
[PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
srielau opened a new pull request, #58584:
URL: https://github.com/apache/spark/pull/58584
### What changes were proposed in this pull request?
Support first-class CHAR/VARCHAR across Hive extension boundaries when first-class semantics are enabled:
- Convert Catalyst CHAR/VARCHAR to Hive CHAR/VARCHAR object inspectors while preserving length.
- Convert Hive CHAR/VARCHAR inspectors back to first-class Catalyst types for UDF, UDAF, and UDTF results.
- Wrap Java and writable Hive CHAR/VARCHAR values using their declared lengths.
- Preserve constant and nested CHAR/VARCHAR inspector shapes.
- Support CHAR/VARCHAR output from script TRANSFORM with and without Hive SerDe.
- Continue exposing Hive CHAR/VARCHAR results as STRING under legacy semantics.
Hive object inspectors cannot carry Spark collation metadata. Collated inputs are accepted, while Hive function results retain the CHAR/VARCHAR length with the default collation.
JIRA: https://issues.apache.org/jira/browse/SPARK-59277
### Why are the changes needed?
With `spark.sql.charVarchar.standardSemantics.enabled=true`, Catalyst keeps CHAR/VARCHAR as first-class types. Hive extension paths still handled them as unsupported types or downgraded Hive CHAR/VARCHAR results to STRING. This prevented Hive UDF/UDAF/UDTF functions and script TRANSFORM from operating consistently on scalar, collated, and nested CHAR/VARCHAR values.
### Does this PR introduce _any_ user-facing change?
Yes. When first-class CHAR/VARCHAR semantics are enabled, Hive UDF/UDAF/UDTF functions and script TRANSFORM now accept and return CHAR/VARCHAR values while preserving declared lengths. Legacy flag-off behavior remains unchanged.
### How was this patch tested?
Added focused coverage for:
- Java, writable, constant, collated, and nested Hive object inspectors.
- Legacy STRING fallback.
- Hive scalar UDF, UDAF, and UDTF execution.
- Script TRANSFORM with Hive SerDe and without SerDe.
Ran:
```
sbt -java-home /usr/lib/jvm/java-17-openjdk-amd64 \
-Dsbt.override.build.repos=true \
'hive/testOnly org.apache.spark.sql.hive.HiveInspectorSuite org.apache.spark.sql.hive.execution.HiveUDFSuite org.apache.spark.sql.hive.execution.HiveUDAFSuite org.apache.spark.sql.hive.execution.HiveScriptTransformationSuite -- -z SPARK-59277'
sbt -java-home /usr/lib/jvm/java-17-openjdk-amd64 \
-Dsbt.override.build.repos=true \
'sql/testOnly org.apache.spark.sql.execution.SparkScriptTransformationSuite -- -z SPARK-59277'
dev/scalastyle sql
```
All 10 focused tests and Scala style checks passed.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Cursor Auto
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r3992124832
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala:
##########
@@ -245,7 +250,7 @@ private[hive] case class HiveGenericUDTF(
private lazy val wrappers = children.map(x => wrapperFor(toInspector(x), x.dataType)).toArray
@transient
- private lazy val unwrapper = unwrapperFor(outputInspector)
+ private lazy val unwrapper = unwrapperFor(outputInspector, elementSchema)
Review Comment:
**Non-blocking (P2):** `GenerateExec` binds this generator with `BindReferences`, which calls `withNewChildrenInternal` and creates a copy whose lazy `elementSchema` is unset. For a persisted view created with first-class CHAR/VARCHAR and queried under legacy settings, that copy can rebuild the runtime unwrapper with STRING targets, losing CHAR padding and length checks even though analysis used the captured schema. Please retain the resolved element schema across copies, as the GenericUDF and UDAF paths now retain their analyzed types, and add the corresponding opposite-config persisted-view UDTF coverage.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -897,6 +953,19 @@ private[hive] trait HiveInspectors {
(value: Any, row: InternalRow, ordinal: Int) => row(ordinal) = unwrapper(value)
}
+ /**
+ * Builds an in-place unwrapper that also honors target-type-specific conversions.
+ */
+ def unwrapperFor(
+ field: HiveStructField,
+ dataType: DataType): (Any, InternalRow, Int) => Unit = dataType match {
+ case _: CharType | _: VarcharType =>
+ val unwrapper = unwrapperFor(field.getFieldObjectInspector, dataType)
+ (value: Any, row: InternalRow, ordinal: Int) => row(ordinal) = unwrapper(value)
+ case _ =>
Review Comment:
**Non-blocking (P2):** This fallback drops the Catalyst child types for ARRAY/MAP/STRUCT outputs, even though the ObjectInspector/DataType overload above already recurses with them. A SerDe exposing a nested generic string inspector can therefore return an unpadded CHAR value or let an over-length CHAR/VARCHAR value through without `EXCEED_LIMIT_LENGTH`. Please select the target-aware unwrapper whenever the declared field type contains CHAR/VARCHAR recursively, while retaining the specialized field setters for types that do not.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor closed pull request #58584: [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions
URL: https://github.com/apache/spark/pull/58584
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4008642285
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDAFSuite.scala:
##########
@@ -200,6 +201,25 @@ class HiveUDAFSuite extends QueryTest
}
}
+ test("SPARK-59277: Hive UDAF supports first-class CHAR/VARCHAR") {
Review Comment:
**Non-blocking (P2):** Could we add a UDAF fixture whose PARTIAL1 inspector type differs from FINAL? `resolvedDataTypes` is an ordered `(partial, final)` pair: `AggregationBufferSerDe` uses the first entry while result conversion uses the second. `GenericUDAFMax` exposes the same type in both modes, so a tuple swap or use of the final type for the shuffle buffer would still pass this test. A distinct-type fixture would protect the serialization contract added here.
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveInspectorSuite.scala:
##########
@@ -292,6 +300,140 @@ class HiveInspectorSuite extends SparkFunSuite with HiveInspectors {
assert(typeInfo2.scale() === 10)
}
+ test("SPARK-59277: Hive object inspectors preserve CHAR/VARCHAR type information") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](CharType(5), VarcharType(7)).foreach { dataType =>
+ val inspector = toInspector(dataType).asInstanceOf[PrimitiveObjectInspector]
+ assert(inspectorToDataType(inspector) === dataType)
+ dataType match {
+ case c: CharType =>
+ assert(inspector.getTypeInfo.asInstanceOf[CharTypeInfo].getLength === c.length)
+ case v: VarcharType =>
+ assert(inspector.getTypeInfo.asInstanceOf[VarcharTypeInfo].getLength === v.length)
+ }
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive object inspectors accept collated CHAR/VARCHAR values") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](
+ CharType(5, "UTF8_LCASE"),
+ VarcharType(7, "UNICODE_CI")).foreach { dataType =>
+ val inspector = toInspector(dataType)
+ val value = UTF8String.fromString(dataType match {
+ case _: CharType => "ab"
+ case _: VarcharType => "abc"
+ })
+ val expectedValue = dataType match {
+ case _: CharType => UTF8String.fromString("ab ")
+ case _: VarcharType => value
+ }
+ val expectedType = dataType match {
+ case c: CharType => CharType(c.length)
+ case v: VarcharType => VarcharType(v.length)
+ }
+ assert(inspectorToDataType(inspector) === expectedType)
+ assert(unwrap(wrap(value, inspector, dataType), inspector) === expectedValue)
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive object inspectors support nested CHAR/VARCHAR") {
+ withFirstClassCharVarchar(enabled = true) {
+ val dataType = StructType(Seq(
+ StructField("chars", ArrayType(CharType(4))),
+ StructField("varchars", MapType(IntegerType, VarcharType(8)))))
+ val inspector = toInspector(dataType)
+ assert(inspectorToDataType(inspector) === dataType)
+
+ val input = InternalRow(
+ new GenericArrayData(Array[Any](UTF8String.fromString("a"))),
+ ArrayBasedMapData(
+ Array[Any](1),
+ Array[Any](UTF8String.fromString("value"))))
+ val result = unwrapperFor(inspector, dataType)(
+ wrap(input, inspector, dataType)).asInstanceOf[InternalRow]
+ assert(result.getArray(0).getUTF8String(0) === UTF8String.fromString("a "))
+ assert(result.getMap(1).valueArray().getUTF8String(0) === UTF8String.fromString("value"))
+
+ val outerType = StructType(Seq(StructField("nested", dataType)))
+ val outerInspector = toInspector(outerType).asInstanceOf[StructObjectInspector]
+ val field = outerInspector.getAllStructFieldRefs.get(0)
+ val targetRow = new SpecificInternalRow(Seq(dataType))
+ unwrapperFor(field, dataType)(wrap(input, inspector, dataType), targetRow, 0)
+ val nestedResult = targetRow.getStruct(0, dataType.length)
+ assert(nestedResult.getArray(0).getUTF8String(0) === UTF8String.fromString("a "))
+ assert(
+ nestedResult.getMap(1).valueArray().getUTF8String(0) === UTF8String.fromString("value"))
+ }
+ }
+
+ test("SPARK-59277: Hive constant inspectors preserve CHAR/VARCHAR type information") {
Review Comment:
**Non-blocking (P2):** Please also exercise null values here. Both new constant-inspector helpers have explicit `value == null` branches, but this loop only passes `"abc"`, and the older constant-null matrix does not include `CharType` or `VarcharType`. Without a null case, removing either branch can turn a valid NULL literal into a cast/construction failure without failing this suite.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -897,6 +953,19 @@ private[hive] trait HiveInspectors {
(value: Any, row: InternalRow, ordinal: Int) => row(ordinal) = unwrapper(value)
}
+ /**
+ * Builds an in-place unwrapper that also honors target-type-specific conversions.
Review Comment:
**Nit (P3):** Could we narrow this comment to say that the overload honors CHAR/VARCHAR-specific conversion? The implementation only calls the DataType-aware unwrapper when `hasCharVarchar(dataType)`; nanosecond timestamp targets, for example, still fall through to `unwrapperFor(field)` even though the adjacent overload treats them specially. As written, this promises a broader contract than the method implements.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFEvaluators.scala:
##########
@@ -162,9 +162,12 @@ class HiveGenericUDFEvaluator(
}
@transient
- private lazy val unwrapper: Any => Any = unwrapperFor(returnInspector)
+ private lazy val catalystReturnType = inspectorToDataType(returnInspector)
Review Comment:
Confirmed. The analyzed GenericUDF and UDAF types now survive copying and evaluator reconstruction, and the persisted-view test covers both opposite-configuration directions. Thanks for the fix.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:3962204053","thread_id":"inline:3962204053","verdict_sha256":"0c5976318b24590c378a566e055ad8eeb77ad8abf531e89f57f1b5020c23e5c9"} -->
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala:
##########
@@ -892,6 +892,42 @@ class HiveUDFSuite extends QueryTest with TestHiveSingleton {
hiveContext.reset()
}
+ test("SPARK-59277: Hive UDF and UDTF support first-class CHAR/VARCHAR") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
Review Comment:
Confirmed. The preserve-only case now disables standard semantics and asserts both CharType(5) and the padded value. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:3962204063","thread_id":"inline:3962204063","verdict_sha256":"0c5976318b24590c378a566e055ad8eeb77ad8abf531e89f57f1b5020c23e5c9"} -->
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -829,6 +864,26 @@ private[hive] trait HiveInspectors {
null
}
}
+ case (_, c: CharType) =>
Review Comment:
Confirmed. The typed unwrapper Scaladoc now documents the CHAR/VARCHAR length and padding checks. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:3962204070","thread_id":"inline:3962204070","verdict_sha256":"0c5976318b24590c378a566e055ad8eeb77ad8abf531e89f57f1b5020c23e5c9"} -->
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveScriptTransformationExec.scala:
##########
@@ -130,7 +132,7 @@ private[hive] case class HiveScriptTransformationExec(
if (dataList.get(i) == null) {
mutableRow.setNullAt(i)
} else {
- unwrappers(i)(dataList.get(i), mutableRow, i)
+ mutableRow.update(i, unwrappers(i)(dataList.get(i)))
Review Comment:
Confirmed. Primitive fields retain their specialized setters, while CHAR/VARCHAR-bearing fields use the target-aware path. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:3962204075","thread_id":"inline:3962204075","verdict_sha256":"0c5976318b24590c378a566e055ad8eeb77ad8abf531e89f57f1b5020c23e5c9"} -->
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -897,6 +953,19 @@ private[hive] trait HiveInspectors {
(value: Any, row: InternalRow, ordinal: Int) => row(ordinal) = unwrapper(value)
}
+ /**
+ * Builds an in-place unwrapper that also honors target-type-specific conversions.
+ */
+ def unwrapperFor(
+ field: HiveStructField,
+ dataType: DataType): (Any, InternalRow, Int) => Unit = dataType match {
+ case _: CharType | _: VarcharType =>
+ val unwrapper = unwrapperFor(field.getFieldObjectInspector, dataType)
+ (value: Any, row: InternalRow, ordinal: Int) => row(ordinal) = unwrapper(value)
+ case _ =>
Review Comment:
Confirmed. Nested CHAR/VARCHAR field types now select recursive target-aware conversion, and the focused test exercises that in-place path. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:3992124822","thread_id":"inline:3992124822","verdict_sha256":"0c5976318b24590c378a566e055ad8eeb77ad8abf531e89f57f1b5020c23e5c9"} -->
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4054082215
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/BaseScriptTransformationSuite.scala:
##########
@@ -86,6 +86,217 @@ abstract class BaseScriptTransformationSuite extends QueryTest {
assert(uncaughtExceptionHandler.exception.isEmpty)
}
+ test("SPARK-59277: TRANSFORM output supports first-class CHAR/VARCHAR without SerDe") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val input = Seq(("ab", "xyz")).toDF("c", "v")
+ checkAnswer(
+ input,
+ (child: SparkPlan) => createScriptTransformationExec(
+ script = "cat",
+ output = Seq(
+ AttributeReference("c", CharType(4, "UTF8_LCASE"))(),
+ AttributeReference("v", VarcharType(5, "UNICODE_CI"))()),
+ child = child,
+ ioschema = defaultIOSchema),
+ Seq(Row("ab ", "xyz")))
+ }
+ assert(uncaughtExceptionHandler.exception.isEmpty)
+ }
+
+ test("SPARK-59277: TRANSFORM CHAR overflow without SerDe raises EXCEED_LIMIT_LENGTH") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val input = Seq("abcdef").toDF("c")
+ val exception = intercept[Exception] {
+ QueryTest.executePlan(
+ createScriptTransformationExec(
+ script = "cat",
+ output = Seq(AttributeReference("c", CharType(4))()),
+ child = input.queryExecution.sparkPlan,
+ ioschema = defaultIOSchema),
+ spark.sqlContext)
+ }
+ val runtimeException = exception match {
+ case s: org.apache.spark.SparkRuntimeException => s
+ case other =>
+ other.getCause.asInstanceOf[org.apache.spark.SparkRuntimeException]
+ }
+ checkError(
+ exception = runtimeException,
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "4"))
+ }
+ }
+
+ test("SPARK-59277: TRANSFORM VARCHAR overflow without SerDe raises EXCEED_LIMIT_LENGTH") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val input = Seq("abcdefgh").toDF("v")
+ val exception = intercept[Exception] {
+ QueryTest.executePlan(
+ createScriptTransformationExec(
+ script = "cat",
+ output = Seq(AttributeReference("v", VarcharType(5))()),
+ child = input.queryExecution.sparkPlan,
+ ioschema = defaultIOSchema),
+ spark.sqlContext)
+ }
+ val runtimeException = exception match {
+ case s: org.apache.spark.SparkRuntimeException => s
+ case other =>
+ other.getCause.asInstanceOf[org.apache.spark.SparkRuntimeException]
+ }
+ checkError(
+ exception = runtimeException,
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "5"))
+ }
+ }
+
+ test("SPARK-59277: TRANSFORM converts nested CHAR/VARCHAR without SerDe") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ Seq(
+ ("""["ab"]""", ArrayType(CharType(4)), Row(Seq("ab "))),
+ ("""["xy"]""", ArrayType(VarcharType(4)), Row(Seq("xy"))),
+ (
+ """{"1":"ab"}""",
+ MapType(IntegerType, CharType(4)),
+ Row(Map(1 -> "ab "))),
+ (
+ """{"1":{"2":"ab"}}""",
+ MapType(IntegerType, MapType(IntegerType, CharType(4))),
+ Row(Map(1 -> Map(2 -> "ab ")))),
+ (
+ """[{"1":"ab"}]""",
+ ArrayType(MapType(IntegerType, CharType(4))),
+ Row(Seq(Map(1 -> "ab ")))),
+ (
+ """{"m":{"1":"ab"}}""",
+ StructType(Seq(StructField("m", MapType(IntegerType, CharType(4))))),
+ Row(Row(Map(1 -> "ab ")))),
+ (
+ """{"value":"xy"}""",
+ StructType(Seq(StructField("value", CharType(5)))),
+ Row(Row("xy ")))).foreach { case (json, dataType, expected) =>
+ val input = Seq(json).toDF("value")
+ checkAnswer(
+ input,
+ (child: SparkPlan) => createScriptTransformationExec(
+ script = "cat",
+ output = Seq(AttributeReference("value", dataType)()),
+ child = child,
+ ioschema = defaultIOSchema),
+ Seq(expected))
+ }
+ }
+ assert(uncaughtExceptionHandler.exception.isEmpty)
+ }
+
+ test("SPARK-59277: TRANSFORM nested CHAR/VARCHAR overflow without SerDe") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ Seq(
+ (ArrayType(CharType(4)), """["abcdef"]"""),
+ (MapType(IntegerType, CharType(4)), """{"1":"abcdef"}"""),
+ (
+ MapType(IntegerType, MapType(IntegerType, CharType(4))),
+ """{"1":{"2":"abcdef"}}"""),
+ (
+ StructType(Seq(StructField("value", VarcharType(4)))),
+ """{"value":"abcdef"}""")).foreach { case (dataType, json) =>
+ val input = Seq(json).toDF("value")
+ val exception = intercept[Exception] {
+ QueryTest.executePlan(
+ createScriptTransformationExec(
+ script = "cat",
+ output = Seq(AttributeReference("value", dataType)()),
+ child = input.queryExecution.sparkPlan,
+ ioschema = defaultIOSchema),
+ spark.sqlContext)
+ }
+ val runtimeException = exception match {
+ case s: org.apache.spark.SparkRuntimeException => s
+ case other =>
+ other.getCause.asInstanceOf[org.apache.spark.SparkRuntimeException]
+ }
+ checkError(
+ exception = runtimeException,
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "4"))
+ }
+ }
+ }
+
+ test("SPARK-59277: malformed nested CHAR JSON without SerDe returns null") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val input = Seq("""{"1":""").toDF("value")
+ checkAnswer(
+ input,
+ (child: SparkPlan) => createScriptTransformationExec(
+ script = "cat",
+ output = Seq(
+ AttributeReference("value", MapType(IntegerType, CharType(4)))()),
+ child = child,
+ ioschema = defaultIOSchema),
+ Seq(Row(null)))
+ }
+ assert(uncaughtExceptionHandler.exception.isEmpty)
+ }
+
+ test("SPARK-59277: TRANSFORM validates restored JSON map keys without SerDe") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val mapType = MapType(IntegerType, CharType(4))
+ Seq(
+ ("""{"1":"ab"}""", mapType, Row(Map(1 -> "ab "))),
+ ("""{"not-an-int":"ab"}""", mapType, Row(null)),
+ ("""{"1":"a","01":"b"}""", mapType, Row(null)),
+ (
+ """[{"not-an-int":"ab"}]""",
+ ArrayType(mapType),
+ Row(null)),
+ (
+ """{"m":{"1":"a","01":"b"}}""",
+ StructType(Seq(StructField("m", mapType))),
+ Row(null))).foreach { case (json, dataType, expected) =>
+ val input = Seq(json).toDF("value")
+ checkAnswer(
+ input,
+ (child: SparkPlan) => createScriptTransformationExec(
+ script = "cat",
+ output = Seq(AttributeReference("value", dataType)()),
+ child = child,
+ ioschema = defaultIOSchema),
+ Seq(expected))
+ }
+ }
+ assert(uncaughtExceptionHandler.exception.isEmpty)
+ }
+
+ test("SPARK-59277: colliding map key followed by valid row without SerDe") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val mapType = MapType(IntegerType, CharType(4))
+ // Row 1 has duplicate converted keys (1 and 01 both cast to 1).
+ // Row 2 is valid. Both rows are in the same partition.
+ val input = Seq(
+ """{"1":"a","01":"b"}""",
+ """{"2":"cd"}""").toDF("value")
Review Comment:
This does not enforce the same-partition condition that the regression needs. `Seq(...).toDF` produces a `LocalTableScanExec`, which uses `min(rowCount, leafNodeDefaultParallelism)` partitions; under this suite's `local[2]` session, these two rows normally run in separate tasks, so the test would also pass with the original shared-builder bug. Please force the input into one partition.
```suggestion
"""{"2":"cd"}""").toDF("value").coalesce(1)
```
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -376,6 +424,98 @@ case class ScriptTransformationIOSchema(
}
object ScriptTransformationIOSchema {
+ private[sql] def toUnboundedStringType(dataType: DataType): DataType = {
+ dataType.transformRecursively {
+ case c: CharType => c.toStringType
+ case v: VarcharType => v.toStringType
+ }
+ }
+
+ // JSON object keys are always strings. Rewrite every map key, including nested maps.
+ // `transformRecursively` would stop at the first matching MapType and skip children.
+ private[sql] def toJsonMapKeyType(dataType: DataType): DataType = dataType match {
+ case ArrayType(et, n) => ArrayType(toJsonMapKeyType(et), n)
+ case MapType(kt, vt, n) =>
+ val jsonKey = if (kt.isInstanceOf[StringType]) kt else StringType
+ MapType(jsonKey, toJsonMapKeyType(vt), n)
+ case StructType(fields) =>
+ StructType(fields.map(f => f.copy(dataType = toJsonMapKeyType(f.dataType))))
+ case other => other
+ }
+
+ /**
+ * Build a per-call map-key restorer that converts parsed JSON string keys
+ * back to the declared physical key type and validates the result through a
+ * fresh [[ArrayBasedMapBuilder]] on every invocation, so a failed or
+ * duplicate key cannot leave shared state dirty for the next row.
+ */
+ private[sql] def makeJsonMapKeyRestorer(
+ jsonType: DataType,
Review Comment:
This signature accepts `jsonType` and `targetType` independently, but the recursive implementation assumes matching container topology. In the struct branch, `zip` can truncate the restorers and the subsequent loop through `ts.length` can produce a cryptic index error; other shape mismatches silently return `identity`. Since the only caller derives the JSON type with `toJsonMapKeyType(targetType)`, could we make that invariant structural by accepting only the target type and deriving the JSON type here, or validate mismatches with a clear error?
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -376,6 +424,98 @@ case class ScriptTransformationIOSchema(
}
object ScriptTransformationIOSchema {
+ private[sql] def toUnboundedStringType(dataType: DataType): DataType = {
+ dataType.transformRecursively {
+ case c: CharType => c.toStringType
+ case v: VarcharType => v.toStringType
+ }
+ }
+
+ // JSON object keys are always strings. Rewrite every map key, including nested maps.
+ // `transformRecursively` would stop at the first matching MapType and skip children.
+ private[sql] def toJsonMapKeyType(dataType: DataType): DataType = dataType match {
+ case ArrayType(et, n) => ArrayType(toJsonMapKeyType(et), n)
+ case MapType(kt, vt, n) =>
+ val jsonKey = if (kt.isInstanceOf[StringType]) kt else StringType
+ MapType(jsonKey, toJsonMapKeyType(vt), n)
+ case StructType(fields) =>
+ StructType(fields.map(f => f.copy(dataType = toJsonMapKeyType(f.dataType))))
+ case other => other
+ }
+
+ /**
+ * Build a per-call map-key restorer that converts parsed JSON string keys
+ * back to the declared physical key type and validates the result through a
+ * fresh [[ArrayBasedMapBuilder]] on every invocation, so a failed or
+ * duplicate key cannot leave shared state dirty for the next row.
Review Comment:
Clarify that the failure is in key conversion.
```suggestion
* fresh [[ArrayBasedMapBuilder]] on every invocation, so a failed key
* conversion or duplicate key cannot leave shared state dirty for the next row.
```
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -253,7 +301,7 @@ trait BaseScriptTransformationExec extends UnaryExecNode {
}
}
- // Keep consistent with Hive `LazySimpleSerde`, when there is a type case error, return null
+ // Keep consistent with Hive `LazySimpleSerDe`, when there is a type case error, return null
Review Comment:
`type case` should be `type cast`, and this can be phrased more directly.
```suggestion
// Match Hive `LazySimpleSerDe`: return null when a type cast fails.
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4054096910
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/BaseScriptTransformationSuite.scala:
##########
@@ -86,6 +86,217 @@ abstract class BaseScriptTransformationSuite extends QueryTest {
assert(uncaughtExceptionHandler.exception.isEmpty)
}
+ test("SPARK-59277: TRANSFORM output supports first-class CHAR/VARCHAR without SerDe") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val input = Seq(("ab", "xyz")).toDF("c", "v")
+ checkAnswer(
+ input,
+ (child: SparkPlan) => createScriptTransformationExec(
+ script = "cat",
+ output = Seq(
+ AttributeReference("c", CharType(4, "UTF8_LCASE"))(),
+ AttributeReference("v", VarcharType(5, "UNICODE_CI"))()),
+ child = child,
+ ioschema = defaultIOSchema),
+ Seq(Row("ab ", "xyz")))
+ }
+ assert(uncaughtExceptionHandler.exception.isEmpty)
+ }
+
+ test("SPARK-59277: TRANSFORM CHAR overflow without SerDe raises EXCEED_LIMIT_LENGTH") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val input = Seq("abcdef").toDF("c")
+ val exception = intercept[Exception] {
+ QueryTest.executePlan(
+ createScriptTransformationExec(
+ script = "cat",
+ output = Seq(AttributeReference("c", CharType(4))()),
+ child = input.queryExecution.sparkPlan,
+ ioschema = defaultIOSchema),
+ spark.sqlContext)
+ }
+ val runtimeException = exception match {
+ case s: org.apache.spark.SparkRuntimeException => s
+ case other =>
+ other.getCause.asInstanceOf[org.apache.spark.SparkRuntimeException]
+ }
+ checkError(
+ exception = runtimeException,
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "4"))
+ }
+ }
+
+ test("SPARK-59277: TRANSFORM VARCHAR overflow without SerDe raises EXCEED_LIMIT_LENGTH") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val input = Seq("abcdefgh").toDF("v")
+ val exception = intercept[Exception] {
+ QueryTest.executePlan(
+ createScriptTransformationExec(
+ script = "cat",
+ output = Seq(AttributeReference("v", VarcharType(5))()),
+ child = input.queryExecution.sparkPlan,
+ ioschema = defaultIOSchema),
+ spark.sqlContext)
+ }
+ val runtimeException = exception match {
+ case s: org.apache.spark.SparkRuntimeException => s
+ case other =>
+ other.getCause.asInstanceOf[org.apache.spark.SparkRuntimeException]
+ }
+ checkError(
+ exception = runtimeException,
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "5"))
+ }
+ }
+
+ test("SPARK-59277: TRANSFORM converts nested CHAR/VARCHAR without SerDe") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ Seq(
+ ("""["ab"]""", ArrayType(CharType(4)), Row(Seq("ab "))),
+ ("""["xy"]""", ArrayType(VarcharType(4)), Row(Seq("xy"))),
+ (
+ """{"1":"ab"}""",
+ MapType(IntegerType, CharType(4)),
+ Row(Map(1 -> "ab "))),
+ (
+ """{"1":{"2":"ab"}}""",
+ MapType(IntegerType, MapType(IntegerType, CharType(4))),
+ Row(Map(1 -> Map(2 -> "ab ")))),
+ (
+ """[{"1":"ab"}]""",
+ ArrayType(MapType(IntegerType, CharType(4))),
+ Row(Seq(Map(1 -> "ab ")))),
+ (
+ """{"m":{"1":"ab"}}""",
+ StructType(Seq(StructField("m", MapType(IntegerType, CharType(4))))),
+ Row(Row(Map(1 -> "ab ")))),
+ (
+ """{"value":"xy"}""",
+ StructType(Seq(StructField("value", CharType(5)))),
+ Row(Row("xy ")))).foreach { case (json, dataType, expected) =>
+ val input = Seq(json).toDF("value")
+ checkAnswer(
+ input,
+ (child: SparkPlan) => createScriptTransformationExec(
+ script = "cat",
+ output = Seq(AttributeReference("value", dataType)()),
+ child = child,
+ ioschema = defaultIOSchema),
+ Seq(expected))
+ }
+ }
+ assert(uncaughtExceptionHandler.exception.isEmpty)
+ }
+
+ test("SPARK-59277: TRANSFORM nested CHAR/VARCHAR overflow without SerDe") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ Seq(
+ (ArrayType(CharType(4)), """["abcdef"]"""),
+ (MapType(IntegerType, CharType(4)), """{"1":"abcdef"}"""),
+ (
+ MapType(IntegerType, MapType(IntegerType, CharType(4))),
+ """{"1":{"2":"abcdef"}}"""),
+ (
+ StructType(Seq(StructField("value", VarcharType(4)))),
+ """{"value":"abcdef"}""")).foreach { case (dataType, json) =>
+ val input = Seq(json).toDF("value")
+ val exception = intercept[Exception] {
+ QueryTest.executePlan(
+ createScriptTransformationExec(
+ script = "cat",
+ output = Seq(AttributeReference("value", dataType)()),
+ child = input.queryExecution.sparkPlan,
+ ioschema = defaultIOSchema),
+ spark.sqlContext)
+ }
+ val runtimeException = exception match {
+ case s: org.apache.spark.SparkRuntimeException => s
+ case other =>
+ other.getCause.asInstanceOf[org.apache.spark.SparkRuntimeException]
+ }
+ checkError(
+ exception = runtimeException,
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "4"))
+ }
+ }
+ }
+
+ test("SPARK-59277: malformed nested CHAR JSON without SerDe returns null") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val input = Seq("""{"1":""").toDF("value")
+ checkAnswer(
+ input,
+ (child: SparkPlan) => createScriptTransformationExec(
+ script = "cat",
+ output = Seq(
+ AttributeReference("value", MapType(IntegerType, CharType(4)))()),
+ child = child,
+ ioschema = defaultIOSchema),
+ Seq(Row(null)))
+ }
+ assert(uncaughtExceptionHandler.exception.isEmpty)
+ }
+
+ test("SPARK-59277: TRANSFORM validates restored JSON map keys without SerDe") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val mapType = MapType(IntegerType, CharType(4))
+ Seq(
+ ("""{"1":"ab"}""", mapType, Row(Map(1 -> "ab "))),
+ ("""{"not-an-int":"ab"}""", mapType, Row(null)),
+ ("""{"1":"a","01":"b"}""", mapType, Row(null)),
+ (
+ """[{"not-an-int":"ab"}]""",
+ ArrayType(mapType),
+ Row(null)),
+ (
+ """{"m":{"1":"a","01":"b"}}""",
+ StructType(Seq(StructField("m", mapType))),
+ Row(null))).foreach { case (json, dataType, expected) =>
+ val input = Seq(json).toDF("value")
+ checkAnswer(
+ input,
+ (child: SparkPlan) => createScriptTransformationExec(
+ script = "cat",
+ output = Seq(AttributeReference("value", dataType)()),
+ child = child,
+ ioschema = defaultIOSchema),
+ Seq(expected))
+ }
+ }
+ assert(uncaughtExceptionHandler.exception.isEmpty)
+ }
+
+ test("SPARK-59277: colliding map key followed by valid row without SerDe") {
+ assume(TestUtils.testCommandAvailable("/bin/bash"))
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val mapType = MapType(IntegerType, CharType(4))
+ // Row 1 has duplicate converted keys (1 and 01 both cast to 1).
+ // Row 2 is valid. Both rows are in the same partition.
+ val input = Seq(
+ """{"1":"a","01":"b"}""",
+ """{"2":"cd"}""").toDF("value")
Review Comment:
Added `.coalesce(1)` to force both rows into the same partition. Fixed in d9044026aa2.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -376,6 +424,98 @@ case class ScriptTransformationIOSchema(
}
object ScriptTransformationIOSchema {
+ private[sql] def toUnboundedStringType(dataType: DataType): DataType = {
+ dataType.transformRecursively {
+ case c: CharType => c.toStringType
+ case v: VarcharType => v.toStringType
+ }
+ }
+
+ // JSON object keys are always strings. Rewrite every map key, including nested maps.
+ // `transformRecursively` would stop at the first matching MapType and skip children.
+ private[sql] def toJsonMapKeyType(dataType: DataType): DataType = dataType match {
+ case ArrayType(et, n) => ArrayType(toJsonMapKeyType(et), n)
+ case MapType(kt, vt, n) =>
+ val jsonKey = if (kt.isInstanceOf[StringType]) kt else StringType
+ MapType(jsonKey, toJsonMapKeyType(vt), n)
+ case StructType(fields) =>
+ StructType(fields.map(f => f.copy(dataType = toJsonMapKeyType(f.dataType))))
+ case other => other
+ }
+
+ /**
+ * Build a per-call map-key restorer that converts parsed JSON string keys
+ * back to the declared physical key type and validates the result through a
+ * fresh [[ArrayBasedMapBuilder]] on every invocation, so a failed or
+ * duplicate key cannot leave shared state dirty for the next row.
+ */
+ private[sql] def makeJsonMapKeyRestorer(
+ jsonType: DataType,
Review Comment:
Tightened the signature to accept only the target type; the JSON type is now derived internally via `toJsonMapKeyType`. Fixed in d9044026aa2.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -376,6 +424,98 @@ case class ScriptTransformationIOSchema(
}
object ScriptTransformationIOSchema {
+ private[sql] def toUnboundedStringType(dataType: DataType): DataType = {
+ dataType.transformRecursively {
+ case c: CharType => c.toStringType
+ case v: VarcharType => v.toStringType
+ }
+ }
+
+ // JSON object keys are always strings. Rewrite every map key, including nested maps.
+ // `transformRecursively` would stop at the first matching MapType and skip children.
+ private[sql] def toJsonMapKeyType(dataType: DataType): DataType = dataType match {
+ case ArrayType(et, n) => ArrayType(toJsonMapKeyType(et), n)
+ case MapType(kt, vt, n) =>
+ val jsonKey = if (kt.isInstanceOf[StringType]) kt else StringType
+ MapType(jsonKey, toJsonMapKeyType(vt), n)
+ case StructType(fields) =>
+ StructType(fields.map(f => f.copy(dataType = toJsonMapKeyType(f.dataType))))
+ case other => other
+ }
+
+ /**
+ * Build a per-call map-key restorer that converts parsed JSON string keys
+ * back to the declared physical key type and validates the result through a
+ * fresh [[ArrayBasedMapBuilder]] on every invocation, so a failed or
+ * duplicate key cannot leave shared state dirty for the next row.
Review Comment:
Updated the Scaladoc to say "failed key conversion or duplicate key". Fixed in d9044026aa2.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -253,7 +301,7 @@ trait BaseScriptTransformationExec extends UnaryExecNode {
}
}
- // Keep consistent with Hive `LazySimpleSerde`, when there is a type case error, return null
+ // Keep consistent with Hive `LazySimpleSerDe`, when there is a type case error, return null
Review Comment:
Replaced with `Match Hive LazySimpleSerDe: return null when a type cast fails.` Fixed in d9044026aa2.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4063704195
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -829,6 +869,26 @@ private[hive] trait HiveInspectors {
null
}
}
+ case (_, c: CharType) =>
+ val unwrapper = unwrapperFor(objectInspector)
+ data: Any => {
+ val value = unwrapper(data).asInstanceOf[UTF8String]
+ if (value == null) {
+ null
+ } else {
+ CharVarcharCodegenUtils.charTypeReadSideCheck(value, c.length)
Review Comment:
The map unwrapper now builds through a fresh `ArrayBasedMapBuilder` when the key type contains CHAR/VARCHAR, so CHAR-padded key collisions follow `MAP_KEY_DEDUP_POLICY`. The fast `ArrayBasedMapData` path is retained for non-bounded key types. Fixed in a0fb4390ae3.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala:
##########
@@ -554,4 +592,77 @@ private[hive] case class HiveUDAFFunction(
copy(children = newChildren)
}
+object HiveUDAFFunction extends HiveInspectors {
+ private[hive] case class InitializedEvaluators(
+ partialEvaluator: GenericUDAFEvaluator,
+ partialInspector: ObjectInspector,
+ finalEvaluator: GenericUDAFEvaluator,
+ finalInspector: ObjectInspector)
+
+ def apply(
+ name: String,
+ funcWrapper: HiveFunctionWrapper,
+ children: Seq[Expression]): HiveUDAFFunction = {
+ apply(name, funcWrapper, children, isUDAFBridgeRequired = false)
+ }
+
+ def apply(
+ name: String,
+ funcWrapper: HiveFunctionWrapper,
+ children: Seq[Expression],
+ isUDAFBridgeRequired: Boolean): HiveUDAFFunction = {
+ val (partialType, resultType) =
+ inferResolvedTypes(funcWrapper, children, isUDAFBridgeRequired)
+ HiveUDAFFunction(
+ name,
+ funcWrapper,
+ children,
+ isUDAFBridgeRequired,
+ mutableAggBufferOffset = 0,
+ inputAggBufferOffset = 0,
+ partialType,
+ resultType)
+ }
+
+ private[hive] def initializeEvaluators(
+ funcWrapper: HiveFunctionWrapper,
+ children: Seq[Expression],
+ isUDAFBridgeRequired: Boolean,
+ expectedPartialType: Option[DataType] = None,
+ expectedResultType: Option[DataType] = None): InitializedEvaluators = {
+ val inputInspectors = children.map(toInspector).toArray
+ def newEvaluator(): GenericUDAFEvaluator = {
+ val resolver = if (isUDAFBridgeRequired) {
+ new SparkGenericUDAFBridge(funcWrapper.createFunction[UDAF]())
+ } else {
+ funcWrapper.createFunction[AbstractGenericUDAFResolver]()
+ }
+ val parameterInfo = new SimpleGenericUDAFParameterInfo(
+ inputInspectors, false, false, false)
+ resolver.getEvaluator(parameterInfo)
+ }
+ val partial1 = newEvaluator()
+ val partialInspector = partial1.init(GenericUDAFEvaluator.Mode.PARTIAL1, inputInspectors)
+ val finalEvaluator = newEvaluator()
+ val finalInspector =
+ finalEvaluator.init(GenericUDAFEvaluator.Mode.FINAL, Array(partialInspector))
+ expectedPartialType.foreach(checkCompatibleHiveReturnType(partialInspector, _))
Review Comment:
Added two focused tests: one supplies `LongType` as expected partial (runtime returns STRING - incompatible), the other supplies `VarcharType(5)` as expected final (runtime returns `CHAR(5)` - different bounded kind). Both intercept the `SparkException` from `checkCompatibleHiveReturnType`. Removing either check at the call site fails the corresponding test. Fixed in a0fb4390ae3.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFEvaluators.scala:
##########
@@ -155,16 +157,42 @@ class HiveGenericUDFEvaluator(
oi
}
}
+}
+
+private[hive] class HiveGenericUDFEvaluator(
+ funcWrapper: HiveFunctionWrapper,
+ children: Seq[Expression],
+ catalystReturnType: DataType)
+ extends HiveUDFEvaluatorBase[GenericUDF](funcWrapper, children) {
+
+ // SPARK-58792: copied expression nodes (e.g. via withNewChildrenInternal) share one
+ // HiveFunctionWrapper, whose cached GenericUDF instance is mutable: initialize()
+ // rewrites its converters and output holders based on the arguments of whichever
+ // copy initialized it last. Give every evaluator its own clone so copied nodes
+ // cannot corrupt each other.
+ @transient
+ override lazy val function: GenericUDF =
+ HiveFunctionRegistryUtils.cloneGenericUDF(funcWrapper.createFunction[GenericUDF]())
+
+ @transient
+ private lazy val argumentInspectors = children.map(toInspector).toArray
+
+ @transient
+ lazy val returnInspector = {
+ val inspector = HiveGenericUDFEvaluator.initialize(function, argumentInspectors)
+ checkCompatibleHiveReturnType(inspector, catalystReturnType)
Review Comment:
Added two focused tests: one constructs a `HiveGenericUDF` with `catalystReturnType = VarcharType(5)` but the UDF returns CHAR(5) at runtime; the other constructs a `HiveGenericUDTF` with an element schema claiming `VarcharType(5)` but the UDTF returns CHAR(5). Both intercept the `SparkException`. Removing either `checkCompatibleHiveReturnType` call fails its test. Fixed in a0fb4390ae3.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4068179406
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -829,6 +869,26 @@ private[hive] trait HiveInspectors {
null
}
}
+ case (_, c: CharType) =>
Review Comment:
**Non-blocking (P2):** This typed unwrapper is also used by Hive ORC whenever a complex field contains a nanosecond timestamp. With preserve-only types and `spark.sql.readSideCharPadding=false`, recursion now reaches this unconditional CHAR branch and pads an adjacent nested value anyway; the same CHAR remains raw when the unrelated nanos sibling is absent. Please separate the ORC nanos-only conversion policy from the strict Hive-function CHAR/VARCHAR policy, propagate that choice through nested containers, and cover this disabled-padding ORC interaction.
**Recommended change:** Separate nanos-only recursive conversion from Hive-function bounded-string enforcement, make the selected policy propagate through array/map/struct recursion, route the Hive ORC nanos path through the nanos-only policy, and add caller-level coverage for preserve-only read padding disabled alongside nested nanos timestamps.
**Why this works:** Introduce an explicit internal conversion policy or distinct recursive entry point in HiveInspectors. Hive function evaluators select the policy that applies CHAR/VARCHAR read checks and validated bounded map-key conversion. Hive ORC selects the policy that preserves nanos timestamps but delegates CHAR/VARCHAR leaves to raw untyped extraction; ApplyCharTypePadding remains the authority that conditionally pads table reads. Ensure every recursive container call carries the selected policy rather than falling back to the all-checks default.
**Scope:** Disentangle Hive function result validation from Hive ORC nanos decoding without changing either feature's supported type surface.
**Compatibility:** Legacy disabled-mode Hive function results remain unbounded StringType; enabled Hive function boundaries remain checked first-class CHAR/VARCHAR; Script TRANSFORM remains excluded.
**Risks:** A policy flag that is not propagated into nested arrays, maps, and structs would leave the regression in deeper shapes. Disabling bounded checks for Hive function callers would lose required padding, length enforcement, and normalized map-key validation. Changing the nanos timestamp arms could reintroduce microsecond truncation in Hive ORC.
**Constraints:** Hive UDF/UDAF/UDTF outputs analyzed as CHAR/VARCHAR must retain their declared kind, length, padding, default collation, and overflow behavior. Hive ORC must preserve TimestampNTZNanosType and TimestampLTZNanosType precision recursively. Table-read padding must continue to follow readSideCharPadding, with standard semantics overriding a disabled setting through ApplyCharTypePadding.
**Success:** In preserve-only mode with readSideCharPadding=false, a Hive ORC CHAR/VARCHAR value remains raw whether or not an adjacent nested field uses a nanos timestamp. When read-side padding is enabled or standard semantics requires it, table reads still expose the expected CHAR padding through the established logical padding rule. Hive UDF/UDAF/UDTF typed conversion continues to enforce bounded-string padding, overflow, runtime-inspector compatibility, and duplicate normalized map keys. Hive ORC recursively round-trips both nanos timestamp kinds without precision loss.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r3994222572
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -897,6 +953,19 @@ private[hive] trait HiveInspectors {
(value: Any, row: InternalRow, ordinal: Int) => row(ordinal) = unwrapper(value)
}
+ /**
+ * Builds an in-place unwrapper that also honors target-type-specific conversions.
+ */
+ def unwrapperFor(
+ field: HiveStructField,
+ dataType: DataType): (Any, InternalRow, Int) => Unit = dataType match {
+ case _: CharType | _: VarcharType =>
+ val unwrapper = unwrapperFor(field.getFieldObjectInspector, dataType)
+ (value: Any, row: InternalRow, ordinal: Int) => row(ordinal) = unwrapper(value)
+ case _ =>
Review Comment:
Fixed in c77094331c5. The in-place field unwrapper now selects target-aware recursive conversion whenever the declared type contains CHAR/VARCHAR, while primitive and unrelated fields retain their specialized setters. HiveInspectorSuite now exercises this nested field path directly.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala:
##########
@@ -245,7 +250,7 @@ private[hive] case class HiveGenericUDTF(
private lazy val wrappers = children.map(x => wrapperFor(toInspector(x), x.dataType)).toArray
@transient
- private lazy val unwrapper = unwrapperFor(outputInspector)
+ private lazy val unwrapper = unwrapperFor(outputInspector, elementSchema)
Review Comment:
Fixed in c77094331c5. HiveGenericUDTF now carries its resolved element schema as a case-class field and preserves it in withNewChildrenInternal. The persisted-view test now covers a first-class CHAR UDTF created and executed under opposite settings.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4059430586
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -829,6 +869,26 @@ private[hive] trait HiveInspectors {
null
}
}
+ case (_, c: CharType) =>
+ val unwrapper = unwrapperFor(objectInspector)
+ data: Any => {
+ val value = unwrapper(data).asInstanceOf[UTF8String]
+ if (value == null) {
+ null
+ } else {
+ CharVarcharCodegenUtils.charTypeReadSideCheck(value, c.length)
Review Comment:
Acknowledged. Will address the Hive map-key collision validation in this PR.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -241,6 +266,29 @@ trait BaseScriptTransformationExec extends UnaryExecNode {
data => IntervalUtils.microsToDuration(
IntervalUtils.castStringToDTInterval(UTF8String.fromString(data), start, end)),
converter)
+ case dt @ (_: ArrayType | _: MapType | _: StructType)
+ if CharVarcharUtils.hasCharVarchar(dt) =>
+ val physicalType = ScriptTransformationIOSchema.toUnboundedStringType(dt)
+ // JSON object keys are strings. Cast them to the declared map key type after parsing.
+ val jsonType = ScriptTransformationIOSchema.toJsonMapKeyType(physicalType)
+ val complexTypeFactory = JsonToStructs(
+ jsonType,
+ ioschema.outputSerdeProps.toMap,
+ Literal(null),
+ Some(conf.sessionLocalTimeZone))
+ val parsedToPhysical = if (jsonType.sameType(physicalType)) {
+ identity[Any] _
+ } else {
+ val restoreMapKeys = ScriptTransformationIOSchema.makeJsonMapKeyRestorer(
+ physicalType, Some(conf.sessionLocalTimeZone))
+ value: Any => restoreMapKeys(value)
+ }
+ val toScala = CatalystTypeConverters.createToScalaConverter(physicalType)
+ val parser = wrapperConvertException(
+ data => parsedToPhysical(
+ complexTypeFactory.nullSafeEval(UTF8String.fromString(data))),
+ identity)
+ data => converter(toScala(parser(data)))
Review Comment:
Moved to follow-up: script TRANSFORM CHAR/VARCHAR support has been split out per the review suggestion. The follow-up branch is at `serge-rielau_data/SPARK-59277-transform`.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala:
##########
@@ -554,4 +592,77 @@ private[hive] case class HiveUDAFFunction(
copy(children = newChildren)
}
+object HiveUDAFFunction extends HiveInspectors {
+ private[hive] case class InitializedEvaluators(
+ partialEvaluator: GenericUDAFEvaluator,
+ partialInspector: ObjectInspector,
+ finalEvaluator: GenericUDAFEvaluator,
+ finalInspector: ObjectInspector)
+
+ def apply(
+ name: String,
+ funcWrapper: HiveFunctionWrapper,
+ children: Seq[Expression]): HiveUDAFFunction = {
+ apply(name, funcWrapper, children, isUDAFBridgeRequired = false)
+ }
+
+ def apply(
+ name: String,
+ funcWrapper: HiveFunctionWrapper,
+ children: Seq[Expression],
+ isUDAFBridgeRequired: Boolean): HiveUDAFFunction = {
+ val (partialType, resultType) =
+ inferResolvedTypes(funcWrapper, children, isUDAFBridgeRequired)
+ HiveUDAFFunction(
+ name,
+ funcWrapper,
+ children,
+ isUDAFBridgeRequired,
+ mutableAggBufferOffset = 0,
+ inputAggBufferOffset = 0,
+ partialType,
+ resultType)
+ }
+
+ private[hive] def initializeEvaluators(
+ funcWrapper: HiveFunctionWrapper,
+ children: Seq[Expression],
+ isUDAFBridgeRequired: Boolean,
+ expectedPartialType: Option[DataType] = None,
+ expectedResultType: Option[DataType] = None): InitializedEvaluators = {
+ val inputInspectors = children.map(toInspector).toArray
+ def newEvaluator(): GenericUDAFEvaluator = {
+ val resolver = if (isUDAFBridgeRequired) {
+ new SparkGenericUDAFBridge(funcWrapper.createFunction[UDAF]())
+ } else {
+ funcWrapper.createFunction[AbstractGenericUDAFResolver]()
+ }
+ val parameterInfo = new SimpleGenericUDAFParameterInfo(
+ inputInspectors, false, false, false)
+ resolver.getEvaluator(parameterInfo)
+ }
+ val partial1 = newEvaluator()
+ val partialInspector = partial1.init(GenericUDAFEvaluator.Mode.PARTIAL1, inputInspectors)
+ val finalEvaluator = newEvaluator()
+ val finalInspector =
+ finalEvaluator.init(GenericUDAFEvaluator.Mode.FINAL, Array(partialInspector))
+ expectedPartialType.foreach(checkCompatibleHiveReturnType(partialInspector, _))
Review Comment:
Acknowledged. Will add incompatible-runtime-inspector fixtures for UDAF partial and final mismatches.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFEvaluators.scala:
##########
@@ -155,16 +157,42 @@ class HiveGenericUDFEvaluator(
oi
}
}
+}
+
+private[hive] class HiveGenericUDFEvaluator(
+ funcWrapper: HiveFunctionWrapper,
+ children: Seq[Expression],
+ catalystReturnType: DataType)
+ extends HiveUDFEvaluatorBase[GenericUDF](funcWrapper, children) {
+
+ // SPARK-58792: copied expression nodes (e.g. via withNewChildrenInternal) share one
+ // HiveFunctionWrapper, whose cached GenericUDF instance is mutable: initialize()
+ // rewrites its converters and output holders based on the arguments of whichever
+ // copy initialized it last. Give every evaluator its own clone so copied nodes
+ // cannot corrupt each other.
+ @transient
+ override lazy val function: GenericUDF =
+ HiveFunctionRegistryUtils.cloneGenericUDF(funcWrapper.createFunction[GenericUDF]())
+
+ @transient
+ private lazy val argumentInspectors = children.map(toInspector).toArray
+
+ @transient
+ lazy val returnInspector = {
+ val inspector = HiveGenericUDFEvaluator.initialize(function, argumentInspectors)
+ checkCompatibleHiveReturnType(inspector, catalystReturnType)
Review Comment:
Acknowledged. Will add custom GenericUDF and GenericUDTF fixtures whose runtime inspector is incompatible with the analysis snapshot.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -201,7 +218,15 @@ trait BaseScriptTransformationExec extends UnaryExecNode {
private lazy val outputFieldWriters: Seq[String => Any] = output.map { attr =>
val converter = CatalystTypeConverters.createToCatalystConverter(attr.dataType)
attr.dataType match {
- case StringType => wrapperConvertException(data => data, converter)
+ case _: CharType | _: VarcharType =>
+ // First-class CHAR/VARCHAR must not use Hive LazySimpleSerDe's null-on-error path.
+ (data: String) =>
+ if (data == ioschema.outputRowFormatMap("TOK_TABLEROWFORMATNULL")) {
Review Comment:
Moved to follow-up: script TRANSFORM CHAR/VARCHAR support has been split out per the review suggestion.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4008644379
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala:
##########
@@ -245,7 +250,7 @@ private[hive] case class HiveGenericUDTF(
private lazy val wrappers = children.map(x => wrapperFor(toInspector(x), x.dataType)).toArray
@transient
- private lazy val unwrapper = unwrapperFor(outputInspector)
+ private lazy val unwrapper = unwrapperFor(outputInspector, elementSchema)
Review Comment:
Confirmed. HiveGenericUDTF now carries the resolved element schema through child replacement, and the persisted-view test covers opposite-settings execution. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:3992124832","thread_id":"inline:3992124832","verdict_sha256":"0c5976318b24590c378a566e055ad8eeb77ad8abf531e89f57f1b5020c23e5c9"} -->
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4036387252
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFEvaluators.scala:
##########
@@ -111,34 +111,36 @@ class HiveSimpleUDFEvaluator(
}
}
-class HiveGenericUDFEvaluator(
- funcWrapper: HiveFunctionWrapper, children: Seq[Expression])
- extends HiveUDFEvaluatorBase[GenericUDF](funcWrapper, children) {
-
- // SPARK-58792: copied expression nodes (e.g. via withNewChildrenInternal) share one
- // HiveFunctionWrapper, whose cached GenericUDF instance is mutable: initialize()
- // rewrites its converters and output holders based on the arguments of whichever
- // copy initialized it last. Give every evaluator its own clone so copied nodes
- // cannot corrupt each other.
- @transient
- override lazy val function: GenericUDF =
- HiveFunctionRegistryUtils.cloneGenericUDF(funcWrapper.createFunction[GenericUDF]())
-
- @transient
- private lazy val argumentInspectors = children.map(toInspector).toArray
+private[hive] object HiveGenericUDFEvaluator extends HiveInspectors {
+
+ /**
+ * Driver-side Hive initialize for `SELECT hive_udf(...)`. Returns the Catalyst type
Review Comment:
**Nit (P3):** This should say `Driver-side Hive initialization` or explicitly name the `initialize` call. As written, the opening lifecycle sentence uses the verb as a noun and is unclear.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -201,7 +211,15 @@ trait BaseScriptTransformationExec extends UnaryExecNode {
private lazy val outputFieldWriters: Seq[String => Any] = output.map { attr =>
val converter = CatalystTypeConverters.createToCatalystConverter(attr.dataType)
attr.dataType match {
- case StringType => wrapperConvertException(data => data, converter)
+ case _: CharType | _: VarcharType =>
+ // First-class CHAR/VARCHAR must not use Hive LazySimpleSerde's null-on-error path.
Review Comment:
**Nit (P3):** Please spell this `LazySimpleSerDe`, matching the concrete Hive class and the name used elsewhere in the implementation and tests.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -241,6 +259,31 @@ trait BaseScriptTransformationExec extends UnaryExecNode {
data => IntervalUtils.microsToDuration(
IntervalUtils.castStringToDTInterval(UTF8String.fromString(data), start, end)),
converter)
+ case dt @ (_: ArrayType | _: MapType | _: StructType)
+ if CharVarcharUtils.hasCharVarchar(dt) =>
+ val physicalType = ScriptTransformationIOSchema.toUnboundedStringType(dt)
+ // JSON object keys are strings. Cast them to the declared map key type after parsing.
+ val jsonType = ScriptTransformationIOSchema.toJsonMapKeyType(physicalType)
+ val complexTypeFactory = JsonToStructs(
+ jsonType,
+ ioschema.outputSerdeProps.toMap,
+ Literal(null),
+ Some(conf.sessionLocalTimeZone))
+ val parsedToPhysical = if (jsonType.sameType(physicalType)) {
+ identity[Any] _
+ } else {
+ val cast = Cast(
Review Comment:
**Non-blocking (P2):** This directly evaluates a whole-map Cast after parsing every JSON object key as STRING. For a non-ANSI `MAP<INT, CHAR(4)>`, `{"not-an-int":"ab"}` can produce a null key, while `{"1":"a","01":"b"}` can produce duplicate integer keys. Because this bypasses the analyzer's map-key nullability check and the resulting MapData performs no validation, malformed script output can escape as invalid internal data.
**Recommended change:** Restore non-string JSON map keys recursively through a validated Catalyst map-construction path, treat failed or colliding key conversions as malformed script fields, and add focused valid, invalid, duplicate-producing, and nested-key coverage.
**Why this works:** After JsonToStructs parses string object keys, recursively convert each map's key array to the declared physical key type and rebuild the map through a constructor that enforces non-null and deduplicated keys. Evaluate this restoration inside the existing malformed-field wrapper, then run the existing declared CHAR/VARCHAR conversion outside it so length errors retain their specific signal.
**Scope:** Make the new no-SerDe nested CHAR/VARCHAR path restore JSON map keys without bypassing Catalyst map-key validity invariants.
**Compatibility:** Preserve valid no-SerDe complex TRANSFORM output, legacy null-on-parse-error handling, first-class CHAR/VARCHAR padding and length enforcement, and Hive SerDe behavior.
**Risks:** Moving CHAR/VARCHAR checks inside the generic malformed-field wrapper could accidentally convert required EXCEED_LIMIT_LENGTH failures to null. Rebuilding nested maps at only the outer level could leave invalid keys in maps nested under arrays, structs, or map values.
**Constraints:** Valid non-string map keys in CHAR/VARCHAR-bearing nested outputs must remain supported. CHAR/VARCHAR overflow must continue to raise EXCEED_LIMIT_LENGTH rather than being swallowed as a generic malformed field. Malformed non-CHAR/VARCHAR parse or key-conversion failures must retain the established null-on-error behavior where applicable.
**Success:** A failed non-string JSON map-key conversion cannot produce a null key in Catalyst MapData. Distinct JSON keys that convert to one declared key cannot produce undefined duplicate-key MapData. Valid top-level and nested non-string map keys retain their declared Catalyst types and values. Nested CHAR/VARCHAR padding and overflow behavior remains unchanged.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -897,6 +953,19 @@ private[hive] trait HiveInspectors {
(value: Any, row: InternalRow, ordinal: Int) => row(ordinal) = unwrapper(value)
}
+ /**
+ * Builds an in-place unwrapper that also honors target-type-specific conversions.
Review Comment:
Confirmed. In the current source, CHAR/VARCHAR-bearing and nanosecond-timestamp targets use the DataType-aware path, while unrelated primitive targets retain their specialized setters, so the comment and implementation now align. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:4008642276","thread_id":"inline:4008642276","verdict_sha256":"f604aded9a0ed9de1cf7f06c3acfeb8e4e13defa3c50d17e3ed7b9e2910309fe"} -->
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveInspectorSuite.scala:
##########
@@ -292,6 +301,240 @@ class HiveInspectorSuite extends SparkFunSuite with HiveInspectors {
assert(typeInfo2.scale() === 10)
}
+ test("SPARK-59277: Hive object inspectors preserve CHAR/VARCHAR type information") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](CharType(5), VarcharType(7)).foreach { dataType =>
+ val inspector = toInspector(dataType).asInstanceOf[PrimitiveObjectInspector]
+ assert(inspectorToDataType(inspector) === dataType)
+ dataType match {
+ case c: CharType =>
+ assert(inspector.getTypeInfo.asInstanceOf[CharTypeInfo].getLength === c.length)
+ case v: VarcharType =>
+ assert(inspector.getTypeInfo.asInstanceOf[VarcharTypeInfo].getLength === v.length)
+ }
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive object inspectors accept collated CHAR/VARCHAR values") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](
+ CharType(5, "UTF8_LCASE"),
+ VarcharType(7, "UNICODE_CI")).foreach { dataType =>
+ val inspector = toInspector(dataType)
+ val value = UTF8String.fromString(dataType match {
+ case _: CharType => "ab"
+ case _: VarcharType => "abc"
+ })
+ val expectedValue = dataType match {
+ case _: CharType => UTF8String.fromString("ab ")
+ case _: VarcharType => value
+ }
+ val expectedType = dataType match {
+ case c: CharType => CharType(c.length)
+ case v: VarcharType => VarcharType(v.length)
+ }
+ assert(inspectorToDataType(inspector) === expectedType)
+ assert(unwrap(wrap(value, inspector, dataType), inspector) === expectedValue)
+ }
+ }
+ }
+
+ test("SPARK-59277: writable CHAR/VARCHAR inspectors round-trip values and nulls") {
+ withFirstClassCharVarchar(enabled = true) {
+ val charType = CharType(5)
+ val charInspector = PrimitiveObjectInspectorFactory.getPrimitiveWritableObjectInspector(
+ new CharTypeInfo(charType.length))
+ val charValue = UTF8String.fromString("abc")
+ val wrappedChar = wrap(charValue, charInspector, charType)
+ assert(wrappedChar.isInstanceOf[HiveCharWritable])
+ assert(wrappedChar.asInstanceOf[HiveCharWritable].getHiveChar.getPaddedValue === "abc ")
+ assert(unwrapperFor(charInspector, charType)(wrappedChar) ===
+ UTF8String.fromString("abc "))
+ assert(wrap(null, charInspector, charType) === null)
+ assert(unwrapperFor(charInspector, charType)(null) === null)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ wrap(UTF8String.fromString("abcdef"), charInspector, charType)
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "5"))
+
+ val varcharType = VarcharType(7)
+ val varcharInspector = PrimitiveObjectInspectorFactory.getPrimitiveWritableObjectInspector(
+ new VarcharTypeInfo(varcharType.length))
+ val varcharValue = UTF8String.fromString("abc")
+ val wrappedVarchar = wrap(varcharValue, varcharInspector, varcharType)
+ assert(wrappedVarchar.isInstanceOf[HiveVarcharWritable])
+ assert(wrappedVarchar.asInstanceOf[HiveVarcharWritable].getHiveVarchar.getValue === "abc")
+ assert(unwrapperFor(varcharInspector, varcharType)(wrappedVarchar) === varcharValue)
+ assert(wrap(null, varcharInspector, varcharType) === null)
+ assert(unwrapperFor(varcharInspector, varcharType)(null) === null)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ wrap(UTF8String.fromString("abcdefgh"), varcharInspector, varcharType)
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "7"))
+ }
+ }
+
+ test("SPARK-59277: Hive object inspectors support nested CHAR/VARCHAR") {
+ withFirstClassCharVarchar(enabled = true) {
+ val dataType = StructType(Seq(
+ StructField("chars", ArrayType(CharType(4))),
+ StructField("varchars", MapType(IntegerType, VarcharType(8)))))
+ val inspector = toInspector(dataType)
+ assert(inspectorToDataType(inspector) === dataType)
+
+ val input = InternalRow(
+ new GenericArrayData(Array[Any](UTF8String.fromString("a"))),
+ ArrayBasedMapData(
+ Array[Any](1),
+ Array[Any](UTF8String.fromString("value"))))
+ val result = unwrapperFor(inspector, dataType)(
+ wrap(input, inspector, dataType)).asInstanceOf[InternalRow]
+ assert(result.getArray(0).getUTF8String(0) === UTF8String.fromString("a "))
+ assert(result.getMap(1).valueArray().getUTF8String(0) === UTF8String.fromString("value"))
+
+ val outerType = StructType(Seq(StructField("nested", dataType)))
+ val outerInspector = toInspector(outerType).asInstanceOf[StructObjectInspector]
+ val field = outerInspector.getAllStructFieldRefs.get(0)
+ val targetRow = new SpecificInternalRow(Seq(dataType))
+ unwrapperFor(field, dataType)(wrap(input, inspector, dataType), targetRow, 0)
+ val nestedResult = targetRow.getStruct(0, dataType.length)
+ assert(nestedResult.getArray(0).getUTF8String(0) === UTF8String.fromString("a "))
+ assert(
+ nestedResult.getMap(1).valueArray().getUTF8String(0) === UTF8String.fromString("value"))
+ }
+ }
+
+ test("SPARK-59277: typed field unwrappers preserve nanosecond timestamps") {
+ val value = Timestamp.valueOf("2026-09-16 12:34:56.123456789")
+ Seq(
+ TimestampNTZNanosType(9) ->
+ DateTimeUtils.localDateTimeToTimestampNanos(value.toLocalDateTime, 9),
+ TimestampLTZNanosType(9) ->
+ DateTimeUtils.instantToTimestampNanos(value.toInstant, 9)).foreach {
+ case (dataType, expected) =>
+ val inspector = ObjectInspectorFactory.getStandardStructObjectInspector(
+ util.Arrays.asList("value"),
+ util.Arrays.asList(PrimitiveObjectInspectorFactory.javaTimestampObjectInspector))
+ val field = inspector.getAllStructFieldRefs.get(0)
+ val targetRow = new SpecificInternalRow(Seq(dataType))
+ unwrapperFor(field, dataType)(value, targetRow, 0)
+ assert(targetRow.get(0, dataType) === expected)
+ }
+ }
+
+ test("SPARK-59277: Hive constant inspectors preserve CHAR/VARCHAR type information") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](CharType(5), VarcharType(7)).foreach { dataType =>
+ val value = UTF8String.fromString("abc")
+ val inspector = toInspector(Literal.create(value, dataType))
+ assert(inspector.isInstanceOf[ConstantObjectInspector])
+ assert(inspectorToDataType(inspector) === dataType)
+ val expected = dataType match {
+ case _: CharType => UTF8String.fromString("abc ")
+ case _: VarcharType => value
+ }
+ assert(unwrapperFor(inspector, dataType)(
+ inspector.asInstanceOf[ConstantObjectInspector].getWritableConstantValue) === expected)
+
+ val nullInspector = toInspector(Literal.create(null, dataType))
+ assert(nullInspector.isInstanceOf[ConstantObjectInspector])
+ assert(inspectorToDataType(nullInspector) === dataType)
+ assert(unwrapperFor(nullInspector, dataType)(
+ nullInspector.asInstanceOf[ConstantObjectInspector].getWritableConstantValue) === null)
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive CHAR/VARCHAR inspectors remain STRING under legacy semantics") {
+ withFirstClassCharVarchar(enabled = false) {
+ Seq[DataType](CharType(5), VarcharType(7)).foreach { dataType =>
+ val inspector = toInspector(dataType)
+ assert(inspectorToDataType(inspector) === StringType)
+ assert(inspectorToDataType(inspector, preserveCharVarchar = true) === dataType)
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive return type compatibility allows only STRING boundary drift") {
+ checkCompatibleHiveReturnType(StringType, CharType(5))
+ checkCompatibleHiveReturnType(CharType(5), StringType)
+ checkCompatibleHiveReturnType(ArrayType(StringType), ArrayType(VarcharType(7)))
+ checkCompatibleHiveReturnType(CharType(5), CharType(5))
+ checkCompatibleHiveReturnType(VarcharType(3), VarcharType(3))
+
+ Seq[(DataType, DataType)](
+ VarcharType(3) -> CharType(5),
+ CharType(4) -> CharType(5),
+ VarcharType(4) -> VarcharType(5),
+ IntegerType -> CharType(5)).foreach { case (runtimeType, expectedType) =>
+ intercept[SparkException] {
+ checkCompatibleHiveReturnType(runtimeType, expectedType)
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive CHAR/VARCHAR boundaries enforce Spark length semantics") {
+ withFirstClassCharVarchar(enabled = true) {
+ val varchar = VarcharType(3)
+ val varcharInspector = toInspector(varchar)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ wrap(UTF8String.fromString("abcd"), varcharInspector, varchar)
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+
+ val char = CharType(5)
+ val charInspector = toInspector(char)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ wrap(UTF8String.fromString("abcdef"), charInspector, char)
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "5"))
+
+ val varcharUnwrapper =
+ unwrapperFor(PrimitiveObjectInspectorFactory.javaStringObjectInspector, varchar)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ varcharUnwrapper("abcd")
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+
+ val charUnwrapper =
+ unwrapperFor(PrimitiveObjectInspectorFactory.javaStringObjectInspector, char)
+ assert(charUnwrapper("ab") === UTF8String.fromString("ab "))
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ charUnwrapper("abcdef")
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "5"))
+ }
+ }
+
+ test("SPARK-59277: Hive object inspectors reject unsupported CHAR/VARCHAR lengths") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](CharType(0), CharType(256), VarcharType(65536)).foreach { dataType =>
Review Comment:
**Non-blocking (P2):** Please add `VarcharType(0)` to both the inspector and type-info invalid-length checks. `CharType(0)` does not exercise the independent VARCHAR lower-bound helper, so that new rejection can currently regress without a test failure.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -1122,6 +1244,51 @@ private[hive] trait HiveInspectors {
case _: JavaVoidObjectInspector => NullType
}
+ /**
+ * Analysis snapshots the Catalyst return type, but runtime inspectors are rebuilt from the
+ * current children (including foldability and session CHAR/VARCHAR settings). STRING may drift
+ * to or from a bounded string type across that boundary. Two bounded types must match exactly:
+ * accepting a different kind or length would apply the snapshotted conversion to an incompatible
+ * runtime value.
+ */
+ def checkCompatibleHiveReturnType(
+ inspector: ObjectInspector,
+ expectedType: DataType): Unit = {
+ checkCompatibleHiveReturnType(
+ inspectorToDataType(inspector, preserveCharVarchar = true),
+ expectedType)
+ }
+
+ def checkCompatibleHiveReturnType(
+ runtimeType: DataType,
+ expectedType: DataType): Unit = {
+ if (!compatibleHiveReturnType(runtimeType, expectedType)) {
+ throw SparkException.internalError(
+ s"Hive function runtime type ${runtimeType.catalogString} is incompatible " +
+ s"with analysis type ${expectedType.catalogString}.")
+ }
+ }
+
+ private def compatibleHiveReturnType(
+ runtimeType: DataType,
+ expectedType: DataType): Boolean = {
+ (runtimeType, expectedType) match {
+ case (rt: CharType, et: CharType) => rt == et
+ case (rt: VarcharType, et: VarcharType) => rt == et
+ case (_: CharType | _: VarcharType, _: CharType | _: VarcharType) => false
+ case (_: StringType, _: CharType | _: VarcharType) => true
+ case (_: CharType | _: VarcharType, _: StringType) => true
+ case (ArrayType(rt, _), ArrayType(et, _)) => compatibleHiveReturnType(rt, et)
+ case (MapType(rk, rv, _), MapType(ek, ev, _)) =>
Review Comment:
**Non-blocking (P2):** The focused compatibility test reaches only scalar and array cases, so neither the new MapType key/value recursion nor the StructType arity/field recursion has a regression-sensitive assertion. Please add positive and negative map and struct cases; otherwise a runtime inspector mismatch in either branch can pass the changed suite.
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDAFSuite.scala:
##########
@@ -200,6 +201,25 @@ class HiveUDAFSuite extends QueryTest
}
}
+ test("SPARK-59277: Hive UDAF supports first-class CHAR/VARCHAR") {
Review Comment:
Confirmed. The new UDAF uses STRING for its partial inspector and CHAR(5) for the final inspector, and the repartitioned test exercises the ordered pair across shuffle. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:4008642285","thread_id":"inline:4008642285","verdict_sha256":"f604aded9a0ed9de1cf7f06c3acfeb8e4e13defa3c50d17e3ed7b9e2910309fe"} -->
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveInspectorSuite.scala:
##########
@@ -292,6 +300,140 @@ class HiveInspectorSuite extends SparkFunSuite with HiveInspectors {
assert(typeInfo2.scale() === 10)
}
+ test("SPARK-59277: Hive object inspectors preserve CHAR/VARCHAR type information") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](CharType(5), VarcharType(7)).foreach { dataType =>
+ val inspector = toInspector(dataType).asInstanceOf[PrimitiveObjectInspector]
+ assert(inspectorToDataType(inspector) === dataType)
+ dataType match {
+ case c: CharType =>
+ assert(inspector.getTypeInfo.asInstanceOf[CharTypeInfo].getLength === c.length)
+ case v: VarcharType =>
+ assert(inspector.getTypeInfo.asInstanceOf[VarcharTypeInfo].getLength === v.length)
+ }
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive object inspectors accept collated CHAR/VARCHAR values") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](
+ CharType(5, "UTF8_LCASE"),
+ VarcharType(7, "UNICODE_CI")).foreach { dataType =>
+ val inspector = toInspector(dataType)
+ val value = UTF8String.fromString(dataType match {
+ case _: CharType => "ab"
+ case _: VarcharType => "abc"
+ })
+ val expectedValue = dataType match {
+ case _: CharType => UTF8String.fromString("ab ")
+ case _: VarcharType => value
+ }
+ val expectedType = dataType match {
+ case c: CharType => CharType(c.length)
+ case v: VarcharType => VarcharType(v.length)
+ }
+ assert(inspectorToDataType(inspector) === expectedType)
+ assert(unwrap(wrap(value, inspector, dataType), inspector) === expectedValue)
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive object inspectors support nested CHAR/VARCHAR") {
+ withFirstClassCharVarchar(enabled = true) {
+ val dataType = StructType(Seq(
+ StructField("chars", ArrayType(CharType(4))),
+ StructField("varchars", MapType(IntegerType, VarcharType(8)))))
+ val inspector = toInspector(dataType)
+ assert(inspectorToDataType(inspector) === dataType)
+
+ val input = InternalRow(
+ new GenericArrayData(Array[Any](UTF8String.fromString("a"))),
+ ArrayBasedMapData(
+ Array[Any](1),
+ Array[Any](UTF8String.fromString("value"))))
+ val result = unwrapperFor(inspector, dataType)(
+ wrap(input, inspector, dataType)).asInstanceOf[InternalRow]
+ assert(result.getArray(0).getUTF8String(0) === UTF8String.fromString("a "))
+ assert(result.getMap(1).valueArray().getUTF8String(0) === UTF8String.fromString("value"))
+
+ val outerType = StructType(Seq(StructField("nested", dataType)))
+ val outerInspector = toInspector(outerType).asInstanceOf[StructObjectInspector]
+ val field = outerInspector.getAllStructFieldRefs.get(0)
+ val targetRow = new SpecificInternalRow(Seq(dataType))
+ unwrapperFor(field, dataType)(wrap(input, inspector, dataType), targetRow, 0)
+ val nestedResult = targetRow.getStruct(0, dataType.length)
+ assert(nestedResult.getArray(0).getUTF8String(0) === UTF8String.fromString("a "))
+ assert(
+ nestedResult.getMap(1).valueArray().getUTF8String(0) === UTF8String.fromString("value"))
+ }
+ }
+
+ test("SPARK-59277: Hive constant inspectors preserve CHAR/VARCHAR type information") {
Review Comment:
Confirmed. The constant-inspector loop now exercises null CHAR and VARCHAR literals through both inspector creation and typed unwrapping. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:4008642290","thread_id":"inline:4008642290","verdict_sha256":"f604aded9a0ed9de1cf7f06c3acfeb8e4e13defa3c50d17e3ed7b9e2910309fe"} -->
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor commented on PR #58584:
URL: https://github.com/apache/spark/pull/58584#issuecomment-5783418813
**Merge Summary:**
- merged into master https://github.com/apache/spark/commit/bb4ef43fa331ebc69e25b38cb99a41da92bb5d93
*Posted by `merge_spark_pr.py`*
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4064656814
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -1122,6 +1258,51 @@ private[hive] trait HiveInspectors {
case _: JavaVoidObjectInspector => NullType
}
+ /**
+ * Analysis snapshots the Catalyst return type, but runtime inspectors are rebuilt from the
+ * current children (including foldability and session CHAR/VARCHAR settings). STRING may drift
+ * to or from a bounded string type across that boundary. Two bounded types must match exactly:
+ * accepting a different kind or length would apply the snapshotted conversion to an incompatible
+ * runtime value.
+ */
+ def checkCompatibleHiveReturnType(
+ inspector: ObjectInspector,
+ expectedType: DataType): Unit = {
+ checkCompatibleHiveReturnType(
+ inspectorToDataType(inspector, preserveCharVarchar = true),
+ expectedType)
+ }
+
+ def checkCompatibleHiveReturnType(
+ runtimeType: DataType,
+ expectedType: DataType): Unit = {
+ if (!compatibleHiveReturnType(runtimeType, expectedType)) {
+ throw SparkException.internalError(
+ s"Hive function runtime type ${runtimeType.catalogString} is incompatible " +
+ s"with analysis type ${expectedType.catalogString}.")
+ }
+ }
+
+ private def compatibleHiveReturnType(
+ runtimeType: DataType,
+ expectedType: DataType): Boolean = {
+ (runtimeType, expectedType) match {
+ case (rt: CharType, et: CharType) => rt == et
+ case (rt: VarcharType, et: VarcharType) => rt == et
+ case (_: CharType | _: VarcharType, _: CharType | _: VarcharType) => false
+ case (_: StringType, _: CharType | _: VarcharType) => true
+ case (_: CharType | _: VarcharType, _: StringType) => true
+ case (ArrayType(rt, _), ArrayType(et, _)) => compatibleHiveReturnType(rt, et)
+ case (MapType(rk, rv, _), MapType(ek, ev, _)) =>
+ compatibleHiveReturnType(rk, ek) && compatibleHiveReturnType(rv, ev)
+ case (rt: StructType, et: StructType) if rt.length == et.length =>
+ rt.fields.zip(et.fields).forall { case (rf, ef) =>
Review Comment:
Could we include the struct field name in this recursive compatibility check? `unwrapperFor(StructObjectInspector, StructType)` pairs runtime inspector fields with the snapshotted schema by ordinal, so runtime `struct<b:int,a:int>` is currently accepted against analysis `struct<a:int,b:int>` and its values are silently exposed under the wrong names. Requiring corresponding names to match before recursing keeps the snapshot guard from accepting renamed or reordered output.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -897,6 +971,25 @@ private[hive] trait HiveInspectors {
(value: Any, row: InternalRow, ordinal: Int) => row(ordinal) = unwrapper(value)
}
+ /**
+ * Builds an in-place unwrapper using the target Catalyst `dataType` only when target-aware
+ * conversion is required. Other types retain the primitive setter fast paths.
+ */
+ def unwrapperFor(
Review Comment:
After Script TRANSFORM support moved to the follow-up, this overload has no production caller; the only remaining calls are in `HiveInspectorSuite`. Could we remove it and the now-orphaned in-place/nanosecond test coverage from this PR, and let the follow-up introduce it together with its production call?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "dtenedor (via GitHub)" <gi...@apache.org>.
dtenedor commented on PR #58584:
URL: https://github.com/apache/spark/pull/58584#issuecomment-5783429281
**Merge Summary:**
- merged into branch-4.x https://github.com/apache/spark/commit/9332d4a1b7a06fb6c3e276a4cf48ba778cf8f085
*Posted by `merge_spark_pr.py`*
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4053991970
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -376,6 +430,83 @@ case class ScriptTransformationIOSchema(
}
object ScriptTransformationIOSchema {
+ private[sql] def toUnboundedStringType(dataType: DataType): DataType = {
+ dataType.transformRecursively {
+ case c: CharType => c.toStringType
+ case v: VarcharType => v.toStringType
+ }
+ }
+
+ // JSON object keys are always strings. Rewrite every map key, including nested maps.
+ // `transformRecursively` would stop at the first matching MapType and skip children.
+ private[sql] def toJsonMapKeyType(dataType: DataType): DataType = dataType match {
+ case ArrayType(et, n) => ArrayType(toJsonMapKeyType(et), n)
+ case MapType(kt, vt, n) =>
+ val jsonKey = if (kt.isInstanceOf[StringType]) kt else StringType
+ MapType(jsonKey, toJsonMapKeyType(vt), n)
+ case StructType(fields) =>
+ StructType(fields.map(f => f.copy(dataType = toJsonMapKeyType(f.dataType))))
+ case other => other
+ }
+
+ private[sql] def restoreJsonMapKeys(
+ expression: Expression,
+ targetType: DataType,
+ timeZoneId: Option[String]): Expression = {
+ def restore(
+ expression: Expression,
+ jsonType: DataType,
+ targetType: DataType): Expression = (jsonType, targetType) match {
+ case (ArrayType(jsonElementType, containsNull), ArrayType(targetElementType, _)) =>
+ val element = NamedLambdaVariable("element", jsonElementType, containsNull)
+ val restoredElement = restore(element, jsonElementType, targetElementType)
+ if (restoredElement.fastEquals(element)) {
+ expression
+ } else {
+ ArrayTransform(expression, LambdaFunction(restoredElement, Seq(element)))
+ }
+ case (
+ MapType(jsonKeyType, jsonValueType, valueContainsNull),
+ MapType(targetKeyType, targetValueType, _)) =>
+ val keys = MapKeys(expression)
+ val key = NamedLambdaVariable("key", jsonKeyType, nullable = false)
+ val restoredKey = if (jsonKeyType.sameType(targetKeyType)) {
+ key
+ } else {
+ Cast(key, targetKeyType, timeZoneId)
+ }
+ val restoredKeys = ArrayTransform(keys, LambdaFunction(restoredKey, Seq(key)))
+
+ val values = MapValues(expression)
+ val value = NamedLambdaVariable("value", jsonValueType, valueContainsNull)
+ val restoredValue = restore(value, jsonValueType, targetValueType)
+ val restoredValues = if (restoredValue.fastEquals(value)) {
+ values
+ } else {
+ ArrayTransform(values, LambdaFunction(restoredValue, Seq(value)))
+ }
+ MapFromArrays(restoredKeys, restoredValues)
Review Comment:
`MapFromArrays` reuses an `ArrayBasedMapBuilder`, and `from` resets it only after a successful build. Here a duplicate converted key throws after the first key has been inserted, but `wrapperConvertException` catches that failure and leaves the builder dirty; the next otherwise-valid row then hits `"'from' can only be called with a fresh ArrayBasedMapBuilder."` and is also converted to null. Please make this restoration state reset-safe (or fresh per input) and cover a colliding row followed by a valid row in the same partition.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -201,7 +222,15 @@ trait BaseScriptTransformationExec extends UnaryExecNode {
private lazy val outputFieldWriters: Seq[String => Any] = output.map { attr =>
val converter = CatalystTypeConverters.createToCatalystConverter(attr.dataType)
attr.dataType match {
- case StringType => wrapperConvertException(data => data, converter)
+ case _: CharType | _: VarcharType =>
+ // First-class CHAR/VARCHAR must not use Hive LazySimpleSerDe's null-on-error path.
Review Comment:
`LazySimpleSerDe` is spelled correctly here, but the existing comment above `wrapperConvertException` in this method still says `LazySimpleSerde`. Could we update that remaining occurrence as well?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4027846797
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -897,6 +953,19 @@ private[hive] trait HiveInspectors {
(value: Any, row: InternalRow, ordinal: Int) => row(ordinal) = unwrapper(value)
}
+ /**
+ * Builds an in-place unwrapper that also honors target-type-specific conversions.
Review Comment:
Done: the in-place unwrapper Scaladoc now states that only CHAR/VARCHAR-bearing targets use the DataType-aware path; other types, including nanosecond timestamps, still use `unwrapperFor(field)`.
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDAFSuite.scala:
##########
@@ -200,6 +201,25 @@ class HiveUDAFSuite extends QueryTest
}
}
+ test("SPARK-59277: Hive UDAF supports first-class CHAR/VARCHAR") {
Review Comment:
Done: added `MockPartialStringFinalCharUDAF`, whose PARTIAL inspector is STRING and whose FINAL inspector is CHAR(5), plus a shuffle-covering test so the `(partial, final)` serde pair cannot silently use one type for both.
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveInspectorSuite.scala:
##########
@@ -292,6 +300,140 @@ class HiveInspectorSuite extends SparkFunSuite with HiveInspectors {
assert(typeInfo2.scale() === 10)
}
+ test("SPARK-59277: Hive object inspectors preserve CHAR/VARCHAR type information") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](CharType(5), VarcharType(7)).foreach { dataType =>
+ val inspector = toInspector(dataType).asInstanceOf[PrimitiveObjectInspector]
+ assert(inspectorToDataType(inspector) === dataType)
+ dataType match {
+ case c: CharType =>
+ assert(inspector.getTypeInfo.asInstanceOf[CharTypeInfo].getLength === c.length)
+ case v: VarcharType =>
+ assert(inspector.getTypeInfo.asInstanceOf[VarcharTypeInfo].getLength === v.length)
+ }
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive object inspectors accept collated CHAR/VARCHAR values") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](
+ CharType(5, "UTF8_LCASE"),
+ VarcharType(7, "UNICODE_CI")).foreach { dataType =>
+ val inspector = toInspector(dataType)
+ val value = UTF8String.fromString(dataType match {
+ case _: CharType => "ab"
+ case _: VarcharType => "abc"
+ })
+ val expectedValue = dataType match {
+ case _: CharType => UTF8String.fromString("ab ")
+ case _: VarcharType => value
+ }
+ val expectedType = dataType match {
+ case c: CharType => CharType(c.length)
+ case v: VarcharType => VarcharType(v.length)
+ }
+ assert(inspectorToDataType(inspector) === expectedType)
+ assert(unwrap(wrap(value, inspector, dataType), inspector) === expectedValue)
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive object inspectors support nested CHAR/VARCHAR") {
+ withFirstClassCharVarchar(enabled = true) {
+ val dataType = StructType(Seq(
+ StructField("chars", ArrayType(CharType(4))),
+ StructField("varchars", MapType(IntegerType, VarcharType(8)))))
+ val inspector = toInspector(dataType)
+ assert(inspectorToDataType(inspector) === dataType)
+
+ val input = InternalRow(
+ new GenericArrayData(Array[Any](UTF8String.fromString("a"))),
+ ArrayBasedMapData(
+ Array[Any](1),
+ Array[Any](UTF8String.fromString("value"))))
+ val result = unwrapperFor(inspector, dataType)(
+ wrap(input, inspector, dataType)).asInstanceOf[InternalRow]
+ assert(result.getArray(0).getUTF8String(0) === UTF8String.fromString("a "))
+ assert(result.getMap(1).valueArray().getUTF8String(0) === UTF8String.fromString("value"))
+
+ val outerType = StructType(Seq(StructField("nested", dataType)))
+ val outerInspector = toInspector(outerType).asInstanceOf[StructObjectInspector]
+ val field = outerInspector.getAllStructFieldRefs.get(0)
+ val targetRow = new SpecificInternalRow(Seq(dataType))
+ unwrapperFor(field, dataType)(wrap(input, inspector, dataType), targetRow, 0)
+ val nestedResult = targetRow.getStruct(0, dataType.length)
+ assert(nestedResult.getArray(0).getUTF8String(0) === UTF8String.fromString("a "))
+ assert(
+ nestedResult.getMap(1).valueArray().getUTF8String(0) === UTF8String.fromString("value"))
+ }
+ }
+
+ test("SPARK-59277: Hive constant inspectors preserve CHAR/VARCHAR type information") {
Review Comment:
Done: the constant-inspector CHAR/VARCHAR cases now also wrap and unwrap a null literal through the new helpers.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4054033006
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -376,6 +430,83 @@ case class ScriptTransformationIOSchema(
}
object ScriptTransformationIOSchema {
+ private[sql] def toUnboundedStringType(dataType: DataType): DataType = {
+ dataType.transformRecursively {
+ case c: CharType => c.toStringType
+ case v: VarcharType => v.toStringType
+ }
+ }
+
+ // JSON object keys are always strings. Rewrite every map key, including nested maps.
+ // `transformRecursively` would stop at the first matching MapType and skip children.
+ private[sql] def toJsonMapKeyType(dataType: DataType): DataType = dataType match {
+ case ArrayType(et, n) => ArrayType(toJsonMapKeyType(et), n)
+ case MapType(kt, vt, n) =>
+ val jsonKey = if (kt.isInstanceOf[StringType]) kt else StringType
+ MapType(jsonKey, toJsonMapKeyType(vt), n)
+ case StructType(fields) =>
+ StructType(fields.map(f => f.copy(dataType = toJsonMapKeyType(f.dataType))))
+ case other => other
+ }
+
+ private[sql] def restoreJsonMapKeys(
+ expression: Expression,
+ targetType: DataType,
+ timeZoneId: Option[String]): Expression = {
+ def restore(
+ expression: Expression,
+ jsonType: DataType,
+ targetType: DataType): Expression = (jsonType, targetType) match {
+ case (ArrayType(jsonElementType, containsNull), ArrayType(targetElementType, _)) =>
+ val element = NamedLambdaVariable("element", jsonElementType, containsNull)
+ val restoredElement = restore(element, jsonElementType, targetElementType)
+ if (restoredElement.fastEquals(element)) {
+ expression
+ } else {
+ ArrayTransform(expression, LambdaFunction(restoredElement, Seq(element)))
+ }
+ case (
+ MapType(jsonKeyType, jsonValueType, valueContainsNull),
+ MapType(targetKeyType, targetValueType, _)) =>
+ val keys = MapKeys(expression)
+ val key = NamedLambdaVariable("key", jsonKeyType, nullable = false)
+ val restoredKey = if (jsonKeyType.sameType(targetKeyType)) {
+ key
+ } else {
+ Cast(key, targetKeyType, timeZoneId)
+ }
+ val restoredKeys = ArrayTransform(keys, LambdaFunction(restoredKey, Seq(key)))
+
+ val values = MapValues(expression)
+ val value = NamedLambdaVariable("value", jsonValueType, valueContainsNull)
+ val restoredValue = restore(value, jsonValueType, targetValueType)
+ val restoredValues = if (restoredValue.fastEquals(value)) {
+ values
+ } else {
+ ArrayTransform(values, LambdaFunction(restoredValue, Seq(value)))
+ }
+ MapFromArrays(restoredKeys, restoredValues)
Review Comment:
Replaced the expression-tree approach (`MapFromArrays` with a shared `ArrayBasedMapBuilder`) with a runtime function that allocates a fresh `ArrayBasedMapBuilder` per invocation. A failed or duplicate key now cannot leave shared state dirty for the next row. Added a focused test with a colliding row followed by a valid row in the same partition. Fixed in 05da5e1d256.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -201,7 +222,15 @@ trait BaseScriptTransformationExec extends UnaryExecNode {
private lazy val outputFieldWriters: Seq[String => Any] = output.map { attr =>
val converter = CatalystTypeConverters.createToCatalystConverter(attr.dataType)
attr.dataType match {
- case StringType => wrapperConvertException(data => data, converter)
+ case _: CharType | _: VarcharType =>
+ // First-class CHAR/VARCHAR must not use Hive LazySimpleSerDe's null-on-error path.
Review Comment:
Updated the remaining `LazySimpleSerde` to `LazySimpleSerDe` in the `wrapperConvertException` comment. Fixed in 05da5e1d256.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4056972670
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -829,6 +869,26 @@ private[hive] trait HiveInspectors {
null
}
}
+ case (_, c: CharType) =>
+ val unwrapper = unwrapperFor(objectInspector)
+ data: Any => {
+ val value = unwrapper(data).asInstanceOf[UTF8String]
+ if (value == null) {
+ null
+ } else {
+ CharVarcharCodegenUtils.charTypeReadSideCheck(value, c.length)
Review Comment:
**Non-blocking (P2):** This recursive key conversion can collapse distinct runtime keys before the map is built. For example, STRING keys `a` and `a ` both become `a ` for an analyzed `MAP<CHAR(2), ...>`, but this path constructs `ArrayBasedMapData` directly, so the duplicate bypasses `MAP_KEY_DEDUP_POLICY` and leaves undefined lookup behavior. Please rebuild the converted entries through a fresh `ArrayBasedMapBuilder` and add bounded-key coverage for both Java and writable inspectors.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala:
##########
@@ -554,4 +592,77 @@ private[hive] case class HiveUDAFFunction(
copy(children = newChildren)
}
+object HiveUDAFFunction extends HiveInspectors {
+ private[hive] case class InitializedEvaluators(
+ partialEvaluator: GenericUDAFEvaluator,
+ partialInspector: ObjectInspector,
+ finalEvaluator: GenericUDAFEvaluator,
+ finalInspector: ObjectInspector)
+
+ def apply(
+ name: String,
+ funcWrapper: HiveFunctionWrapper,
+ children: Seq[Expression]): HiveUDAFFunction = {
+ apply(name, funcWrapper, children, isUDAFBridgeRequired = false)
+ }
+
+ def apply(
+ name: String,
+ funcWrapper: HiveFunctionWrapper,
+ children: Seq[Expression],
+ isUDAFBridgeRequired: Boolean): HiveUDAFFunction = {
+ val (partialType, resultType) =
+ inferResolvedTypes(funcWrapper, children, isUDAFBridgeRequired)
+ HiveUDAFFunction(
+ name,
+ funcWrapper,
+ children,
+ isUDAFBridgeRequired,
+ mutableAggBufferOffset = 0,
+ inputAggBufferOffset = 0,
+ partialType,
+ resultType)
+ }
+
+ private[hive] def initializeEvaluators(
+ funcWrapper: HiveFunctionWrapper,
+ children: Seq[Expression],
+ isUDAFBridgeRequired: Boolean,
+ expectedPartialType: Option[DataType] = None,
+ expectedResultType: Option[DataType] = None): InitializedEvaluators = {
+ val inputInspectors = children.map(toInspector).toArray
+ def newEvaluator(): GenericUDAFEvaluator = {
+ val resolver = if (isUDAFBridgeRequired) {
+ new SparkGenericUDAFBridge(funcWrapper.createFunction[UDAF]())
+ } else {
+ funcWrapper.createFunction[AbstractGenericUDAFResolver]()
+ }
+ val parameterInfo = new SimpleGenericUDAFParameterInfo(
+ inputInspectors, false, false, false)
+ resolver.getEvaluator(parameterInfo)
+ }
+ val partial1 = newEvaluator()
+ val partialInspector = partial1.init(GenericUDAFEvaluator.Mode.PARTIAL1, inputInspectors)
+ val finalEvaluator = newEvaluator()
+ val finalInspector =
+ finalEvaluator.init(GenericUDAFEvaluator.Mode.FINAL, Array(partialInspector))
+ expectedPartialType.foreach(checkCompatibleHiveReturnType(partialInspector, _))
Review Comment:
**Non-blocking (P2):** The current UDAF cases cover compatible copied children and a valid STRING-partial/CHAR-final pair, but neither runtime inspector is incompatible with its corresponding snapshot. Removing either of these checks would therefore leave the suite green while shuffle serialization or final conversion uses the wrong captured type. Please add an end-to-end fixture that independently triggers partial and final mismatches.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -201,7 +218,15 @@ trait BaseScriptTransformationExec extends UnaryExecNode {
private lazy val outputFieldWriters: Seq[String => Any] = output.map { attr =>
val converter = CatalystTypeConverters.createToCatalystConverter(attr.dataType)
attr.dataType match {
- case StringType => wrapperConvertException(data => data, converter)
+ case _: CharType | _: VarcharType =>
+ // First-class CHAR/VARCHAR must not use Hive LazySimpleSerDe's null-on-error path.
+ (data: String) =>
+ if (data == ioschema.outputRowFormatMap("TOK_TABLEROWFORMATNULL")) {
Review Comment:
**Non-blocking (P2):** Please add scalar CHAR and VARCHAR cases for the configured output null token. Without a direct assertion, removing this check leaves the focused tests green while the token is padded as a literal value or rejected for length instead of producing SQL null; a custom token would also prove the configuration is honored.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -1122,6 +1244,51 @@ private[hive] trait HiveInspectors {
case _: JavaVoidObjectInspector => NullType
}
+ /**
+ * Analysis snapshots the Catalyst return type, but runtime inspectors are rebuilt from the
+ * current children (including foldability and session CHAR/VARCHAR settings). STRING may drift
+ * to or from a bounded string type across that boundary. Two bounded types must match exactly:
+ * accepting a different kind or length would apply the snapshotted conversion to an incompatible
+ * runtime value.
+ */
+ def checkCompatibleHiveReturnType(
+ inspector: ObjectInspector,
+ expectedType: DataType): Unit = {
+ checkCompatibleHiveReturnType(
+ inspectorToDataType(inspector, preserveCharVarchar = true),
+ expectedType)
+ }
+
+ def checkCompatibleHiveReturnType(
+ runtimeType: DataType,
+ expectedType: DataType): Unit = {
+ if (!compatibleHiveReturnType(runtimeType, expectedType)) {
+ throw SparkException.internalError(
+ s"Hive function runtime type ${runtimeType.catalogString} is incompatible " +
+ s"with analysis type ${expectedType.catalogString}.")
+ }
+ }
+
+ private def compatibleHiveReturnType(
+ runtimeType: DataType,
+ expectedType: DataType): Boolean = {
+ (runtimeType, expectedType) match {
+ case (rt: CharType, et: CharType) => rt == et
+ case (rt: VarcharType, et: VarcharType) => rt == et
+ case (_: CharType | _: VarcharType, _: CharType | _: VarcharType) => false
+ case (_: StringType, _: CharType | _: VarcharType) => true
+ case (_: CharType | _: VarcharType, _: StringType) => true
+ case (ArrayType(rt, _), ArrayType(et, _)) => compatibleHiveReturnType(rt, et)
+ case (MapType(rk, rv, _), MapType(ek, ev, _)) =>
Review Comment:
Confirmed. The compatibility suite now exercises map key/value recursion and struct field recursion, including arity and bounded-length mismatches. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:4036387242","thread_id":"inline:4036387242","verdict_sha256":"686cf8b8db16e9edfdf7fa644425fede25c5c3f146e894c6f1f37b7f07580e18"} -->
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFEvaluators.scala:
##########
@@ -155,16 +157,42 @@ class HiveGenericUDFEvaluator(
oi
}
}
+}
+
+private[hive] class HiveGenericUDFEvaluator(
+ funcWrapper: HiveFunctionWrapper,
+ children: Seq[Expression],
+ catalystReturnType: DataType)
+ extends HiveUDFEvaluatorBase[GenericUDF](funcWrapper, children) {
+
+ // SPARK-58792: copied expression nodes (e.g. via withNewChildrenInternal) share one
+ // HiveFunctionWrapper, whose cached GenericUDF instance is mutable: initialize()
+ // rewrites its converters and output holders based on the arguments of whichever
+ // copy initialized it last. Give every evaluator its own clone so copied nodes
+ // cannot corrupt each other.
+ @transient
+ override lazy val function: GenericUDF =
+ HiveFunctionRegistryUtils.cloneGenericUDF(funcWrapper.createFunction[GenericUDF]())
+
+ @transient
+ private lazy val argumentInspectors = children.map(toInspector).toArray
+
+ @transient
+ lazy val returnInspector = {
+ val inspector = HiveGenericUDFEvaluator.initialize(function, argumentInspectors)
+ checkCompatibleHiveReturnType(inspector, catalystReturnType)
Review Comment:
**Non-blocking (P2):** The helper-level mismatch matrix does not prove this consuming guard remains wired in, and the current child-constantness cases preserve the same CHAR/VARCHAR kind and length. Please add custom GenericUDF and GenericUDTF cases whose runtime inspector is incompatible with the analysis snapshot, so deleting either call-site check fails its owning execution test.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -241,6 +266,29 @@ trait BaseScriptTransformationExec extends UnaryExecNode {
data => IntervalUtils.microsToDuration(
IntervalUtils.castStringToDTInterval(UTF8String.fromString(data), start, end)),
converter)
+ case dt @ (_: ArrayType | _: MapType | _: StructType)
+ if CharVarcharUtils.hasCharVarchar(dt) =>
+ val physicalType = ScriptTransformationIOSchema.toUnboundedStringType(dt)
+ // JSON object keys are strings. Cast them to the declared map key type after parsing.
+ val jsonType = ScriptTransformationIOSchema.toJsonMapKeyType(physicalType)
+ val complexTypeFactory = JsonToStructs(
+ jsonType,
+ ioschema.outputSerdeProps.toMap,
+ Literal(null),
+ Some(conf.sessionLocalTimeZone))
+ val parsedToPhysical = if (jsonType.sameType(physicalType)) {
+ identity[Any] _
+ } else {
+ val restoreMapKeys = ScriptTransformationIOSchema.makeJsonMapKeyRestorer(
+ physicalType, Some(conf.sessionLocalTimeZone))
+ value: Any => restoreMapKeys(value)
+ }
+ val toScala = CatalystTypeConverters.createToScalaConverter(physicalType)
+ val parser = wrapperConvertException(
+ data => parsedToPhysical(
+ complexTypeFactory.nullSafeEval(UTF8String.fromString(data))),
+ identity)
+ data => converter(toScala(parser(data)))
Review Comment:
**Non-blocking (P2):** The fresh builder above validates only restoration to the unbounded physical key type. This final conversion can still collapse distinct keys: `a` and `a ` both pad to `CHAR(2)`, and permitted VARCHAR trailing-space trimming has the same issue. `CatalystTypeConverters` then creates `ArrayBasedMapData` without duplicate validation. Please make the final bounded-key conversion use validated map construction recursively so collisions follow Spark's configured duplicate-key behavior.
See **Shared repair plan 1** in the review body.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFEvaluators.scala:
##########
@@ -111,34 +111,36 @@ class HiveSimpleUDFEvaluator(
}
}
-class HiveGenericUDFEvaluator(
- funcWrapper: HiveFunctionWrapper, children: Seq[Expression])
- extends HiveUDFEvaluatorBase[GenericUDF](funcWrapper, children) {
-
- // SPARK-58792: copied expression nodes (e.g. via withNewChildrenInternal) share one
- // HiveFunctionWrapper, whose cached GenericUDF instance is mutable: initialize()
- // rewrites its converters and output holders based on the arguments of whichever
- // copy initialized it last. Give every evaluator its own clone so copied nodes
- // cannot corrupt each other.
- @transient
- override lazy val function: GenericUDF =
- HiveFunctionRegistryUtils.cloneGenericUDF(funcWrapper.createFunction[GenericUDF]())
-
- @transient
- private lazy val argumentInspectors = children.map(toInspector).toArray
+private[hive] object HiveGenericUDFEvaluator extends HiveInspectors {
+
+ /**
+ * Driver-side Hive initialize for `SELECT hive_udf(...)`. Returns the Catalyst type
Review Comment:
Confirmed. The Scaladoc now clearly says `Driver-side Hive initialization`. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:4036387252","thread_id":"inline:4036387252","verdict_sha256":"686cf8b8db16e9edfdf7fa644425fede25c5c3f146e894c6f1f37b7f07580e18"} -->
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -201,7 +211,15 @@ trait BaseScriptTransformationExec extends UnaryExecNode {
private lazy val outputFieldWriters: Seq[String => Any] = output.map { attr =>
val converter = CatalystTypeConverters.createToCatalystConverter(attr.dataType)
attr.dataType match {
- case StringType => wrapperConvertException(data => data, converter)
+ case _: CharType | _: VarcharType =>
+ // First-class CHAR/VARCHAR must not use Hive LazySimpleSerde's null-on-error path.
Review Comment:
Confirmed. The cited comment now spells `LazySimpleSerDe` correctly. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:4036387257","thread_id":"inline:4036387257","verdict_sha256":"686cf8b8db16e9edfdf7fa644425fede25c5c3f146e894c6f1f37b7f07580e18"} -->
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -241,6 +259,31 @@ trait BaseScriptTransformationExec extends UnaryExecNode {
data => IntervalUtils.microsToDuration(
IntervalUtils.castStringToDTInterval(UTF8String.fromString(data), start, end)),
converter)
+ case dt @ (_: ArrayType | _: MapType | _: StructType)
+ if CharVarcharUtils.hasCharVarchar(dt) =>
+ val physicalType = ScriptTransformationIOSchema.toUnboundedStringType(dt)
+ // JSON object keys are strings. Cast them to the declared map key type after parsing.
+ val jsonType = ScriptTransformationIOSchema.toJsonMapKeyType(physicalType)
+ val complexTypeFactory = JsonToStructs(
+ jsonType,
+ ioschema.outputSerdeProps.toMap,
+ Literal(null),
+ Some(conf.sessionLocalTimeZone))
+ val parsedToPhysical = if (jsonType.sameType(physicalType)) {
+ identity[Any] _
+ } else {
+ val cast = Cast(
Review Comment:
Confirmed. JSON map keys are now restored recursively through a fresh validated builder, and the same-partition regression proves a failed row cannot contaminate the next row. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:4036387263","thread_id":"inline:4036387263","verdict_sha256":"686cf8b8db16e9edfdf7fa644425fede25c5c3f146e894c6f1f37b7f07580e18"} -->
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveInspectorSuite.scala:
##########
@@ -292,6 +301,240 @@ class HiveInspectorSuite extends SparkFunSuite with HiveInspectors {
assert(typeInfo2.scale() === 10)
}
+ test("SPARK-59277: Hive object inspectors preserve CHAR/VARCHAR type information") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](CharType(5), VarcharType(7)).foreach { dataType =>
+ val inspector = toInspector(dataType).asInstanceOf[PrimitiveObjectInspector]
+ assert(inspectorToDataType(inspector) === dataType)
+ dataType match {
+ case c: CharType =>
+ assert(inspector.getTypeInfo.asInstanceOf[CharTypeInfo].getLength === c.length)
+ case v: VarcharType =>
+ assert(inspector.getTypeInfo.asInstanceOf[VarcharTypeInfo].getLength === v.length)
+ }
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive object inspectors accept collated CHAR/VARCHAR values") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](
+ CharType(5, "UTF8_LCASE"),
+ VarcharType(7, "UNICODE_CI")).foreach { dataType =>
+ val inspector = toInspector(dataType)
+ val value = UTF8String.fromString(dataType match {
+ case _: CharType => "ab"
+ case _: VarcharType => "abc"
+ })
+ val expectedValue = dataType match {
+ case _: CharType => UTF8String.fromString("ab ")
+ case _: VarcharType => value
+ }
+ val expectedType = dataType match {
+ case c: CharType => CharType(c.length)
+ case v: VarcharType => VarcharType(v.length)
+ }
+ assert(inspectorToDataType(inspector) === expectedType)
+ assert(unwrap(wrap(value, inspector, dataType), inspector) === expectedValue)
+ }
+ }
+ }
+
+ test("SPARK-59277: writable CHAR/VARCHAR inspectors round-trip values and nulls") {
+ withFirstClassCharVarchar(enabled = true) {
+ val charType = CharType(5)
+ val charInspector = PrimitiveObjectInspectorFactory.getPrimitiveWritableObjectInspector(
+ new CharTypeInfo(charType.length))
+ val charValue = UTF8String.fromString("abc")
+ val wrappedChar = wrap(charValue, charInspector, charType)
+ assert(wrappedChar.isInstanceOf[HiveCharWritable])
+ assert(wrappedChar.asInstanceOf[HiveCharWritable].getHiveChar.getPaddedValue === "abc ")
+ assert(unwrapperFor(charInspector, charType)(wrappedChar) ===
+ UTF8String.fromString("abc "))
+ assert(wrap(null, charInspector, charType) === null)
+ assert(unwrapperFor(charInspector, charType)(null) === null)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ wrap(UTF8String.fromString("abcdef"), charInspector, charType)
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "5"))
+
+ val varcharType = VarcharType(7)
+ val varcharInspector = PrimitiveObjectInspectorFactory.getPrimitiveWritableObjectInspector(
+ new VarcharTypeInfo(varcharType.length))
+ val varcharValue = UTF8String.fromString("abc")
+ val wrappedVarchar = wrap(varcharValue, varcharInspector, varcharType)
+ assert(wrappedVarchar.isInstanceOf[HiveVarcharWritable])
+ assert(wrappedVarchar.asInstanceOf[HiveVarcharWritable].getHiveVarchar.getValue === "abc")
+ assert(unwrapperFor(varcharInspector, varcharType)(wrappedVarchar) === varcharValue)
+ assert(wrap(null, varcharInspector, varcharType) === null)
+ assert(unwrapperFor(varcharInspector, varcharType)(null) === null)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ wrap(UTF8String.fromString("abcdefgh"), varcharInspector, varcharType)
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "7"))
+ }
+ }
+
+ test("SPARK-59277: Hive object inspectors support nested CHAR/VARCHAR") {
+ withFirstClassCharVarchar(enabled = true) {
+ val dataType = StructType(Seq(
+ StructField("chars", ArrayType(CharType(4))),
+ StructField("varchars", MapType(IntegerType, VarcharType(8)))))
+ val inspector = toInspector(dataType)
+ assert(inspectorToDataType(inspector) === dataType)
+
+ val input = InternalRow(
+ new GenericArrayData(Array[Any](UTF8String.fromString("a"))),
+ ArrayBasedMapData(
+ Array[Any](1),
+ Array[Any](UTF8String.fromString("value"))))
+ val result = unwrapperFor(inspector, dataType)(
+ wrap(input, inspector, dataType)).asInstanceOf[InternalRow]
+ assert(result.getArray(0).getUTF8String(0) === UTF8String.fromString("a "))
+ assert(result.getMap(1).valueArray().getUTF8String(0) === UTF8String.fromString("value"))
+
+ val outerType = StructType(Seq(StructField("nested", dataType)))
+ val outerInspector = toInspector(outerType).asInstanceOf[StructObjectInspector]
+ val field = outerInspector.getAllStructFieldRefs.get(0)
+ val targetRow = new SpecificInternalRow(Seq(dataType))
+ unwrapperFor(field, dataType)(wrap(input, inspector, dataType), targetRow, 0)
+ val nestedResult = targetRow.getStruct(0, dataType.length)
+ assert(nestedResult.getArray(0).getUTF8String(0) === UTF8String.fromString("a "))
+ assert(
+ nestedResult.getMap(1).valueArray().getUTF8String(0) === UTF8String.fromString("value"))
+ }
+ }
+
+ test("SPARK-59277: typed field unwrappers preserve nanosecond timestamps") {
+ val value = Timestamp.valueOf("2026-09-16 12:34:56.123456789")
+ Seq(
+ TimestampNTZNanosType(9) ->
+ DateTimeUtils.localDateTimeToTimestampNanos(value.toLocalDateTime, 9),
+ TimestampLTZNanosType(9) ->
+ DateTimeUtils.instantToTimestampNanos(value.toInstant, 9)).foreach {
+ case (dataType, expected) =>
+ val inspector = ObjectInspectorFactory.getStandardStructObjectInspector(
+ util.Arrays.asList("value"),
+ util.Arrays.asList(PrimitiveObjectInspectorFactory.javaTimestampObjectInspector))
+ val field = inspector.getAllStructFieldRefs.get(0)
+ val targetRow = new SpecificInternalRow(Seq(dataType))
+ unwrapperFor(field, dataType)(value, targetRow, 0)
+ assert(targetRow.get(0, dataType) === expected)
+ }
+ }
+
+ test("SPARK-59277: Hive constant inspectors preserve CHAR/VARCHAR type information") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](CharType(5), VarcharType(7)).foreach { dataType =>
+ val value = UTF8String.fromString("abc")
+ val inspector = toInspector(Literal.create(value, dataType))
+ assert(inspector.isInstanceOf[ConstantObjectInspector])
+ assert(inspectorToDataType(inspector) === dataType)
+ val expected = dataType match {
+ case _: CharType => UTF8String.fromString("abc ")
+ case _: VarcharType => value
+ }
+ assert(unwrapperFor(inspector, dataType)(
+ inspector.asInstanceOf[ConstantObjectInspector].getWritableConstantValue) === expected)
+
+ val nullInspector = toInspector(Literal.create(null, dataType))
+ assert(nullInspector.isInstanceOf[ConstantObjectInspector])
+ assert(inspectorToDataType(nullInspector) === dataType)
+ assert(unwrapperFor(nullInspector, dataType)(
+ nullInspector.asInstanceOf[ConstantObjectInspector].getWritableConstantValue) === null)
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive CHAR/VARCHAR inspectors remain STRING under legacy semantics") {
+ withFirstClassCharVarchar(enabled = false) {
+ Seq[DataType](CharType(5), VarcharType(7)).foreach { dataType =>
+ val inspector = toInspector(dataType)
+ assert(inspectorToDataType(inspector) === StringType)
+ assert(inspectorToDataType(inspector, preserveCharVarchar = true) === dataType)
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive return type compatibility allows only STRING boundary drift") {
+ checkCompatibleHiveReturnType(StringType, CharType(5))
+ checkCompatibleHiveReturnType(CharType(5), StringType)
+ checkCompatibleHiveReturnType(ArrayType(StringType), ArrayType(VarcharType(7)))
+ checkCompatibleHiveReturnType(CharType(5), CharType(5))
+ checkCompatibleHiveReturnType(VarcharType(3), VarcharType(3))
+
+ Seq[(DataType, DataType)](
+ VarcharType(3) -> CharType(5),
+ CharType(4) -> CharType(5),
+ VarcharType(4) -> VarcharType(5),
+ IntegerType -> CharType(5)).foreach { case (runtimeType, expectedType) =>
+ intercept[SparkException] {
+ checkCompatibleHiveReturnType(runtimeType, expectedType)
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive CHAR/VARCHAR boundaries enforce Spark length semantics") {
+ withFirstClassCharVarchar(enabled = true) {
+ val varchar = VarcharType(3)
+ val varcharInspector = toInspector(varchar)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ wrap(UTF8String.fromString("abcd"), varcharInspector, varchar)
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+
+ val char = CharType(5)
+ val charInspector = toInspector(char)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ wrap(UTF8String.fromString("abcdef"), charInspector, char)
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "5"))
+
+ val varcharUnwrapper =
+ unwrapperFor(PrimitiveObjectInspectorFactory.javaStringObjectInspector, varchar)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ varcharUnwrapper("abcd")
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+
+ val charUnwrapper =
+ unwrapperFor(PrimitiveObjectInspectorFactory.javaStringObjectInspector, char)
+ assert(charUnwrapper("ab") === UTF8String.fromString("ab "))
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ charUnwrapper("abcdef")
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "5"))
+ }
+ }
+
+ test("SPARK-59277: Hive object inspectors reject unsupported CHAR/VARCHAR lengths") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](CharType(0), CharType(256), VarcharType(65536)).foreach { dataType =>
Review Comment:
Confirmed. `VarcharType(0)` now exercises both the object-inspector and type-info lower-bound checks. Thanks.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:4036387274","thread_id":"inline:4036387274","verdict_sha256":"686cf8b8db16e9edfdf7fa644425fede25c5c3f146e894c6f1f37b7f07580e18"} -->
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4075677629
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -829,6 +869,26 @@ private[hive] trait HiveInspectors {
null
}
}
+ case (_, c: CharType) =>
Review Comment:
Acknowledged. A SQLConf guard does not work here because the persisted-view test legitimately queries a first-class-analyzed view under legacy settings, and the captured `catalystReturnType` must still be honored -- gating on execution-time `charVarcharFirstClassTypes` breaks that contract.
The deeper fix (separating the ORC nanos-only policy from UDF CHAR/VARCHAR enforcement so `readSideCharPadding=false` is respected in ORC reads with a nanos sibling) requires a distinct recursive entry point or conversion policy, which is out of scope for this PR. The ORC/table-reader callers today only reach CHAR/VARCHAR targets when preserve-only mode is enabled, and in that case padding is the correct behavior (`ApplyCharTypePadding` would apply it anyway). The `readSideCharPadding=false` interaction is a pre-existing edge case.
I will file a follow-up JIRA for this.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4052038261
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -1122,6 +1244,51 @@ private[hive] trait HiveInspectors {
case _: JavaVoidObjectInspector => NullType
}
+ /**
+ * Analysis snapshots the Catalyst return type, but runtime inspectors are rebuilt from the
+ * current children (including foldability and session CHAR/VARCHAR settings). STRING may drift
+ * to or from a bounded string type across that boundary. Two bounded types must match exactly:
+ * accepting a different kind or length would apply the snapshotted conversion to an incompatible
+ * runtime value.
+ */
+ def checkCompatibleHiveReturnType(
+ inspector: ObjectInspector,
+ expectedType: DataType): Unit = {
+ checkCompatibleHiveReturnType(
+ inspectorToDataType(inspector, preserveCharVarchar = true),
+ expectedType)
+ }
+
+ def checkCompatibleHiveReturnType(
+ runtimeType: DataType,
+ expectedType: DataType): Unit = {
+ if (!compatibleHiveReturnType(runtimeType, expectedType)) {
+ throw SparkException.internalError(
+ s"Hive function runtime type ${runtimeType.catalogString} is incompatible " +
+ s"with analysis type ${expectedType.catalogString}.")
+ }
+ }
+
+ private def compatibleHiveReturnType(
+ runtimeType: DataType,
+ expectedType: DataType): Boolean = {
+ (runtimeType, expectedType) match {
+ case (rt: CharType, et: CharType) => rt == et
+ case (rt: VarcharType, et: VarcharType) => rt == et
+ case (_: CharType | _: VarcharType, _: CharType | _: VarcharType) => false
+ case (_: StringType, _: CharType | _: VarcharType) => true
+ case (_: CharType | _: VarcharType, _: StringType) => true
+ case (ArrayType(rt, _), ArrayType(et, _)) => compatibleHiveReturnType(rt, et)
+ case (MapType(rk, rv, _), MapType(ek, ev, _)) =>
Review Comment:
Added positive and negative map and struct compatibility cases in `HiveInspectorSuite`, covering key/value recursion, struct field recursion, arity mismatch, and bounded-length mismatch. Fixed in 0cc7cdb2bc9.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFEvaluators.scala:
##########
@@ -111,34 +111,36 @@ class HiveSimpleUDFEvaluator(
}
}
-class HiveGenericUDFEvaluator(
- funcWrapper: HiveFunctionWrapper, children: Seq[Expression])
- extends HiveUDFEvaluatorBase[GenericUDF](funcWrapper, children) {
-
- // SPARK-58792: copied expression nodes (e.g. via withNewChildrenInternal) share one
- // HiveFunctionWrapper, whose cached GenericUDF instance is mutable: initialize()
- // rewrites its converters and output holders based on the arguments of whichever
- // copy initialized it last. Give every evaluator its own clone so copied nodes
- // cannot corrupt each other.
- @transient
- override lazy val function: GenericUDF =
- HiveFunctionRegistryUtils.cloneGenericUDF(funcWrapper.createFunction[GenericUDF]())
-
- @transient
- private lazy val argumentInspectors = children.map(toInspector).toArray
+private[hive] object HiveGenericUDFEvaluator extends HiveInspectors {
+
+ /**
+ * Driver-side Hive initialize for `SELECT hive_udf(...)`. Returns the Catalyst type
Review Comment:
Updated the Scaladoc to say `Driver-side Hive initialization`. Fixed in 0cc7cdb2bc9.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -201,7 +211,15 @@ trait BaseScriptTransformationExec extends UnaryExecNode {
private lazy val outputFieldWriters: Seq[String => Any] = output.map { attr =>
val converter = CatalystTypeConverters.createToCatalystConverter(attr.dataType)
attr.dataType match {
- case StringType => wrapperConvertException(data => data, converter)
+ case _: CharType | _: VarcharType =>
+ // First-class CHAR/VARCHAR must not use Hive LazySimpleSerde's null-on-error path.
Review Comment:
Corrected the class name to `LazySimpleSerDe`. Fixed in 0cc7cdb2bc9.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/BaseScriptTransformationExec.scala:
##########
@@ -241,6 +259,31 @@ trait BaseScriptTransformationExec extends UnaryExecNode {
data => IntervalUtils.microsToDuration(
IntervalUtils.castStringToDTInterval(UTF8String.fromString(data), start, end)),
converter)
+ case dt @ (_: ArrayType | _: MapType | _: StructType)
+ if CharVarcharUtils.hasCharVarchar(dt) =>
+ val physicalType = ScriptTransformationIOSchema.toUnboundedStringType(dt)
+ // JSON object keys are strings. Cast them to the declared map key type after parsing.
+ val jsonType = ScriptTransformationIOSchema.toJsonMapKeyType(physicalType)
+ val complexTypeFactory = JsonToStructs(
+ jsonType,
+ ioschema.outputSerdeProps.toMap,
+ Literal(null),
+ Some(conf.sessionLocalTimeZone))
+ val parsedToPhysical = if (jsonType.sameType(physicalType)) {
+ identity[Any] _
+ } else {
+ val cast = Cast(
Review Comment:
Replaced the whole-map cast with recursive key restoration through `MapFromArrays`, which validates null and duplicate converted keys. The restoration recurses through arrays, structs, and map values inside the malformed-field wrapper, while CHAR/VARCHAR conversion remains outside so overflow still raises `EXCEED_LIMIT_LENGTH`. Added valid, invalid, duplicate-producing, and nested-key coverage. Fixed in 0cc7cdb2bc9.
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveInspectorSuite.scala:
##########
@@ -292,6 +301,240 @@ class HiveInspectorSuite extends SparkFunSuite with HiveInspectors {
assert(typeInfo2.scale() === 10)
}
+ test("SPARK-59277: Hive object inspectors preserve CHAR/VARCHAR type information") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](CharType(5), VarcharType(7)).foreach { dataType =>
+ val inspector = toInspector(dataType).asInstanceOf[PrimitiveObjectInspector]
+ assert(inspectorToDataType(inspector) === dataType)
+ dataType match {
+ case c: CharType =>
+ assert(inspector.getTypeInfo.asInstanceOf[CharTypeInfo].getLength === c.length)
+ case v: VarcharType =>
+ assert(inspector.getTypeInfo.asInstanceOf[VarcharTypeInfo].getLength === v.length)
+ }
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive object inspectors accept collated CHAR/VARCHAR values") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](
+ CharType(5, "UTF8_LCASE"),
+ VarcharType(7, "UNICODE_CI")).foreach { dataType =>
+ val inspector = toInspector(dataType)
+ val value = UTF8String.fromString(dataType match {
+ case _: CharType => "ab"
+ case _: VarcharType => "abc"
+ })
+ val expectedValue = dataType match {
+ case _: CharType => UTF8String.fromString("ab ")
+ case _: VarcharType => value
+ }
+ val expectedType = dataType match {
+ case c: CharType => CharType(c.length)
+ case v: VarcharType => VarcharType(v.length)
+ }
+ assert(inspectorToDataType(inspector) === expectedType)
+ assert(unwrap(wrap(value, inspector, dataType), inspector) === expectedValue)
+ }
+ }
+ }
+
+ test("SPARK-59277: writable CHAR/VARCHAR inspectors round-trip values and nulls") {
+ withFirstClassCharVarchar(enabled = true) {
+ val charType = CharType(5)
+ val charInspector = PrimitiveObjectInspectorFactory.getPrimitiveWritableObjectInspector(
+ new CharTypeInfo(charType.length))
+ val charValue = UTF8String.fromString("abc")
+ val wrappedChar = wrap(charValue, charInspector, charType)
+ assert(wrappedChar.isInstanceOf[HiveCharWritable])
+ assert(wrappedChar.asInstanceOf[HiveCharWritable].getHiveChar.getPaddedValue === "abc ")
+ assert(unwrapperFor(charInspector, charType)(wrappedChar) ===
+ UTF8String.fromString("abc "))
+ assert(wrap(null, charInspector, charType) === null)
+ assert(unwrapperFor(charInspector, charType)(null) === null)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ wrap(UTF8String.fromString("abcdef"), charInspector, charType)
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "5"))
+
+ val varcharType = VarcharType(7)
+ val varcharInspector = PrimitiveObjectInspectorFactory.getPrimitiveWritableObjectInspector(
+ new VarcharTypeInfo(varcharType.length))
+ val varcharValue = UTF8String.fromString("abc")
+ val wrappedVarchar = wrap(varcharValue, varcharInspector, varcharType)
+ assert(wrappedVarchar.isInstanceOf[HiveVarcharWritable])
+ assert(wrappedVarchar.asInstanceOf[HiveVarcharWritable].getHiveVarchar.getValue === "abc")
+ assert(unwrapperFor(varcharInspector, varcharType)(wrappedVarchar) === varcharValue)
+ assert(wrap(null, varcharInspector, varcharType) === null)
+ assert(unwrapperFor(varcharInspector, varcharType)(null) === null)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ wrap(UTF8String.fromString("abcdefgh"), varcharInspector, varcharType)
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "7"))
+ }
+ }
+
+ test("SPARK-59277: Hive object inspectors support nested CHAR/VARCHAR") {
+ withFirstClassCharVarchar(enabled = true) {
+ val dataType = StructType(Seq(
+ StructField("chars", ArrayType(CharType(4))),
+ StructField("varchars", MapType(IntegerType, VarcharType(8)))))
+ val inspector = toInspector(dataType)
+ assert(inspectorToDataType(inspector) === dataType)
+
+ val input = InternalRow(
+ new GenericArrayData(Array[Any](UTF8String.fromString("a"))),
+ ArrayBasedMapData(
+ Array[Any](1),
+ Array[Any](UTF8String.fromString("value"))))
+ val result = unwrapperFor(inspector, dataType)(
+ wrap(input, inspector, dataType)).asInstanceOf[InternalRow]
+ assert(result.getArray(0).getUTF8String(0) === UTF8String.fromString("a "))
+ assert(result.getMap(1).valueArray().getUTF8String(0) === UTF8String.fromString("value"))
+
+ val outerType = StructType(Seq(StructField("nested", dataType)))
+ val outerInspector = toInspector(outerType).asInstanceOf[StructObjectInspector]
+ val field = outerInspector.getAllStructFieldRefs.get(0)
+ val targetRow = new SpecificInternalRow(Seq(dataType))
+ unwrapperFor(field, dataType)(wrap(input, inspector, dataType), targetRow, 0)
+ val nestedResult = targetRow.getStruct(0, dataType.length)
+ assert(nestedResult.getArray(0).getUTF8String(0) === UTF8String.fromString("a "))
+ assert(
+ nestedResult.getMap(1).valueArray().getUTF8String(0) === UTF8String.fromString("value"))
+ }
+ }
+
+ test("SPARK-59277: typed field unwrappers preserve nanosecond timestamps") {
+ val value = Timestamp.valueOf("2026-09-16 12:34:56.123456789")
+ Seq(
+ TimestampNTZNanosType(9) ->
+ DateTimeUtils.localDateTimeToTimestampNanos(value.toLocalDateTime, 9),
+ TimestampLTZNanosType(9) ->
+ DateTimeUtils.instantToTimestampNanos(value.toInstant, 9)).foreach {
+ case (dataType, expected) =>
+ val inspector = ObjectInspectorFactory.getStandardStructObjectInspector(
+ util.Arrays.asList("value"),
+ util.Arrays.asList(PrimitiveObjectInspectorFactory.javaTimestampObjectInspector))
+ val field = inspector.getAllStructFieldRefs.get(0)
+ val targetRow = new SpecificInternalRow(Seq(dataType))
+ unwrapperFor(field, dataType)(value, targetRow, 0)
+ assert(targetRow.get(0, dataType) === expected)
+ }
+ }
+
+ test("SPARK-59277: Hive constant inspectors preserve CHAR/VARCHAR type information") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](CharType(5), VarcharType(7)).foreach { dataType =>
+ val value = UTF8String.fromString("abc")
+ val inspector = toInspector(Literal.create(value, dataType))
+ assert(inspector.isInstanceOf[ConstantObjectInspector])
+ assert(inspectorToDataType(inspector) === dataType)
+ val expected = dataType match {
+ case _: CharType => UTF8String.fromString("abc ")
+ case _: VarcharType => value
+ }
+ assert(unwrapperFor(inspector, dataType)(
+ inspector.asInstanceOf[ConstantObjectInspector].getWritableConstantValue) === expected)
+
+ val nullInspector = toInspector(Literal.create(null, dataType))
+ assert(nullInspector.isInstanceOf[ConstantObjectInspector])
+ assert(inspectorToDataType(nullInspector) === dataType)
+ assert(unwrapperFor(nullInspector, dataType)(
+ nullInspector.asInstanceOf[ConstantObjectInspector].getWritableConstantValue) === null)
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive CHAR/VARCHAR inspectors remain STRING under legacy semantics") {
+ withFirstClassCharVarchar(enabled = false) {
+ Seq[DataType](CharType(5), VarcharType(7)).foreach { dataType =>
+ val inspector = toInspector(dataType)
+ assert(inspectorToDataType(inspector) === StringType)
+ assert(inspectorToDataType(inspector, preserveCharVarchar = true) === dataType)
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive return type compatibility allows only STRING boundary drift") {
+ checkCompatibleHiveReturnType(StringType, CharType(5))
+ checkCompatibleHiveReturnType(CharType(5), StringType)
+ checkCompatibleHiveReturnType(ArrayType(StringType), ArrayType(VarcharType(7)))
+ checkCompatibleHiveReturnType(CharType(5), CharType(5))
+ checkCompatibleHiveReturnType(VarcharType(3), VarcharType(3))
+
+ Seq[(DataType, DataType)](
+ VarcharType(3) -> CharType(5),
+ CharType(4) -> CharType(5),
+ VarcharType(4) -> VarcharType(5),
+ IntegerType -> CharType(5)).foreach { case (runtimeType, expectedType) =>
+ intercept[SparkException] {
+ checkCompatibleHiveReturnType(runtimeType, expectedType)
+ }
+ }
+ }
+
+ test("SPARK-59277: Hive CHAR/VARCHAR boundaries enforce Spark length semantics") {
+ withFirstClassCharVarchar(enabled = true) {
+ val varchar = VarcharType(3)
+ val varcharInspector = toInspector(varchar)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ wrap(UTF8String.fromString("abcd"), varcharInspector, varchar)
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+
+ val char = CharType(5)
+ val charInspector = toInspector(char)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ wrap(UTF8String.fromString("abcdef"), charInspector, char)
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "5"))
+
+ val varcharUnwrapper =
+ unwrapperFor(PrimitiveObjectInspectorFactory.javaStringObjectInspector, varchar)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ varcharUnwrapper("abcd")
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+
+ val charUnwrapper =
+ unwrapperFor(PrimitiveObjectInspectorFactory.javaStringObjectInspector, char)
+ assert(charUnwrapper("ab") === UTF8String.fromString("ab "))
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ charUnwrapper("abcdef")
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "5"))
+ }
+ }
+
+ test("SPARK-59277: Hive object inspectors reject unsupported CHAR/VARCHAR lengths") {
+ withFirstClassCharVarchar(enabled = true) {
+ Seq[DataType](CharType(0), CharType(256), VarcharType(65536)).foreach { dataType =>
Review Comment:
Added `VarcharType(0)` to both the object-inspector and type-info invalid-length checks. Fixed in 0cc7cdb2bc9.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r3969635578
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFEvaluators.scala:
##########
@@ -162,9 +162,12 @@ class HiveGenericUDFEvaluator(
}
@transient
- private lazy val unwrapper: Any => Any = unwrapperFor(returnInspector)
+ private lazy val catalystReturnType = inspectorToDataType(returnInspector)
Review Comment:
Fixed in 3b611b326d4. The analyzed GenericUDF return type and UDAF partial/final types are now retained across expression copies and evaluator reconstruction. The persisted-view regression covers first-class creation queried under legacy settings and the inverse legacy-to-first-class direction.
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala:
##########
@@ -892,6 +892,42 @@ class HiveUDFSuite extends QueryTest with TestHiveSingleton {
hiveContext.reset()
}
+ test("SPARK-59277: Hive UDF and UDTF support first-class CHAR/VARCHAR") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
Review Comment:
Added preserve-only coverage in 3b611b326d4. It asserts both CharType(5) and the padded value with standard semantics disabled.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -829,6 +864,26 @@ private[hive] trait HiveInspectors {
null
}
}
+ case (_, c: CharType) =>
Review Comment:
Updated the Scaladoc in 3b611b326d4 to document CHAR/VARCHAR read-side length and padding checks.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveScriptTransformationExec.scala:
##########
@@ -130,7 +132,7 @@ private[hive] case class HiveScriptTransformationExec(
if (dataList.get(i) == null) {
mutableRow.setNullAt(i)
} else {
- unwrappers(i)(dataList.get(i), mutableRow, i)
+ mutableRow.update(i, unwrappers(i)(dataList.get(i)))
Review Comment:
Fixed in 3b611b326d4 with a DataType-aware in-place overload. Primitive fields continue to use the specialized setters; only CHAR/VARCHAR targets use the target-aware generic conversion path.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r3962204053
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFEvaluators.scala:
##########
@@ -162,9 +162,12 @@ class HiveGenericUDFEvaluator(
}
@transient
- private lazy val unwrapper: Any => Any = unwrapperFor(returnInspector)
+ private lazy val catalystReturnType = inspectorToDataType(returnInspector)
Review Comment:
**Blocking (P1):** `catalystReturnType` is transient, so this recomputes it from the executor's current `SQLConf`; the new wrapper branches also reject an already-resolved `CharType`/`VarcharType` when the caller has first-class types disabled. A view created under the persisted standard-semantics flag can therefore be analyzed as `CHAR(5)` but execute with STRING converters, losing padding and length checks or passing a different writable class to the Hive function. Please carry the analyzed result and intermediate types through evaluator serialization and select conversion from those types rather than `SQLConf.get`, with persisted-view coverage that analyzes and executes under opposite settings.
**Recommended change:** Bind Hive conversion construction to the Catalyst types resolved during analysis.
**Why this works:** Carry the resolved GenericUDF return type and UDAF partial-result types through evaluator serialization, and make target-aware wrappers select CHAR/VARCHAR behavior from their supplied DataType rather than the execution-time SQLConf.
**Scope:** HiveInspectors conversion selection, transient Hive GenericUDF and UDAF evaluator schemas, and focused cross-session persisted-view tests.
**Compatibility:** Legacy expressions analyzed as STRING remain on the legacy representation, while expressions analyzed as CHAR/VARCHAR retain their declared length, padding, and default-collation boundary regardless of the caller session.
**Risks:** A serialized analyzed type must stay aligned with the Hive ObjectInspector used after evaluator reconstruction. Changing partial-result schema ownership must keep UDAF serialization and deserialization symmetric.
**Constraints:** Preserve the existing unsupported Hive length errors and default-collation behavior. Do not derive a different Catalyst type from mutable caller configuration after analysis.
**Success:** Persisted Hive UDF and UDAF expressions return values matching their analyzed CHAR/VARCHAR schemas when created and queried under opposite settings, while inverse legacy plans continue to use STRING conversion.
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala:
##########
@@ -892,6 +892,42 @@ class HiveUDFSuite extends QueryTest with TestHiveSingleton {
hiveContext.reset()
}
+ test("SPARK-59277: Hive UDF and UDTF support first-class CHAR/VARCHAR") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
Review Comment:
**Non-blocking (P2):** The changed guards use `charVarcharFirstClassTypes`, which is also true when `preserveCharVarcharTypeInfo` is enabled and standard semantics is false, but all SPARK-59277 integration tests set only standard semantics. Please add a preserve-only `HiveUDFSuite` case that asserts both the first-class result type and the CHAR padding/value, so a regression that narrows these branches to standard semantics cannot pass.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveScriptTransformationExec.scala:
##########
@@ -130,7 +132,7 @@ private[hive] case class HiveScriptTransformationExec(
if (dataList.get(i) == null) {
mutableRow.setNullAt(i)
} else {
- unwrappers(i)(dataList.get(i), mutableRow, i)
+ mutableRow.update(i, unwrappers(i)(dataList.get(i)))
Review Comment:
**Non-blocking (P2):** This now routes every SerDe output value through `Any => Any` and `SpecificInternalRow.update`, so primitive fields no longer use the existing `HiveStructField` unwrapper's direct `setBoolean`/`setInt`/`setLong` paths on this per-row loop. Please add a DataType-aware in-place overload that preserves the specialized primitive setters and uses target-type-aware generic conversion only for CHAR/VARCHAR and other types that need it.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -829,6 +864,26 @@ private[hive] trait HiveInspectors {
null
}
}
+ case (_, c: CharType) =>
Review Comment:
**Nit (P3):** The Scaladoc above this overload still says every non-nanosecond target delegates to the `ObjectInspector`-only overload, but these new CHAR/VARCHAR branches pad values and reject excess length. Please update the contract to describe these target-type checks.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59277][SQL][HIVE] Support first-class CHAR/VARCHAR in Hive extensions [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58584:
URL: https://github.com/apache/spark/pull/58584#discussion_r4064721234
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -1122,6 +1258,51 @@ private[hive] trait HiveInspectors {
case _: JavaVoidObjectInspector => NullType
}
+ /**
+ * Analysis snapshots the Catalyst return type, but runtime inspectors are rebuilt from the
+ * current children (including foldability and session CHAR/VARCHAR settings). STRING may drift
+ * to or from a bounded string type across that boundary. Two bounded types must match exactly:
+ * accepting a different kind or length would apply the snapshotted conversion to an incompatible
+ * runtime value.
+ */
+ def checkCompatibleHiveReturnType(
+ inspector: ObjectInspector,
+ expectedType: DataType): Unit = {
+ checkCompatibleHiveReturnType(
+ inspectorToDataType(inspector, preserveCharVarchar = true),
+ expectedType)
+ }
+
+ def checkCompatibleHiveReturnType(
+ runtimeType: DataType,
+ expectedType: DataType): Unit = {
+ if (!compatibleHiveReturnType(runtimeType, expectedType)) {
+ throw SparkException.internalError(
+ s"Hive function runtime type ${runtimeType.catalogString} is incompatible " +
+ s"with analysis type ${expectedType.catalogString}.")
+ }
+ }
+
+ private def compatibleHiveReturnType(
+ runtimeType: DataType,
+ expectedType: DataType): Boolean = {
+ (runtimeType, expectedType) match {
+ case (rt: CharType, et: CharType) => rt == et
+ case (rt: VarcharType, et: VarcharType) => rt == et
+ case (_: CharType | _: VarcharType, _: CharType | _: VarcharType) => false
+ case (_: StringType, _: CharType | _: VarcharType) => true
+ case (_: CharType | _: VarcharType, _: StringType) => true
+ case (ArrayType(rt, _), ArrayType(et, _)) => compatibleHiveReturnType(rt, et)
+ case (MapType(rk, rv, _), MapType(ek, ev, _)) =>
+ compatibleHiveReturnType(rk, ek) && compatibleHiveReturnType(rv, ev)
+ case (rt: StructType, et: StructType) if rt.length == et.length =>
+ rt.fields.zip(et.fields).forall { case (rf, ef) =>
Review Comment:
Added `rf.name == ef.name` to the struct branch of `compatibleHiveReturnType`. Added a `struct<a:INT,b:INT>` vs `struct<b:INT,a:INT>` incompatible case to the test. Fixed in cec2eff61be.
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala:
##########
@@ -897,6 +971,25 @@ private[hive] trait HiveInspectors {
(value: Any, row: InternalRow, ordinal: Int) => row(ordinal) = unwrapper(value)
}
+ /**
+ * Builds an in-place unwrapper using the target Catalyst `dataType` only when target-aware
+ * conversion is required. Other types retain the primitive setter fast paths.
+ */
+ def unwrapperFor(
Review Comment:
Removed the `unwrapperFor(HiveStructField, DataType)` overload and its test coverage (nested in-place struct test and nanosecond timestamp field test). The follow-up PR (#58936) will reintroduce them alongside their production caller. Also cleaned up the now-unused `Timestamp`, `SpecificInternalRow`, and `DateTimeUtils` imports. Added CHAR-padded map-key collision coverage. Fixed in cec2eff61be.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org