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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
