srielau commented on code in PR #58033:
URL: https://github.com/apache/spark/pull/58033#discussion_r3797856346
##########
sql/api/src/main/scala/org/apache/spark/sql/catalyst/encoders/RowEncoder.scala:
##########
@@ -91,9 +91,9 @@ object RowEncoder extends DataTypeErrorsBase {
case DoubleType => BoxedDoubleEncoder
case dt: DecimalType => JavaDecimalEncoder(dt, lenientSerialization =
true)
case BinaryType => BinaryEncoder
- case c: CharType if SqlApiConf.get.preserveCharVarcharTypeInfo =>
+ case c: CharType if SqlApiConf.get.charVarcharFirstClassTypes =>
Review Comment:
Fixed in 9f8eb8be455: `CharEncoder` / `VarcharEncoder` now take the full
`CharType` / `VarcharType` (same pattern as `GeographyEncoder`), and
`RowEncoder` passes the original constrained type through. Added a collated
`createDataFrame` assertion covering both CHAR and VARCHAR with `UTF8_LCASE`.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/util/CharVarcharCodegenUtils.java:
##########
@@ -64,4 +64,27 @@ public static UTF8String readSidePadding(UTF8String
inputStr, int limit) {
return inputStr;
}
}
+
+ /**
+ * Read-side CHAR check under standard semantics: pad to limit, or trim
trailing
+ * spaces then error if still longer than limit.
+ *
+ * Standard semantics require a read to observe the same value a write would
have
+ * produced, so this is deliberately the write-side check rather than
+ * {@link #readSidePadding}, which tolerates over-long values. Keep the two
sides
+ * identical: a fix to one is a fix to both.
+ */
+ public static UTF8String charTypeReadSideCheck(UTF8String inputStr, int
limit) {
+ return charTypeWriteSideCheck(inputStr, limit);
+ }
+
+ /**
+ * Read-side VARCHAR check under standard semantics: allow up to limit
characters,
+ * or trim trailing spaces then error if still longer than limit.
+ *
+ * Identical to the write-side check by design; see {@link
#charTypeReadSideCheck}.
Review Comment:
Fixed: the VARCHAR read-side javadoc now links to
`#varcharTypeWriteSideCheck`.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala:
##########
@@ -363,11 +363,12 @@ trait CheckAnalysis extends LookupCatalog with
QueryErrorsBase with PlanToString
plan.foreachUp {
case p if p.analyzed => // Skip already analyzed sub-plans
- case leaf: LeafNode if !SQLConf.get.preserveCharVarcharTypeInfo &&
- leaf.output.map(_.dataType).exists(CharVarcharUtils.hasCharVarchar) =>
+ case leaf: LeafNode if
leaf.output.map(_.dataType).exists(CharVarcharUtils.hasCharVarchar) &&
Review Comment:
Done: short-circuit on `!charVarcharFirstClassTypes` first, then
`leaf.output.exists(...)`.
##########
sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala:
##########
@@ -1040,6 +1210,41 @@ class FileSourceCharVarcharTestSuite extends
CharVarcharTestSuite with SharedSpa
}
}
+ test("SPARK-58801: standardSemantics scan pads CHAR and errors on oversize")
{
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ withTempPath { dir =>
+ withTable("t") {
+ sql("SELECT '12' as col").write.format(format).save(dir.toString)
+ sql(s"CREATE TABLE t (col CHAR(3)) using $format LOCATION '$dir'")
+ checkAnswer(sql("SELECT * FROM t"), Row("12 "))
+ }
+ }
+ Seq("CHAR", "VARCHAR").foreach { typ =>
+ withTempPath { dir =>
+ withTable("t") {
+ sql("SELECT '123456' as
col").write.format(format).save(dir.toString)
+ sql(s"CREATE TABLE t (col $typ(2)) using $format LOCATION '$dir'")
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ sql("SELECT * FROM t").collect()
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "2")
+ )
+ }
+ }
+ }
+ // Oversize that is only trailing blanks trims successfully.
Review Comment:
Rephrased as suggested.
--
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]
