cloud-fan commented on code in PR #58224:
URL: https://github.com/apache/spark/pull/58224#discussion_r4322260540
##########
docs/sql-migration-guide.md:
##########
@@ -29,9 +29,12 @@ license: |
## Upgrading from Spark SQL 4.3 to 4.4
- Since Spark 4.4, `spark.sql.execution.arrow.pythonUDF.columnarInput.enabled`
is `false` by default: Arrow-based Python UDFs read rows from an Arrow-backed
upstream operator, such as a Data Source V2 scan or the Arrow cache, as before
Spark 4.2. With columnar input, they kept the columns that they pass through
until the results of their batch came back from Python, and failed or returned
wrong results when the operator reused or released the vectors of the batch in
the meantime. To enable columnar input again, set
`spark.sql.execution.arrow.pythonUDF.columnarInput.enabled` to `true`.
-- Since Spark 4.4, when `spark.sql.charVarchar.standardSemantics.enabled` is
true, `from_json` length-checks JSON object names used as `MAP<CHAR(n), _>` or
`MAP<VARCHAR(n), _>` keys without padding or trimming them. A `CHAR(n)` key
must already be exactly `n` characters, and a `VARCHAR(n)` key must already be
at most `n` characters; duplicate names are kept, exactly as for STRING keys,
and `spark.sql.mapKeyDedupPolicy` is not applied. A key that fails the check
makes the enclosing map a bad record: in `PERMISSIVE` mode the nearest
enclosing struct field becomes `null` (its sibling fields are preserved), and
if the map is inside an array or another map, that whole array or map becomes
`null`; in `FAILFAST` mode parsing fails with
`UNSUPPORTED_JSON_CHAR_VARCHAR_MAP_KEY` (SQLSTATE `0A000`) as the cause of
`MALFORMED_RECORD_IN_PARSING`. The same parser-level check runs for any JSON
scan whose reader schema carries a CHAR/VARCHAR map key, including catalog JSON
tables, since under standa
rd semantics those keys stay CHAR/VARCHAR all the way to the scan: an
off-width key becomes a bad record there too, rather than being padded or
raising `EXCEED_LIMIT_LENGTH`. Unlike `from_json`, keys that pass the check are
then rebuilt read-side, so on the scan path `spark.sql.mapKeyDedupPolicy` does
apply and a duplicate name can raise `DUPLICATED_MAP_KEY`. JSON file scans,
including scans inside persisted views, read the flag from the executing
session.
+- Since Spark 4.4, `spark.sql.charVarchar.standardSemantics.enabled` (default
`false`) is a preview feature that may change in future releases. When true, it
enables first-class `CHAR(n)` and `VARCHAR(n)` semantics throughout schemas and
query plans: `CHAR` -> `VARCHAR` -> `STRING` form a type-precedence family,
explicit casts can return constrained types, least-common-type resolution for
`COALESCE`/`CASE`/`UNION` can return `CHAR` or `VARCHAR`, and string
expressions that transform their input return `STRING`. Assignment and reads
enforce the declared length, padding `CHAR` to `n` characters and raising
`EXCEED_LIMIT_LENGTH` when non-space content does not fit. The default remains
the Spark 3.1 table-schema overlay on `STRING`. This opt-in mode supersedes the
experimental `spark.sql.preserveCharVarcharTypeInfo`, which instead keeps
constrained types through transforming expressions. See [Character String
Types](sql-ref-character-string-types.html) for the preview semantics; the ent
ries below note area-specific behavior that changed. Those later CHAR/VARCHAR
bullets apply only when this preview flag is true, except where they name
`preserveCharVarcharTypeInfo` alone.
+- Since Spark 4.4, when `spark.sql.charVarchar.standardSemantics.enabled` is
true, `from_json` length-checks JSON object names used as `MAP<CHAR(n), _>` or
`MAP<VARCHAR(n), _>` keys without padding or trimming them. A `CHAR(n)` key
must already be exactly `n` characters, and a `VARCHAR(n)` key must already be
at most `n` characters; duplicate names are kept, exactly as for STRING keys,
and `spark.sql.mapKeyDedupPolicy` is not applied. A key that fails the check
makes the enclosing map a bad record: in `PERMISSIVE` mode the nearest
enclosing struct field becomes `null` (its sibling fields are preserved), and
if the map is inside an array or another map, that whole array or map becomes
`null`; in `FAILFAST` mode parsing fails with
`UNSUPPORTED_JSON_CHAR_VARCHAR_MAP_KEY` (SQLSTATE `0A000`) as the cause of
`MALFORMED_RECORD_IN_PARSING`. The same parser-level check runs for any JSON
scan whose reader schema carries a CHAR/VARCHAR map key, including catalog JSON
tables, since under standa
rd semantics those keys stay CHAR/VARCHAR all the way to the scan: an
off-width key becomes a bad record there too, rather than being padded or
raising `EXCEED_LIMIT_LENGTH`. Unlike `from_json`, keys that pass the check are
then rebuilt read-side, so on the scan path `spark.sql.mapKeyDedupPolicy` does
apply and a duplicate name can raise `DUPLICATED_MAP_KEY`. JSON file scans,
including scans inside persisted views, read the flag from the executing
session. The optimizer no longer folds `from_json(to_json(m))` to `m` when that
schema has a CHAR/VARCHAR map key, so the length check still runs on the round
trip.
+- Since Spark 4.4, `from_json`, `from_csv`, `from_xml`, and the corresponding
`schema_of_*` functions do not rtrim a CHAR document argument. CHAR padding is
part of the STRING value after CAST, so it is parsed as trailing spaces in the
document. JSON and XML tolerate that trailing whitespace; CSV can tokenize it
into extra fields or attach it to the last unquoted field (use
`ignoreTrailingWhiteSpace` to restore a trimmed last-field parse).
- 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;
they do not become an empty map or a `valueTag` entry. In Spark 4.3, `from_xm
l` and XML reader schemas rejected CHAR/VARCHAR with
`UNSUPPORTED_CHAR_OR_VARCHAR_AS_STRING`, or replaced them with STRING when
`spark.sql.legacy.charVarcharAsString` was true, so CHAR/VARCHAR map keys did
not reach the parser. When the standard-semantics flag is false and
`spark.sql.preserveCharVarcharTypeInfo` is true, short CHAR keys are padded and
over-length CHAR or VARCHAR keys raise `EXCEED_LIMIT_LENGTH`.
+- Since Spark 4.4, when `spark.sql.charVarchar.standardSemantics.enabled` is
true, `from_avro` assigns CHAR/VARCHAR on STRING values (pad, then
trim-and-`EXCEED_LIMIT_LENGTH`; Avro ENUM symbols are not assigned). Avro map
keys of CHAR/VARCHAR are length-checked without padding or trimming in
`from_avro` and Avro file scans, matching JSON/XML object names; an off-width
key fails with `UNSUPPORTED_AVRO_CHAR_VARCHAR_MAP_KEY` (SQLSTATE `0A000`).
`from_avro` PERMISSIVE nulls that malformed record; file scans do not wrap
per-row PERMISSIVE around the error. `to_avro` does not pad or trim. Avro file
scans still leave CHAR padding of STRING values to `ApplyCharTypePadding`.
Review Comment:
**Nit (P3):** `ApplyCharTypePadding` is an internal analyzer rule. No other
docs page mentions it, and users can't look it up or configure it. The new
reference page describes the same Avro scan behavior as "read-side CHAR
padding" (`docs/sql-ref-character-string-types.md:300-301`). Could this
sentence use that term too? For example: "Avro file scans still leave CHAR
padding of STRING values to read-side CHAR padding rather than the
deserializer."
**Verification:**
- **Inspection:** The from_avro migration bullet describes Avro scan CHAR
padding with the same user-facing term as the reference page, and the migration
guide names no internal analyzer rule class for this behavior.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/csvExpressions.scala:
##########
@@ -152,6 +160,10 @@ case class CsvToStructs(
> SELECT _FUNC_('1,abc');
STRUCT<_c0: INT, _c1: STRING>
""",
+ note = """
+ A CHAR `csv` argument is not rtrimmed. Padding can change the inferred
field count and
+ types; for example CAST('1' AS CHAR(3)) infers DOUBLE with the default
comma delimiter.
Review Comment:
**Nit (P3):** This example only holds when CHAR is first-class. With default
settings, `CharVarcharUtils.replaceCharVarcharWithStringForCast` rewrites
`CAST('1' AS CHAR(3))` to a cast to STRING with no padding. `schema_of_csv`
then sees `'1'` and returns `STRUCT<_c0: INT>`.
The only test asserting DOUBLE (the SPARK-59274 test in
`CharVarcharTestSuite`) runs with standard semantics on, and note text isn't
executed by the example runners. The PySpark `schema_of_csv` docstring already
puts this remark under the preview-flag bullet. Could this note be qualified
the same way, e.g. "when CHAR is first-class, CAST('1' AS CHAR(3)) infers
DOUBLE with the default comma delimiter"?
See **Shared repair plan 1** in the review body.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala:
##########
@@ -1926,6 +1926,14 @@ object JsonExistsExpressionBuilder extends
JsonPathExpressionBuilder {
> SELECT _FUNC_('{"teacher": "Alice", "student": [{"name": "Bob",
"rank": 1}, {"name": "Charlie", "rank": 2}]}', 'STRUCT<teacher: STRING,
student: ARRAY<STRUCT<name: STRING, rank: INT>>>');
{"teacher":"Alice","student":[{"name":"Bob","rank":1},{"name":"Charlie","rank":2}]}
""",
+ note = """
+ A CHAR `jsonStr` is not rtrimmed; padding is parsed as trailing whitespace
in the JSON
+ text. CHAR/VARCHAR in `schema` requires the preview flag
+ spark.sql.charVarchar.standardSemantics.enabled: fields are assigned (CHAR
padded;
+ overflow is a bad record) and map keys are length-checked without padding
or trimming.
+ Without the flag, CHAR/VARCHAR in `schema` raises
Review Comment:
**Non-blocking (P2):** This says a CHAR/VARCHAR `schema` needs the preview
flag and otherwise raises `UNSUPPORTED_CHAR_OR_VARCHAR_AS_STRING`. That only
holds for the default configuration. The schema goes through
`ExprUtils.evalTypeExpr` -> `CharVarcharUtils.failIfHasCharVarchar`, which:
- keeps CHAR/VARCHAR whenever `charVarcharFirstClassTypes` is true, i.e.
`spark.sql.preserveCharVarcharTypeInfo` OR standard semantics
(`SparkCharVarcharUtils.scala:47-55`, `SqlApiConf.scala:51-52`);
- replaces them with STRING, without an error, when
`spark.sql.legacy.charVarcharAsString` is true;
- raises only when all three are off.
Value assignment (padding, `EXCEED_LIMIT_LENGTH`) is not gated on standard
semantics either. `JacksonParser.scala:379`, `UnivocityParser.scala:283`, and
`AvroDeserializer.scala:310-311` call `applyTextParseSemantics`
unconditionally, and the comment at `OptimizeCsvJsonExprs.scala:482-485` says
so explicitly. Only the map-key length checks depend on the preview flag.
Existing tests pin both counterexamples. In `CharVarcharTestSuite`
("invalidate char/varchar in functions"), legacy mode returns `Row(Row("str"))`
typed STRING with no error. In `AvroFunctionsSuite`, the preserve-only block
shows `from_avro` into `CHAR(4)` yielding `<ab >`. This PR's own page (lines
322-323) and the XML migration bullet also describe preserve-only `from_xml`
padding CHAR map keys, which couldn't happen if the schema raised.
The same two-state wording appears in:
- the `from_xml`, `from_csv`, and `from_avro` notes;
- the PySpark `from_json`/`from_xml`/`from_csv`/`from_avro` docstrings;
- the 'Without the flag' sentences on the new page (lines 31-33),
`sql-ref-literals.md:94`, `sql-ref-parameter-markers.md:43`, and
`sql-ref-ansi-compliance.md:304-305`.
Could the function notes name the actual condition? For example: accepted
and assigned when CHAR/VARCHAR are first-class (this flag or
`spark.sql.preserveCharVarcharTypeInfo`); read as STRING when
`spark.sql.legacy.charVarcharAsString` is true; otherwise raises
`UNSUPPORTED_CHAR_OR_VARCHAR_AS_STRING`; map-key length checks require this
flag. On the reference pages, saying 'by default' instead of 'without the flag'
would be enough.
See **Shared repair plan 1** in the review body.
##########
docs/sql-ref-parameter-markers.md:
##########
@@ -29,6 +29,21 @@ You cannot mix named and unnamed parameter markers in the
same SQL statement.
Parameter markers can be used wherever literal values are allowed in the SQL
syntax.
+The marker syntax (`:name` or `?`) does not declare a type. The type is the
type of the
+bound value:
+
+* In SQL, `USING` binds a [constant
expression](sql-ref-syntax-aux-exec-imm.html).
+ A [string literal](sql-ref-literals.html#string-literal) such as `'ab'` is
`STRING`.
+ Use a typed literal (`5L`, `DATE '2024-01-01'`) or `CAST` to choose another
type.
+* In Python, Scala, or Java `spark.sql(..., args)`, Spark infers the type from
the host
+ value. A Python or Java `str` / `String` is `STRING`. You may pass a
constant `Column`,
+ including `lit('ab').cast('CHAR(2)')`.
Review Comment:
**Non-blocking (P2):** Passing `lit('ab').cast('CHAR(2)')` through
`spark.sql(..., args)` fails with `INVALID_SQL_ARG` in every configuration.
That breaks both this sentence and the Python example below that shows
`char(2)`.
The path:
1. PySpark sends `_to_java_column(lit(v))`, and `lit` returns a Column
unchanged, so the JVM receives a `Cast`.
2. `SparkSession.resolveAndValidateParameters` analyzes the args and runs
`ParameterizedQueryArgumentsValidator` before any constant folding.
3. The validator allows only `Literal`, `CreateArray`, `CreateNamedStruct`,
`CreateMap`, `MapFromArrays`, `MapFromEntries`, `VariableReference`, and
`Alias`, so the `Cast` is rejected. `ParametersSuite` asserts the same for
`str_to_map`, and Spark Connect goes through the same method.
The `EXECUTE IMMEDIATE ... USING CAST(...)` examples are fine, because
`ParameterBindingUtils.evaluateParameterExpression` folds them into typed
literals first.
Suggest saying host-API args must be literal-like constants (literals and
array/map/struct constructors of them), and changing the Python example to cast
in the SQL text, e.g. `spark.sql("SELECT TYPEOF(CAST(:p AS CHAR(2)))",
args={"p": "ab"})`.
**Verification:**
- **Behavior:** With the preview flag on, a Python spark.sql call that binds
the string 'ab' and selects TYPEOF(CAST(:p AS CHAR(2))) returns char(2),
matching the corrected example.
- **Regression:** A spark.sql call that passes lit('ab').cast('CHAR(2)') as
a named argument fails with INVALID_SQL_ARG, and no remaining sentence or
example on the page presents that form as supported.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/avroSqlFunctions.scala:
##########
@@ -58,6 +58,10 @@ import org.apache.spark.util.Utils
is undefined: it may fail or return arbitrary result.
To deserialize the data with a compatible and evolved schema, the expected
Avro schema can be
set via the corresponding option.
+ CHAR/VARCHAR assignment and map-key checks require the preview flag
+ spark.sql.charVarchar.standardSemantics.enabled. Then STRING values are
assigned to
+ CHAR/VARCHAR (CHAR padded; overflow raises EXCEED_LIMIT_LENGTH). Avro ENUM
symbols are
Review Comment:
**Nit (P3):** `from_avro` doesn't surface `EXCEED_LIMIT_LENGTH` directly.
`AvroDataToCatalyst.nullSafeEval` catches any `NonFatal` deserializer error:
- In FAILFAST, the default (`AvroOptions.parseMode`), it throws the generic
"Malformed records are detected in record parsing" `SparkException`, with
`EXCEED_LIMIT_LENGTH` only as the cause.
- In PERMISSIVE it returns `nullResultRow`, which nulls every field of a
struct result, not just the over-length one.
So with `struct<a: CHAR(4), b: INT>` and an over-length `a`, PERMISSIVE
gives `{a: null, b: null}`.
The new page's "Parsed values" paragraph (`PERMISSIVE nulls the failed
field`, `docs/sql-ref-character-string-types.md:296`) has the same problem for
`from_avro`. It also disagrees with the page's own Avro map-key bullet, which
says PERMISSIVE "nulls that malformed record".
Maybe phrase it as: overflow is a malformed record handled by the `mode`
option (FAILFAST fails with `EXCEED_LIMIT_LENGTH` as the cause; PERMISSIVE
returns a null record). Then add a matching `from_avro` carve-out to the Parsed
values paragraph.
**Verification:**
- **Behavior:** For from_avro into a struct with a CHAR(4) field and an INT
field, an over-length CHAR value under FAILFAST fails with the malformed-record
error caused by EXCEED_LIMIT_LENGTH, and under PERMISSIVE yields a record whose
fields are all null, consistent with the revised wording.
- **Inspection:** The revised from_avro wording matches the parse-mode
handling of deserializer errors and is consistent with the map-key paragraph's
statement that PERMISSIVE nulls the malformed record.
--
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]
