cloud-fan commented on code in PR #58224:
URL: https://github.com/apache/spark/pull/58224#discussion_r4336710538
##########
docs/sql-ref-parameter-markers.md:
##########
@@ -29,6 +29,24 @@ 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.
+ CHAR and VARCHAR are the exception: a `USING` value such as `CAST('ab' AS
CHAR(2))`
Review Comment:
**Nit (P3):** This is true for a CHAR/VARCHAR value without an explicit
collation, but not for one that has one. `ParameterHandler.convertToSql`
substitutes plain `Literal.sql` (`'ab'`, which re-parses as STRING) only when
`hasNonDefaultStringCharOrVarcharType` is false. For `USING CAST('ab' AS
CHAR(2) COLLATE UTF8_LCASE) AS p`, or an explicit `COLLATE UTF8_BINARY`, it
substitutes `CAST('ab' AS CHAR(2) COLLATE UTF8_LCASE)` instead. So under the
preview, `TYPEOF(:p)` is `char(2) collate UTF8_LCASE`, not `string`. I traced
this from source and did not run it.
The same unqualified statement appears at line 198 of this page and in the
`arg_expr` description on the EXECUTE IMMEDIATE page. Limiting all three to
values without an explicit collation would make them accurate, for example "a
CHAR or VARCHAR value without an explicit collation currently binds as STRING".
The advice to cast the marker in the SQL text still applies.
**Verification:**
- **Behavior:** With the preview flag on, TYPEOF of a marker bound through
EXECUTE IMMEDIATE USING to a CHAR(2) value without explicit collation reports
string, and one bound to an explicitly collated CHAR(2) value reports the
collated char(2) type. The revised wording must describe both outcomes.
- **Inspection:** Each revised binding statement agrees with how parse-time
substitution renders CHAR/VARCHAR literals with and without an explicit
collation.
##########
docs/sql-ref-ansi-compliance.md:
##########
@@ -198,6 +202,14 @@ While casting of a decimal with a fraction to an interval
type with SECOND as th
### Store assignment
As mentioned at the beginning, when `spark.sql.storeAssignmentPolicy` is set
to `ANSI`(which is the default value), Spark SQL complies with the ANSI store
assignment rules on table insertions. The valid combinations of source and
target data type in table insertions are given by the following table.
+Table CHAR/VARCHAR columns apply length checks, trailing-space trimming, and
CHAR padding on
+insert in the default overlay. CHAR/VARCHAR that survive in query plans
require first-class
+CHAR/VARCHAR types (the preview flag or
`spark.sql.preserveCharVarcharTypeInfo`). Distinct
+CAST and assignment semantics require the preview flag: an explicit CAST
truncates or pads to
Review Comment:
**Nit (P3):** This holds only when the source is already a character string
type. `ToStringBase.castToString` uses the truncating
`charTypeCast`/`varcharTypeCast` only for a STRING/CHAR/VARCHAR source. Every
other source gets the same write-side check as assignment. So under the
preview, `CAST(12345 AS VARCHAR(4))` raises `EXCEED_LIMIT_LENGTH` and
`TRY_CAST(12345 AS VARCHAR(4))` returns NULL, as pinned in
`charvarchar-standard-semantics.sql.out`; neither one truncates to `'1234'`.
This paragraph sits above a table of numeric, datetime, boolean, and other
source types, so readers will take it to cover those too. It also disagrees
with the Casts section of the Character String Types page, which gets the split
right. Maybe: "an explicit CAST from a character string type truncates or pads
to the length; a CAST from any other type, like assignment, raises
`EXCEED_LIMIT_LENGTH` when the value does not fit".
**Verification:**
- **Inspection:** The revised paragraph separates casts from character
string types (truncate or pad) from casts of non-character values
(assignment-style length check that raises EXCEED_LIMIT_LENGTH, or NULL under
TRY_CAST). This must agree with the CAST(12345 AS VARCHAR(4)) and
TRY_CAST(12345 AS VARCHAR(4)) results under the preview flag.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/avroSqlFunctions.scala:
##########
@@ -58,6 +58,16 @@ 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 come from the spark.sql.catalyst.type property on Avro string
fields
Review Comment:
**Nit (P3):** This covers fields and values, but not the map keys the note
goes on to describe. Avro map keys have no schema of their own.
`SchemaConverters` therefore stamps a CHAR/VARCHAR key type as
`spark.sql.catalyst.mapKey.type` on the enclosing map schema and reads the key
type only from there (`SchemaConverters.scala:98-115` and `304-308`),
defaulting to STRING. A user writing a `jsonFormatSchema` from this note cannot
tell how to declare a CHAR key. Without that property, the map-key check
described below never applies.
Could this name the map-key property too? For example: "CHAR/VARCHAR come
from the spark.sql.catalyst.type property on Avro string fields, and for map
keys from spark.sql.catalyst.mapKey.type on the Avro map schema (both written
by Spark's Avro writer)". The PySpark `from_avro` docstring has the same
sentence.
This extends an [existing
discussion](https://github.com/apache/spark/pull/58224#discussion_r4330347828).
**Verification:**
- **Inspection:** The provenance text matches the property names the Avro
schema converter writes and reads for string fields and for map keys.
- **Inspection:** The edited from_avro note keeps a leading newline with
four-space indentation and a trailing newline with two-space indentation, so
built-in function registration accepts it.
##########
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:
Thanks, verified at the current head: the from_json/from_csv/from_xml notes
and docstrings now describe the three-state gate (first-class via either flag,
legacy as-string, otherwise UNSUPPORTED_CHAR_OR_VARCHAR_AS_STRING), map-key
checks stay on the preview flag, and the reference pages say "by default".
from_avro is correctly two-state now: its Avro schema never goes through
failIfHasCharVarchar, so my suggestion above to give it the three-state wording
was wrong.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4322260523","thread_id":"inline:4322260523","verdict_sha256":"685b9d18cefdcd97fe201d9358349bab2297d25842c2185c234667e6907a8e28"}
-->
##########
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:
Thanks, verified: the from_avro note and the Parsed values paragraph now
describe overflow as a malformed record (FAILFAST with EXCEED_LIMIT_LENGTH as
the cause, PERMISSIVE as a struct with every field NULL), and the migration
bullet uses the same PERMISSIVE wording.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4322260534","thread_id":"inline:4322260534","verdict_sha256":"685b9d18cefdcd97fe201d9358349bab2297d25842c2185c234667e6907a8e28"}
-->
##########
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:
Thanks, verified: the migration bullet now uses the user-facing term
"read-side CHAR padding" instead of the internal rule name.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4322260540","thread_id":"inline:4322260540","verdict_sha256":"685b9d18cefdcd97fe201d9358349bab2297d25842c2185c234667e6907a8e28"}
-->
##########
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:
Thanks, verified: the page now limits host spark.sql args to literal-like
constants, calls out lit(...).cast(...) as INVALID_SQL_ARG, and the Python
example casts the marker in the SQL text. I was also wrong above that the
EXECUTE IMMEDIATE ... USING CAST(...) examples were fine: parameter
substitution renders a default-collation CHAR/VARCHAR value back as a plain
string literal, so it binds as STRING, and removing those examples was right.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4322260542","thread_id":"inline:4322260542","verdict_sha256":"685b9d18cefdcd97fe201d9358349bab2297d25842c2185c234667e6907a8e28"}
-->
##########
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:
Thanks, verified: the schema_of_csv note now qualifies the DOUBLE inference
to first-class CHAR, and the family page states the default-mode INT result.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4322260550","thread_id":"inline:4322260550","verdict_sha256":"685b9d18cefdcd97fe201d9358349bab2297d25842c2185c234667e6907a8e28"}
-->
--
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]
