Re: [PR] [SPARK-59917][SQL] Do not rtrim CHAR arguments to from_json, from_csv, and from_xml [spark]

["HyukjinKwon (via GitHub)" <gi...@apache.org>]

HyukjinKwon commented on code in PR #59188:
URL: https://github.com/apache/spark/pull/59188#discussion_r4156774593


##########
sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala:
##########
@@ -2444,42 +2444,23 @@ class BasicCharVarcharTestSuite extends SharedSparkSession {
 
   test("SPARK-59274: from_json/csv/xml honor CHAR/VARCHAR under standardSemantics") {
     withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
-      def checkTextInput(query: String, inputType: DataType): DataFrame = {
-        val df = sql(query)
-        val inputTypes = df.queryExecution.analyzed.expressions.flatMap(_.collect {
-          case e: SupportTrimmedCharInput => e.child.dataType
-        })
-        assert(inputTypes === Seq(inputType), df.queryExecution.analyzed)
-        df
-      }
-
+      // CHAR padding is part of the STRING value after CAST. Parsers do not rtrim it.
+      // from_json on padded CHAR must match the same document as STRING with trailing spaces.
       checkAnswer(
-        checkTextInput(
-          """SELECT from_json(CAST('{"a":1}' AS CHAR(12)), 'a INT')""",
-          CharType(12)),
-        Row(Row(1)))
+        sql("""SELECT from_json(CAST('{"a":1}' AS CHAR(12)), 'a INT')"""),
+        sql("""SELECT from_json('{"a":1}     ', 'a INT')"""))
+      // With a space delimiter, CHAR padding shows up as empty CSV fields.
       checkAnswer(
-        checkTextInput(
-          """SELECT from_csv(
+        sql(
+          """SELECT schema_of_csv(

Review Comment:
   **Nit (P3):** This line used to be the `from_csv(CAST('1' AS CHAR(3)), '_c0 INT', map('delimiter', ' ', 'mode', 'FAILFAST'))` check. That query is the one whose behavior changes most visibly in this PR: the padded `'1  '` now tokenizes into 3 fields against a 1-field schema, and `UnivocityParser.convert` treats a token-count mismatch as malformed, so under FAILFAST it should now raise the malformed CSV record error instead of returning `Row(Row(1))`. With this case and the `CsvToStructs` CHAR case in `CsvExpressionsSuite` both removed, nothing exercises `from_csv` with a padded CHAR document. `schema_of_csv` goes through `SchemaOfCsv`, and the `from_json` check above passes with or without trimming. Could you keep the `from_csv` case and assert the new outcome, e.g. `intercept` the malformed-record error, or check the PERMISSIVE result? An expression-level `CsvToStructs` check with a `CharType` literal would also cover the codegen path that `doGenCode` now takes via `child.genCode
 `.
   
   **Verification:**
   - **Regression:** Under spark.sql.charVarchar.standardSemantics.enabled, from_csv(CAST('1' AS CHAR(3)), '_c0 INT', map('delimiter', ' ', 'mode', 'FAILFAST')) is asserted to raise the malformed CSV record error (or an equivalent untrimmed-input outcome is asserted), in both interpreted and codegen evaluation where an expression-level check is added.



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