HyukjinKwon commented on code in PR #59316:
URL: https://github.com/apache/spark/pull/59316#discussion_r4239821272
##########
docs/sql-migration-guide.md:
##########
@@ -29,6 +29,7 @@ license: |
## Upgrading from Spark SQL 4.3 to 4.4
- Since Spark 4.4, when `spark.sql.preserveCharVarcharTypeInfo` is true and
`spark.sql.charVarchar.standardSemantics.enabled` is false, ORC reads that
apply a CHAR/VARCHAR schema over STRING storage return the stored values
without ORC truncation, matching Parquet. Previously the ORC reader requested
`char(n)`/`varchar(n)` and truncated STRING-stored values to `n`. Read-side
length checks (`EXCEED_LIMIT_LENGTH`) apply only when
`spark.sql.charVarchar.standardSemantics.enabled` is true.
+- Since Spark 4.4, when `spark.sql.charVarchar.standardSemantics.enabled` is
true, XML names used as `MAP<CHAR(n), _>` or `MAP<VARCHAR(n), _>` keys in
`from_xml` and the XML datasource are length-checked without padding or
trimming. The checked names are element names, `attributePrefix` plus attribute
names (default prefix `_`), and the `valueTag` (default `_VALUE`) when mixed
text is present. A `CHAR(n)` key must already be exactly `n` characters, and a
`VARCHAR(n)` key must already be at most `n` characters. Mismatched keys fail
with `UNSUPPORTED_XML_CHAR_VARCHAR_MAP_KEY` (SQLSTATE `0A000`) and follow the
XML parse mode (`PERMISSIVE` or `FAILFAST`). Exact repeated names last-win;
`spark.sql.mapKeyDedupPolicy` is not applied. XML CHAR/VARCHAR values use the
same pad and `EXCEED_LIMIT_LENGTH` checks as other parsed text. Empty,
attribute-only, and text-only map elements stay SQL NULL or a malformed record,
matching 4.3; they do not become an empty map or a `valueTag` entry. In Spark
4.3, `MAP<CHAR(n), _>` and `MAP<VARCHAR(n), _>` XML maps were malformed
records because only unbounded STRING keys were accepted. Padding of CHAR keys
and VARCHAR `EXCEED_LIMIT_LENGTH` apply when the standard-semantics flag is
false and `spark.sql.preserveCharVarcharTypeInfo` is true, so first-class
CHAR/VARCHAR types still reach the parser.
Review Comment:
**Nit (P3):** Two statements in this bullet don't match the code.
- "In Spark 4.3, `MAP<CHAR(n), _>` and `MAP<VARCHAR(n), _>` XML maps were
malformed records": in branch-4.3, `from_xml` (via `ExprUtils.evalTypeExpr`)
and `DataFrameReader.schema` both call `failIfHasCharVarchar`. It throws
`UNSUPPORTED_CHAR_OR_VARCHAR_AS_STRING` unless
`spark.sql.legacy.charVarcharAsString` is true, in which case it replaces
CHAR/VARCHAR with STRING and the map parses as a plain STRING map. So for the
entry points this bullet names, 4.3 gave an analysis error (or a STRING map),
not a malformed record. A malformed record only happened where a CHAR/VARCHAR
key type actually reached the parser. My earlier review described 4.3 from the
parser dispatch alone, which is where this sentence came from.
- "Padding of CHAR keys and VARCHAR `EXCEED_LIMIT_LENGTH` apply when the
standard-semantics flag is false ...": with the flag off, an over-length CHAR
key also raises `EXCEED_LIMIT_LENGTH`. `charTypeWriteSideCheck` passes it to
`trimTrailingSpaces`, and XML names have no trailing spaces, so `<abcd>` under
`CHAR(3)` fails, for example. The `convertMap` scaladoc ("With the flag off,
CHAR keys are padded and VARCHAR overflow uses EXCEED_LIMIT_LENGTH") has the
same gap.
Describing the 4.3 behavior per entry point, and saying in both places that
over-length CHAR or VARCHAR keys raise `EXCEED_LIMIT_LENGTH` with the flag off,
would make this accurate.
**Verification:**
- **Inspection:** Each statement in the bullet and scaladoc matches
upstream/branch-4.3's failIfHasCharVarchar handling and the head's
convertXmlMapKey/charTypeWriteSideCheck behavior for both flag values.
- **Behavior:** The flag-off test coverage asserts EXCEED_LIMIT_LENGTH for
an over-length CHAR key, alongside the existing padding and VARCHAR overflow
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: [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]
