You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "srielau (via GitHub)" <gi...@apache.org> on 2026/08/16 18:26:44 UTC
[PR] [SPARK-58794][SQL] Add standardSemantics foundation for CHAR/VARCHAR [spark]
srielau opened a new pull request, #58033:
URL: https://github.com/apache/spark/pull/58033
### What changes were proposed in this pull request?
This is the foundation for SQL standard–aligned `CHAR(n)` / `VARCHAR(n)` under parent [SPARK-58794](https://issues.apache.org/jira/browse/SPARK-58794) (subtasks SPARK-58796–SPARK-58801).
Adds `spark.sql.charVarchar.standardSemantics.enabled` (default `false`). When enabled (or together with `preserveCharVarcharTypeInfo` for first-class typing):
- **SPARK-58796**: CHAR/VARCHAR may appear as first-class types in schemas/plans (`charVarcharFirstClassTypes`)
- **SPARK-58797**: CAST / try_cast keep CHAR/VARCHAR targets and apply character-length pad/trim/`EXCEED_LIMIT_LENGTH`
- **SPARK-58798**: LCT lattice `CHAR → VARCHAR → STRING` with `max(n, m)` for COALESCE/CASE/UNION/etc.
- **SPARK-58799**: Transforming string expressions (`upper`, `substr`, `concat`/`||`, …) return plain `STRING` (R1)
- **SPARK-58800**: Write-side checks return CharType/VarcharType when first-class
- **SPARK-58801**: Read-side CHAR padding forced under standardSemantics; oversize after trailing-blank trim errors
`preserveCharVarcharTypeInfo` remains the Spark 4.0 experimental leaky path; only `standardSemantics` applies R1.
### Why are the changes needed?
Today CHAR/VARCHAR are largely annotated STRING: CAST stringifies targets, LCT always widens to STRING when preserve is off, and transforming ops can leak length types when preserve is on. Standard semantics need a gated, consistent path.
### Does this PR introduce _any_ user-facing change?
Yes, when `spark.sql.charVarchar.standardSemantics.enabled` is set to `true` (default remains `false`):
- CAST AS CHAR/VARCHAR preserves typed results and enforces length
- COALESCE/CASE/UNION may return CHAR/VARCHAR
- string transforms return STRING
- scans pad CHAR and reject oversize values
### How was this patch tested?
- New cases in `CharVarcharTestSuite` / `BasicCharVarcharTestSuite` / `FileSourceCharVarcharTestSuite` covering CAST, LCT, R1, createDataFrame, and scan pad/oversize
- Local SBT run was blocked by Maven Central network access in this environment; please verify with:
```
./build/sbt "sql/testOnly *BasicCharVarcharTestSuite *FileSourceCharVarcharTestSuite"
```
--
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
Re: [PR] [SPARK-58794][SQL] Add standardSemantics foundation for CHAR/VARCHAR [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on PR #58033:
URL: https://github.com/apache/spark/pull/58033#issuecomment-5320669756
/trigger all
--
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
Re: [PR] [SPARK-58794][SQL] Add standardSemantics foundation for CHAR/VARCHAR [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58033:
URL: https://github.com/apache/spark/pull/58033#discussion_r3801134189
##########
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/RowSetUtils.scala:
##########
@@ -147,8 +147,12 @@ object RowSetUtils {
// types that reach this branch do not use the `nested` flag in `toHiveString`. Now,
// Geospatial types use it for wrapping EWKT in quotes when nested = true, so we need
// to set `nested` here to false to avoid spurious quotes for standalone geo values.
+ // String types need the same treatment: the fast path above matches only the
+ // default-collation StringType singleton, so CHAR/VARCHAR and collated strings land
+ // here and would otherwise be rendered as "value" instead of value.
val nested = typ match {
case _: GeometryType | _: GeographyType => false
+ case _: StringType => false
Review Comment:
Correct that it is broader than the flag, and deliberately so: only the default-collation `StringType` singleton matches the fast path above, so collated strings have always fallen into the generic branch and been rendered with quotes. Gating the fix on `standardSemantics` would leave that bug in place for collated strings, which have nothing to do with CHAR/VARCHAR.
Added `RowSetUtilsSuite`, which calls `toTRowSet` directly and asserts unquoted rendering for `CharType`, `VarcharType`, a `UTF8_LCASE` string and plain `StringType`. I verified it is actually load-bearing: with the `case _: StringType => false` line removed it fails with `List("\"ab\"") did not equal List("ab")`. I put it at that level rather than in the JDBC suites because it pins exactly the changed branch and runs in a second; the CHAR/VARCHAR end-to-end path stays covered by the golden file through `ThriftServerQueryTestSuite`, which is how the quoting bug surfaced in the first place.
I will call the collated-string fix out in the PR description. Happy to split it into a standalone bugfix if you prefer the cleaner changelog -- it is self-contained now that the test is separate.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala:
##########
@@ -453,7 +454,8 @@ trait String2StringExpression extends ImplicitCastInputTypes {
def convert(v: UTF8String): UTF8String
- override def dataType: DataType = child.dataType
+ override def dataType: DataType =
+ StringHelper.transformingStringResultType(child.dataType)
Review Comment:
Agreed -- "safe by accident" is exactly right, and relying on the `dataType == StringType` guard in `V1Writes` to keep it safe is not something I want to depend on. `Empty2Null` now overrides `dataType` to return the child's type, with a comment saying why it does not take the trait's R1 result. Kept it on the trait rather than moving it off, since it is a string-to-string function in every other respect.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1430,7 +1430,10 @@ case class Reverse(child: Expression)
BinaryType,
ArrayType))
- override def dataType: DataType = child.dataType
+ // Reversing a string transforms its content, so a CHAR/VARCHAR input yields plain STRING (R1).
+ // Array and binary inputs are unaffected. The promotion in ImplicitTypeCasts does not reach
+ // here because the expected type is a TypeCollection rather than a plain string type.
Review Comment:
You are right, and the comment taught the wrong invariant: `expectsStringType` recurses into `TypeCollection`, so `Reverse`'s string branch does get promoted by `ImplicitTypeCasts`. Reworded to say the override covers the paths that do not go through implicit casting (an expression built directly), rather than claiming promotion cannot reach it.
--
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
Re: [PR] [SPARK-58794][SQL] Add standardSemantics foundation for CHAR/VARCHAR [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #58033:
URL: https://github.com/apache/spark/pull/58033#discussion_r3797675285
##########
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:
**Blocking:**
Pass the full `CharType`/`VarcharType` into the leaf encoders instead of only the length. `CharEncoder(length)` reconstructs a default-collation type, and `EncoderField.structField` builds the resulting schema from that encoder type, so `createDataFrame` loses an explicitly declared collation on this newly enabled path. Please retain the original constrained type and add a collated CHAR/VARCHAR encoder test.
##########
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:
**Non-blocking:**
Put `!SQLConf.get.charVarcharFirstClassTypes` first and use `leaf.output.exists(attr => CharVarcharUtils.hasCharVarchar(attr.dataType))`. In the newly enabled mode this guard can never match, but the current order still maps and scans every leaf output before rejecting the case.
##########
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:
**Nit:**
This should link to `#varcharTypeWriteSideCheck`, which is the write-side method this VARCHAR helper actually delegates to. The current CHAR read-side link points readers to the wrong contract.
##########
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:
**Nit:**
Please rephrase this as `An oversized value consisting only of trailing blanks is trimmed successfully.` The current sentence uses `oversize` as a noun and is hard to parse.
--
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
Re: [PR] [SPARK-58794][SQL] Add standardSemantics foundation for CHAR/VARCHAR [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58033:
URL: https://github.com/apache/spark/pull/58033#discussion_r3800859874
##########
sql/core/src/test/resources/sql-tests/inputs/charvarchar-standard-semantics.sql:
##########
@@ -0,0 +1,90 @@
+--SET spark.sql.charVarchar.standardSemantics.enabled=true
+
+-- R3: CAST introduces CHAR/VARCHAR
+SELECT typeof(CAST('ab' AS CHAR(5)));
+SELECT typeof(CAST('hello' AS VARCHAR(5)));
+SELECT 'X' || CAST('5' AS CHAR(5)) || 'X';
+
+-- CAST length enforcement: trailing spaces are trimmed, real overflow errors
+SELECT CAST('ab ' AS CHAR(2));
+SELECT CAST('abcdef' AS CHAR(2));
+SELECT CAST('abcdef' AS VARCHAR(2));
+SELECT try_cast('abcdef' AS CHAR(2));
+SELECT try_cast('abcdef' AS VARCHAR(2));
+
+-- R2: least common type (COALESCE / CASE)
+SELECT typeof(coalesce(cast('hello' AS VARCHAR(5)), cast('world' AS VARCHAR(10))));
+SELECT typeof(coalesce(cast('hello' AS VARCHAR(5)), cast('world!' AS CHAR(6))));
+SELECT typeof(coalesce(cast('hello' AS CHAR(5)), cast('world!' AS CHAR(6))));
+SELECT typeof(coalesce(cast('hello' AS VARCHAR(5)), 'world'));
+SELECT typeof(coalesce(cast('hello' AS CHAR(5)), NULL));
+SELECT typeof(
+ CASE WHEN true THEN cast('a' AS CHAR(2)) ELSE cast('bb' AS CHAR(4)) END);
+
+-- R2: least common type for IN lists
+SELECT cast('a' AS CHAR(2)) IN (cast('a ' AS CHAR(2)), cast('bbb' AS VARCHAR(3)));
+SELECT typeof(c) FROM (SELECT cast('a' AS CHAR(2)) AS c) t WHERE c IN ('a ', 'b');
+
+-- R1: transforming functions return STRING
+SELECT typeof(upper(cast('ab' AS CHAR(2))));
+SELECT typeof(lower(cast('AB' AS VARCHAR(2))));
+SELECT typeof(cast('a' AS CHAR(1)) || cast('b' AS VARCHAR(1)));
+SELECT typeof(substr(cast('hello' AS VARCHAR(5)), 1, 2));
+SELECT typeof(upper(coalesce(cast('a' AS CHAR(2)), cast('b' AS CHAR(4)))));
+SELECT typeof(concat(cast('a' AS CHAR(2)), cast('b' AS CHAR(3))));
+SELECT typeof(trim(cast('ab ' AS CHAR(4))));
+SELECT typeof(lpad(cast('ab' AS CHAR(2)), 5, 'x'));
+
+-- R1: regexp / mask / split family
+SELECT typeof(regexp_replace(cast('ab' AS CHAR(2)), 'a', 'x'));
+SELECT typeof(regexp_extract(cast('ab' AS VARCHAR(2)), '(a)', 1));
+SELECT typeof(regexp_extract_all(cast('aab' AS VARCHAR(3)), '(a)', 1));
+SELECT typeof(split(cast('a,b' AS CHAR(3)), ','));
+SELECT typeof(mask(cast('ab' AS CHAR(2))));
+
+-- R1: CHAR/VARCHAR promote to STRING where a plain string is expected, so expressions that
+-- require all their string inputs to share one type accept them alongside a STRING argument.
+-- Values are wrapped in sentinels because the golden format trims trailing blanks, which would
+-- otherwise hide the CHAR padding these expressions operate on.
+SELECT typeof(overlay(cast('ab' AS CHAR(5)) PLACING 'x' FROM 1));
+SELECT concat('<', overlay(cast('ab' AS CHAR(5)) PLACING 'x' FROM 1), '>');
+SELECT typeof(elt(1, cast('ab' AS CHAR(5)), 'x'));
+SELECT typeof(right(cast('ab' AS CHAR(5)), 2));
+SELECT concat('<', right(cast('ab' AS CHAR(5)), 2), '>');
+SELECT typeof(left(cast('ab' AS CHAR(5)), 2));
+
+-- R1: transforms whose result length differs from the input must not inherit the constraint.
+SELECT typeof(reverse(cast('ab' AS CHAR(5))));
+SELECT typeof(hex(cast('ab' AS CHAR(5))));
+SELECT hex(cast('ab' AS CHAR(5)));
+SELECT typeof(array_join(array(cast('ab' AS CHAR(5)), cast('cd' AS CHAR(5))), '-'));
+SELECT concat('<', array_join(array(cast('ab' AS CHAR(5)), cast('cd' AS CHAR(5))), '-'), '>');
+-- reverse() on a non-string input is unaffected.
+SELECT typeof(reverse(array(1, 2)));
+
+-- R2 with collation.
+-- The recorded "string collate null" is a pre-existing gap, not a standardSemantics behavior:
Review Comment:
**High (Golden/Tests):** Please don’t golden `string collate null` for collated CHAR coalesce. The LCT helper unit test already expects `CharType(4, UTF8_LCASE)`, so this SQL/`typeof` result is a real E2E gap in the foundation — not just documentation of a known quirk.
Prefer removing/ignoring the query until fixed, or regenerating against the correct type. Otherwise we normalize a bug and a later fix looks like a golden regression.
##########
sql/api/src/main/scala/org/apache/spark/sql/types/StringType.scala:
##########
@@ -159,25 +159,76 @@ case object StringHelper extends PartialOrdering[StringConstraint] {
def isPlainString(s: StringType): Boolean = s.constraint == NoConstraint
+ /**
+ * Strip CHAR/VARCHAR length constraints, preserving collation.
+ *
+ * Used by transforming string expressions (upper, substr, concat, ...) so their result type is
+ * plain STRING even when inputs are CharType/VarcharType (SQL standard CHAR/VARCHAR R1), when
+ * standard semantics are on.
+ */
+ def plainStringType(dt: DataType): DataType = dt match {
+ case c: CharType => c.toStringType
+ case v: VarcharType => v.toStringType
+ case other => other
+ }
+
+ def plainStringType(s: StringType): StringType = s match {
+ case c: CharType => c.toStringType
+ case v: VarcharType => v.toStringType
+ case other => other
+ }
+
+ /**
+ * Result type for transforming string expressions. Under
+ * spark.sql.charVarchar.standardSemantics.enabled, always plain STRING (R1). Under
+ * preserveCharVarcharTypeInfo alone, keep child type (legacy leaky path).
+ */
+ def transformingStringResultType(dt: DataType): DataType = {
Review Comment:
**High (Architecture):** R1 is currently a dual protocol (`charVarcharToPlainString` + per-expr `transformingStringResultType`). That will keep regressing — anything using `ExpectsInputTypes` (no cast) or returning `child`/`first.dataType` without the helper still leaks `CharType`/`VarcharType` (see `StringToMap` in the review summary).
Can we centralize “transforming string result → plain STRING under standardSemantics” (trait / shared base) and keep an explicit R2/R3 allowlist for pass-throughs (`coalesce`, `max`, `element_at`, …)? At minimum, please add an inventory test that fails when a string-producing expr still returns constrained types under the flag.
##########
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/RowSetUtils.scala:
##########
@@ -147,8 +147,12 @@ object RowSetUtils {
// types that reach this branch do not use the `nested` flag in `toHiveString`. Now,
// Geospatial types use it for wrapping EWKT in quotes when nested = true, so we need
// to set `nested` here to false to avoid spurious quotes for standalone geo values.
+ // String types need the same treatment: the fast path above matches only the
+ // default-collation StringType singleton, so CHAR/VARCHAR and collated strings land
+ // here and would otherwise be rendered as "value" instead of value.
val nested = typ match {
case _: GeometryType | _: GeographyType => false
+ case _: StringType => false
Review Comment:
**Medium (Compatibility):** The `nested = false` for `_: StringType` also fixes collated strings on the Thrift fallback path, ungated by `standardSemantics`. That’s probably right, but it’s a broader behavior change than the CHAR/VARCHAR flag.
Please document it explicitly in the PR description and add a test (collated string + CHAR/VARCHAR) so we don’t regress quoting again. Consider splitting to a tiny dedicated bugfix if a cleaner changelog is preferred.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala:
##########
@@ -453,7 +454,8 @@ trait String2StringExpression extends ImplicitCastInputTypes {
def convert(v: UTF8String): UTF8String
- override def dataType: DataType = child.dataType
+ override def dataType: DataType =
+ StringHelper.transformingStringResultType(child.dataType)
Review Comment:
**Medium (Architecture):** Putting R1 on `String2StringExpression` also changes `Empty2Null`, which isn’t a transforming function (empty→null is pass-through for non-empty values). V1Writes currently only applies it when `dataType == StringType`, so we’re safe by accident.
Please override `Empty2Null.dataType` to keep the child type (or don’t share that trait).
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -1430,7 +1430,10 @@ case class Reverse(child: Expression)
BinaryType,
ArrayType))
- override def dataType: DataType = child.dataType
+ // Reversing a string transforms its content, so a CHAR/VARCHAR input yields plain STRING (R1).
+ // Array and binary inputs are unaffected. The promotion in ImplicitTypeCasts does not reach
+ // here because the expected type is a TypeCollection rather than a plain string type.
Review Comment:
**Low (Style):** Nit: `expectsStringType` already treats `TypeCollection`, so ImplicitTypeCasts *does* promote CHAR for `Reverse`’s string branch. Mind updating the comment so we don’t teach the wrong invariant? (`transformingStringResultType` remains useful as belt-and-suspenders / for non-cast paths.)
--
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
Re: [PR] [SPARK-58794][SQL] Add standardSemantics foundation for CHAR/VARCHAR [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58033:
URL: https://github.com/apache/spark/pull/58033#discussion_r3801131958
##########
sql/api/src/main/scala/org/apache/spark/sql/types/StringType.scala:
##########
@@ -159,25 +159,76 @@ case object StringHelper extends PartialOrdering[StringConstraint] {
def isPlainString(s: StringType): Boolean = s.constraint == NoConstraint
+ /**
+ * Strip CHAR/VARCHAR length constraints, preserving collation.
+ *
+ * Used by transforming string expressions (upper, substr, concat, ...) so their result type is
+ * plain STRING even when inputs are CharType/VarcharType (SQL standard CHAR/VARCHAR R1), when
+ * standard semantics are on.
+ */
+ def plainStringType(dt: DataType): DataType = dt match {
+ case c: CharType => c.toStringType
+ case v: VarcharType => v.toStringType
+ case other => other
+ }
+
+ def plainStringType(s: StringType): StringType = s match {
+ case c: CharType => c.toStringType
+ case v: VarcharType => v.toStringType
+ case other => other
+ }
+
+ /**
+ * Result type for transforming string expressions. Under
+ * spark.sql.charVarchar.standardSemantics.enabled, always plain STRING (R1). Under
+ * preserveCharVarcharTypeInfo alone, keep child type (legacy leaky path).
+ */
+ def transformingStringResultType(dt: DataType): DataType = {
Review Comment:
Agreed on the diagnosis, and the sweep found more than `StringToMap`: `json_tuple` leaked the same way (both take their input through `ExpectsInputTypes`, so no implicit cast strips the length). Both now reduce the extracted values to plain STRING.
On centralizing: `charVarcharToPlainString` in `ImplicitTypeCasts` already is the central mechanism for everything that goes through implicit casting, which is the large majority. What it structurally cannot reach is `ExpectsInputTypes` (no cast is inserted) and expressions that derive a result type from a child without a cast. A trait would not close that either, since the leak is in the `dataType` override rather than in the input contract, so I would rather not add a second mechanism on top of the two we have.
What I did add is the enforcement you asked for: `SPARK-58794: no unlisted function returns a CHAR/VARCHAR type under standardSemantics` sweeps the whole `FunctionRegistry` with several argument shapes and fails on any function that returns a constrained type unless it is in an explicit pass-through allowlist (the R2/R3 set: aggregates returning an input unchanged, the null-handling family, element access, and the array/map/struct constructors). A newly added expression that leaks now fails the test instead of shipping; a genuine pass-through requires a deliberate allowlist entry.
--
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
Re: [PR] [SPARK-58794][SQL] Add standardSemantics foundation for CHAR/VARCHAR [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58033:
URL: https://github.com/apache/spark/pull/58033#discussion_r3801133080
##########
sql/core/src/test/resources/sql-tests/inputs/charvarchar-standard-semantics.sql:
##########
@@ -0,0 +1,90 @@
+--SET spark.sql.charVarchar.standardSemantics.enabled=true
+
+-- R3: CAST introduces CHAR/VARCHAR
+SELECT typeof(CAST('ab' AS CHAR(5)));
+SELECT typeof(CAST('hello' AS VARCHAR(5)));
+SELECT 'X' || CAST('5' AS CHAR(5)) || 'X';
+
+-- CAST length enforcement: trailing spaces are trimmed, real overflow errors
+SELECT CAST('ab ' AS CHAR(2));
+SELECT CAST('abcdef' AS CHAR(2));
+SELECT CAST('abcdef' AS VARCHAR(2));
+SELECT try_cast('abcdef' AS CHAR(2));
+SELECT try_cast('abcdef' AS VARCHAR(2));
+
+-- R2: least common type (COALESCE / CASE)
+SELECT typeof(coalesce(cast('hello' AS VARCHAR(5)), cast('world' AS VARCHAR(10))));
+SELECT typeof(coalesce(cast('hello' AS VARCHAR(5)), cast('world!' AS CHAR(6))));
+SELECT typeof(coalesce(cast('hello' AS CHAR(5)), cast('world!' AS CHAR(6))));
+SELECT typeof(coalesce(cast('hello' AS VARCHAR(5)), 'world'));
+SELECT typeof(coalesce(cast('hello' AS CHAR(5)), NULL));
+SELECT typeof(
+ CASE WHEN true THEN cast('a' AS CHAR(2)) ELSE cast('bb' AS CHAR(4)) END);
+
+-- R2: least common type for IN lists
+SELECT cast('a' AS CHAR(2)) IN (cast('a ' AS CHAR(2)), cast('bbb' AS VARCHAR(3)));
+SELECT typeof(c) FROM (SELECT cast('a' AS CHAR(2)) AS c) t WHERE c IN ('a ', 'b');
+
+-- R1: transforming functions return STRING
+SELECT typeof(upper(cast('ab' AS CHAR(2))));
+SELECT typeof(lower(cast('AB' AS VARCHAR(2))));
+SELECT typeof(cast('a' AS CHAR(1)) || cast('b' AS VARCHAR(1)));
+SELECT typeof(substr(cast('hello' AS VARCHAR(5)), 1, 2));
+SELECT typeof(upper(coalesce(cast('a' AS CHAR(2)), cast('b' AS CHAR(4)))));
+SELECT typeof(concat(cast('a' AS CHAR(2)), cast('b' AS CHAR(3))));
+SELECT typeof(trim(cast('ab ' AS CHAR(4))));
+SELECT typeof(lpad(cast('ab' AS CHAR(2)), 5, 'x'));
+
+-- R1: regexp / mask / split family
+SELECT typeof(regexp_replace(cast('ab' AS CHAR(2)), 'a', 'x'));
+SELECT typeof(regexp_extract(cast('ab' AS VARCHAR(2)), '(a)', 1));
+SELECT typeof(regexp_extract_all(cast('aab' AS VARCHAR(3)), '(a)', 1));
+SELECT typeof(split(cast('a,b' AS CHAR(3)), ','));
+SELECT typeof(mask(cast('ab' AS CHAR(2))));
+
+-- R1: CHAR/VARCHAR promote to STRING where a plain string is expected, so expressions that
+-- require all their string inputs to share one type accept them alongside a STRING argument.
+-- Values are wrapped in sentinels because the golden format trims trailing blanks, which would
+-- otherwise hide the CHAR padding these expressions operate on.
+SELECT typeof(overlay(cast('ab' AS CHAR(5)) PLACING 'x' FROM 1));
+SELECT concat('<', overlay(cast('ab' AS CHAR(5)) PLACING 'x' FROM 1), '>');
+SELECT typeof(elt(1, cast('ab' AS CHAR(5)), 'x'));
+SELECT typeof(right(cast('ab' AS CHAR(5)), 2));
+SELECT concat('<', right(cast('ab' AS CHAR(5)), 2), '>');
+SELECT typeof(left(cast('ab' AS CHAR(5)), 2));
+
+-- R1: transforms whose result length differs from the input must not inherit the constraint.
+SELECT typeof(reverse(cast('ab' AS CHAR(5))));
+SELECT typeof(hex(cast('ab' AS CHAR(5))));
+SELECT hex(cast('ab' AS CHAR(5)));
+SELECT typeof(array_join(array(cast('ab' AS CHAR(5)), cast('cd' AS CHAR(5))), '-'));
+SELECT concat('<', array_join(array(cast('ab' AS CHAR(5)), cast('cd' AS CHAR(5))), '-'), '>');
+-- reverse() on a non-string input is unaffected.
+SELECT typeof(reverse(array(1, 2)));
+
+-- R2 with collation.
+-- The recorded "string collate null" is a pre-existing gap, not a standardSemantics behavior:
Review Comment:
You are right, and my note on it was wrong in a way worth recording: I had written it off as a constant-folding artifact, but it reproduces with plain column references too, and it reproduces identically under `spark.sql.preserveCharVarcharTypeInfo`, so it is neither folding-specific nor introduced here.
The cause is `CollationTypeCoercion.getWinningStringType`: at equal collation strength it compares the operands with `sameType`, so `CHAR(2) COLLATE UTF8_LCASE` and `CHAR(4) COLLATE UTF8_LCASE` are read as a collation mismatch purely because the lengths differ, and it yields `IndeterminateStringType`. It is not a one-line fix, because that rule then casts all string children to the single winner, so simply making the lengths compare equal would truncate to the shorter one; the length has to stay with the LCT. That belongs in its own change.
So I dropped the query rather than golden the wrong type, and replaced it with the collated cases that do hold (a collated CAST, and an LCT over equally constrained operands, which yields `char(2) collate UTF8_LCASE`). The comment in the input file now states the gap and why it is not covered here.
--
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
Re: [PR] [SPARK-58794][SQL] Add standardSemantics foundation for CHAR/VARCHAR [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on code in PR #58033:
URL: https://github.com/apache/spark/pull/58033#discussion_r3805975376
##########
sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala:
##########
@@ -854,6 +858,452 @@ class BasicCharVarcharTestSuite extends SharedSparkSession {
}
}
+ test("SPARK-58797: CAST to CHAR/VARCHAR with standardSemantics") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val charDf = sql("SELECT CAST('ab' AS CHAR(5)) AS c")
+ assert(charDf.schema.head.dataType === CharType(5))
+ checkAnswer(charDf, Row("ab "))
+
+ val varcharDf = sql("SELECT CAST('hello' AS VARCHAR(5)) AS v")
+ assert(varcharDf.schema.head.dataType === VarcharType(5))
+ checkAnswer(varcharDf, Row("hello"))
+
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ sql("SELECT CAST('hello!' AS VARCHAR(5))").collect()
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "5")
+ )
+
+ // Multi-byte characters: length is in characters, not octets.
+ // scalastyle:off nonascii
+ checkAnswer(sql("SELECT CAST('你好' AS VARCHAR(2)) AS v"), Row("你好"))
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ sql("SELECT CAST('你好啊' AS VARCHAR(2))").collect()
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "2")
+ )
+ // scalastyle:on nonascii
+ }
+ }
+
+ test("SPARK-58798: least common type for COALESCE/CASE with CHAR/VARCHAR") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ assert(sql(
+ "SELECT coalesce(cast('hello' AS VARCHAR(5)), cast('world' AS VARCHAR(10))) AS c")
+ .schema.head.dataType === VarcharType(10))
+ assert(sql(
+ "SELECT coalesce(cast('hello' AS VARCHAR(5)), cast('world!' AS CHAR(6))) AS c")
+ .schema.head.dataType === VarcharType(6))
+ assert(sql(
+ "SELECT coalesce(cast('hello' AS CHAR(5)), cast('world!' AS CHAR(6))) AS c")
+ .schema.head.dataType === CharType(6))
+ assert(sql(
+ "SELECT coalesce(cast('hello' AS VARCHAR(5)), 'world') AS c")
+ .schema.head.dataType === StringType)
+ assert(sql(
+ """SELECT CASE WHEN true THEN cast('a' AS CHAR(2))
+ |ELSE cast('bb' AS CHAR(4)) END AS c""".stripMargin)
+ .schema.head.dataType === CharType(4))
+ // LCT(NULL, T) = T
+ assert(sql("SELECT coalesce(null, cast('a' AS CHAR(5))) AS c")
+ .schema.head.dataType === CharType(5))
+ }
+ }
+
+ test("SPARK-58799: transforming string functions return STRING") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ assert(sql("SELECT upper(cast('ab' AS CHAR(2))) AS c")
+ .schema.head.dataType === StringType)
+ assert(sql("SELECT lower(cast('AB' AS VARCHAR(2))) AS c")
+ .schema.head.dataType === StringType)
+ assert(sql(
+ "SELECT cast('a' AS CHAR(1)) || cast('b' AS VARCHAR(1)) AS c")
+ .schema.head.dataType === StringType)
+ // Pads from CHAR participate in the concatenated value.
+ checkAnswer(
+ sql("SELECT cast('he' AS CHAR(4)) || cast('llo' AS CHAR(3)) AS c"),
+ Row("he llo"))
+ assert(sql("SELECT substr(cast('hello' AS VARCHAR(5)), 1, 2) AS c")
+ .schema.head.dataType === StringType)
+ assert(sql(
+ "SELECT upper(coalesce(cast('a' AS CHAR(2)), cast('b' AS CHAR(4)))) AS c")
+ .schema.head.dataType === StringType)
+ }
+ }
+
+ test("SPARK-58798: LCT preserves collation on CHAR/VARCHAR") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val c1 = CharType(2, "UTF8_LCASE")
+ val c2 = CharType(4, "UTF8_LCASE")
+ assert(StringHelper.tightestCommonString(c1, c2).contains(CharType(4, "UTF8_LCASE")))
+ val v1 = VarcharType(3, "UTF8_LCASE")
+ val v2 = VarcharType(5, "UTF8_LCASE")
+ assert(StringHelper.tightestCommonString(v1, v2).contains(VarcharType(5, "UTF8_LCASE")))
+ assert(StringHelper.tightestCommonString(c1, v2).contains(VarcharType(5, "UTF8_LCASE")))
+ }
+ }
+
+ test("SPARK-58799: regexp/mask/split return STRING under standardSemantics") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ assert(sql("SELECT regexp_replace(cast('ab' AS CHAR(2)), 'a', 'x') AS c")
+ .schema.head.dataType === StringType)
+ assert(sql("SELECT regexp_extract(cast('ab' AS VARCHAR(2)), '(a)', 1) AS c")
+ .schema.head.dataType === StringType)
+ assert(sql("SELECT split(cast('a,b' AS CHAR(3)), ',') AS c")
+ .schema.head.dataType === ArrayType(StringType, containsNull = false))
+ assert(sql("SELECT mask(cast('ab' AS CHAR(2))) AS c")
+ .schema.head.dataType === StringType)
+ }
+ }
+
+ test("SPARK-58796: preserve vs standardSemantics R1 matrix") {
+ // preserve-only: transforming ops may keep Char/Varchar (leaky experimental path).
+ withSQLConf(SQLConf.PRESERVE_CHAR_VARCHAR_TYPE_INFO.key -> "true") {
+ assert(sql("SELECT upper(cast('ab' AS CHAR(2))) AS c")
+ .schema.head.dataType === CharType(2))
+ }
+ // standardSemantics: R1 forces STRING even if preserve is also on.
+ withSQLConf(
+ SQLConf.PRESERVE_CHAR_VARCHAR_TYPE_INFO.key -> "true",
+ SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ assert(sql("SELECT upper(cast('ab' AS CHAR(2))) AS c")
+ .schema.head.dataType === StringType)
+ }
+ }
+
+ test("SPARK-58797: standardSemantics wins over charVarcharAsString") {
+ withSQLConf(
+ SQLConf.LEGACY_CHAR_VARCHAR_AS_STRING.key -> "true",
+ SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val df = sql("SELECT CAST('ab' AS CHAR(5)) AS c")
+ assert(df.schema.head.dataType === CharType(5))
+ checkAnswer(df, Row("ab "))
+ }
+ }
+
+ test("SPARK-58796: createDataFrame allows CHAR/VARCHAR when standardSemantics") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val df = spark.range(1).map(_.toString).toDF()
+ val schema = new StructType().add("id", CharType(5))
+ val created = spark.createDataFrame(df.collectAsList(), schema)
+ assert(created.schema.head.dataType === CharType(5))
+ checkAnswer(created, Row("0 "))
+
+ // RowEncoder must retain a declared collation on the constrained type, not rebuild
+ // CharType(length) / VarcharType(length) with the default collation.
+ val collated = new StructType()
+ .add("c", CharType(5, "UTF8_LCASE"))
+ .add("v", VarcharType(5, "UTF8_LCASE"))
+ val collatedDf = spark.createDataFrame(
+ java.util.Arrays.asList(Row("ab", "cd")), collated)
+ assert(collatedDf.schema("c").dataType === CharType(5, "UTF8_LCASE"))
+ assert(collatedDf.schema("v").dataType === VarcharType(5, "UTF8_LCASE"))
+ checkAnswer(collatedDf, Row("ab ", "cd"))
+ }
+ }
+
+ test("SPARK-58803: Dataset/encoder/UDF CHAR/VARCHAR under standardSemantics") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ // createDataFrame / RowEncoder write-side: pad CHAR, reject oversize.
+ val charSchema = new StructType().add("c", CharType(3))
+ checkAnswer(
+ spark.createDataFrame(java.util.Arrays.asList(Row("ab")), charSchema),
+ Row("ab "))
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ spark.createDataFrame(java.util.Arrays.asList(Row("abcd")), charSchema).collect()
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+ val varcharSchema = new StructType().add("v", VarcharType(3))
+ checkAnswer(
+ spark.createDataFrame(java.util.Arrays.asList(Row("ab")), varcharSchema),
+ Row("ab"))
+ // Oversize by trailing blanks only: trim just enough to fit the limit.
+ checkAnswer(
+ spark.createDataFrame(java.util.Arrays.asList(Row("abc ")), varcharSchema),
+ Row("abc"))
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ spark.createDataFrame(java.util.Arrays.asList(Row("abcd")), varcharSchema).collect()
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+
+ // Explicit Encoders.CHAR / VARCHAR: typed Dataset write-side checks.
+ val charDs = spark.createDataset(Seq("ab"))(Encoders.CHAR(4))
+ assert(charDs.schema.head.dataType === CharType(4))
+ checkAnswer(charDs.toDF(), Row("ab "))
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ spark.createDataset(Seq("abcde"))(Encoders.VARCHAR(3)).collect()
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+
+ // UDF register: return type stays CHAR/VARCHAR; write-side pad / length apply.
+ spark.udf.register("std_char_udf", () => "B", CharType(3))
+ spark.udf.register("std_varchar_udf", (x: String) => x, VarcharType(3))
+ val charUdf = sql("SELECT std_char_udf() AS c")
+ assert(charUdf.schema.head.dataType === CharType(3))
+ checkAnswer(charUdf, Row("B "))
+ val varcharUdf = sql("SELECT std_varchar_udf('ab') AS v")
+ assert(varcharUdf.schema.head.dataType === VarcharType(3))
+ checkAnswer(varcharUdf, Row("ab"))
+ checkError(
+ exception = intercept[SparkException] {
+ sql("SELECT std_varchar_udf('abcd')").collect()
+ }.getCause.asInstanceOf[SparkRuntimeException],
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+
+ // Java udf(..., returnType) path and Dataset.encoder from CHAR result schema.
+ val javaUdf = functions.udf(
+ new org.apache.spark.sql.api.java.UDF0[String] {
+ override def call(): String = "a"
+ },
+ CharType(5))
+ val javaUdfDf = spark.range(1).select(javaUdf().as("c"))
+ assert(javaUdfDf.schema.head.dataType === CharType(5))
+ checkAnswer(javaUdfDf, Row("a "))
+ assert(javaUdfDf.encoder.schema.head.dataType === CharType(5))
+
+ // Dataset.to: CHAR/VARCHAR target schema allowed; Cast applies store assignment.
+ withTable("std_cv_to") {
+ sql("CREATE TABLE std_cv_to (c CHAR(10), v VARCHAR(255)) USING parquet")
+ sql("INSERT INTO std_cv_to VALUES ('spark', 'awesome')")
+ val df = sql("SELECT * FROM std_cv_to")
+ assert(df.schema("c").dataType === CharType(10))
+ assert(df.schema("v").dataType === VarcharType(255))
+ val reordered = StructType.fromDDL("v VARCHAR(255), c CHAR(10)")
+ val toDf = df.to(reordered)
+ assert(toDf.schema.map(_.dataType) === Seq(VarcharType(255), CharType(10)))
+ checkAnswer(toDf, Row("awesome", "spark "))
+ // Narrowing CHAR length is store assignment and must enforce length.
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ df.select($"c").to(new StructType().add("c", CharType(3))).collect()
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+ }
+
+ // DataFrameReader / DataStreamReader user schemas keep CHAR/VARCHAR.
+ val readerSchema = new StructType().add("id", CharType(5))
+ val csvInput = spark.range(1).map(_.toString)
+ val csvDf = spark.read.schema(readerSchema).csv(csvInput)
+ assert(csvDf.schema.head.dataType === CharType(5))
+ checkAnswer(csvDf, Row("0 "))
+ val csvDfDdl = spark.read.schema("id VARCHAR(5)").csv(csvInput)
+ assert(csvDfDdl.schema.head.dataType === VarcharType(5))
+ withTempPath { dir =>
+ spark.range(1).write.save(dir.toString)
+ val streamDf = spark.readStream.schema(readerSchema).load(dir.toString)
+ assert(streamDf.schema.head.dataType === CharType(5))
+ val streamDdl = spark.readStream.schema("id VARCHAR(5)").load(dir.toString)
+ assert(streamDdl.schema.head.dataType === VarcharType(5))
+ }
+ }
+ }
+
+ test("SPARK-58794: R1 promotion unifies CHAR/VARCHAR with STRING at plain-string inputs") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ withTable("std_promote") {
+ sql("CREATE TABLE std_promote (c CHAR(5), v VARCHAR(5)) USING parquet")
+ sql("INSERT INTO std_promote VALUES ('ab', 'ab')")
+
+ // Expressions requiring all their string inputs to share one type must accept a
+ // CHAR/VARCHAR argument alongside a STRING one by promoting it to STRING.
+ Seq(
+ "overlay(c PLACING 'x' FROM 1)" -> "xb ",
+ "overlay(v PLACING 'x' FROM 1)" -> "xb",
+ "string_agg(c, '-')" -> "ab ",
+ "listagg(c, '-')" -> "ab ",
+ "elt(1, c, 'x')" -> "ab ",
+ // right() is RuntimeReplaceable; its literal branches must agree with the substring
+ // branch, which R1 has already reduced to STRING.
+ "right(c, 2)" -> " ",
+ "left(c, 2)" -> "ab").foreach { case (expr, expected) =>
+ val df = sql(s"SELECT $expr AS r FROM std_promote")
+ assert(df.schema.head.dataType === StringType, s"$expr should return STRING")
+ checkAnswer(df, Row(expected))
+ }
+
+ // Transforming expressions must not inherit the input's length constraint: each of these
+ // produces a value whose length differs from the CHAR(5) input.
+ Seq(
+ "reverse(c)" -> " ba",
+ "hex(c)" -> "6162202020",
+ "array_join(array(c, c), '-')" -> "ab -ab ").foreach { case (expr, expected) =>
+ val df = sql(s"SELECT $expr AS r FROM std_promote")
+ assert(df.schema.head.dataType === StringType, s"$expr should return STRING")
+ checkAnswer(df, Row(expected))
+ }
+
+ // Promotion must not reach pass-through / LCT sites, which preserve CHAR/VARCHAR (R2/R3).
+ Seq(
+ "c", "coalesce(c, c)", "case when true then c else c end", "max(c)",
+ "element_at(array(c), 1)", "transform(array(c), x -> x)[0]",
+ "first_value(c) over (order by 1)").foreach { expr =>
+ val df = sql(s"SELECT $expr AS r FROM std_promote")
+ assert(df.schema.head.dataType === CharType(5), s"$expr should stay CHAR(5)")
+ }
+
+ // reverse() on non-string inputs is unaffected by the R1 change.
+ assert(sql("SELECT reverse(array(1, 2)) AS r").schema.head.dataType ===
+ ArrayType(IntegerType, containsNull = false))
+ }
+ }
+ }
+
+ // Every registered function that may return a CHAR(n)/VARCHAR(n) when handed one. These are the
+ // pass-through and container cases R2/R3 require to keep the type: aggregates and ordering
+ // functions that return one of their inputs unchanged, the null-handling family, element access,
+ // and the array/map/struct constructors along with the collection functions that rearrange
+ // elements without rewriting them. Anything not listed here must reduce to plain STRING (R1), so
+ // a newly added expression that leaks a length constraint fails this test rather than shipping.
Review Comment:
Done in 9f4d0066cc8: the allowlist comment and the test name now state that coverage is limited to the seven fixed `argumentShapes` templates, and that a leak only at another arity or nested shape would not fail this inventory.
--
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
Re: [PR] [SPARK-58794][SQL] Add standardSemantics foundation for CHAR/VARCHAR [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang closed pull request #58033: [SPARK-58794][SQL] Add standardSemantics foundation for CHAR/VARCHAR
URL: https://github.com/apache/spark/pull/58033
--
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
Re: [PR] [SPARK-58794][SQL] Add standardSemantics foundation for CHAR/VARCHAR [spark]
Posted by "srielau (via GitHub)" <gi...@apache.org>.
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: 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
Re: [PR] [SPARK-58794][SQL] Add standardSemantics foundation for CHAR/VARCHAR [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #58033:
URL: https://github.com/apache/spark/pull/58033#discussion_r3805711220
##########
sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala:
##########
@@ -854,6 +858,452 @@ class BasicCharVarcharTestSuite extends SharedSparkSession {
}
}
+ test("SPARK-58797: CAST to CHAR/VARCHAR with standardSemantics") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val charDf = sql("SELECT CAST('ab' AS CHAR(5)) AS c")
+ assert(charDf.schema.head.dataType === CharType(5))
+ checkAnswer(charDf, Row("ab "))
+
+ val varcharDf = sql("SELECT CAST('hello' AS VARCHAR(5)) AS v")
+ assert(varcharDf.schema.head.dataType === VarcharType(5))
+ checkAnswer(varcharDf, Row("hello"))
+
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ sql("SELECT CAST('hello!' AS VARCHAR(5))").collect()
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "5")
+ )
+
+ // Multi-byte characters: length is in characters, not octets.
+ // scalastyle:off nonascii
+ checkAnswer(sql("SELECT CAST('你好' AS VARCHAR(2)) AS v"), Row("你好"))
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ sql("SELECT CAST('你好啊' AS VARCHAR(2))").collect()
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "2")
+ )
+ // scalastyle:on nonascii
+ }
+ }
+
+ test("SPARK-58798: least common type for COALESCE/CASE with CHAR/VARCHAR") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ assert(sql(
+ "SELECT coalesce(cast('hello' AS VARCHAR(5)), cast('world' AS VARCHAR(10))) AS c")
+ .schema.head.dataType === VarcharType(10))
+ assert(sql(
+ "SELECT coalesce(cast('hello' AS VARCHAR(5)), cast('world!' AS CHAR(6))) AS c")
+ .schema.head.dataType === VarcharType(6))
+ assert(sql(
+ "SELECT coalesce(cast('hello' AS CHAR(5)), cast('world!' AS CHAR(6))) AS c")
+ .schema.head.dataType === CharType(6))
+ assert(sql(
+ "SELECT coalesce(cast('hello' AS VARCHAR(5)), 'world') AS c")
+ .schema.head.dataType === StringType)
+ assert(sql(
+ """SELECT CASE WHEN true THEN cast('a' AS CHAR(2))
+ |ELSE cast('bb' AS CHAR(4)) END AS c""".stripMargin)
+ .schema.head.dataType === CharType(4))
+ // LCT(NULL, T) = T
+ assert(sql("SELECT coalesce(null, cast('a' AS CHAR(5))) AS c")
+ .schema.head.dataType === CharType(5))
+ }
+ }
+
+ test("SPARK-58799: transforming string functions return STRING") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ assert(sql("SELECT upper(cast('ab' AS CHAR(2))) AS c")
+ .schema.head.dataType === StringType)
+ assert(sql("SELECT lower(cast('AB' AS VARCHAR(2))) AS c")
+ .schema.head.dataType === StringType)
+ assert(sql(
+ "SELECT cast('a' AS CHAR(1)) || cast('b' AS VARCHAR(1)) AS c")
+ .schema.head.dataType === StringType)
+ // Pads from CHAR participate in the concatenated value.
+ checkAnswer(
+ sql("SELECT cast('he' AS CHAR(4)) || cast('llo' AS CHAR(3)) AS c"),
+ Row("he llo"))
+ assert(sql("SELECT substr(cast('hello' AS VARCHAR(5)), 1, 2) AS c")
+ .schema.head.dataType === StringType)
+ assert(sql(
+ "SELECT upper(coalesce(cast('a' AS CHAR(2)), cast('b' AS CHAR(4)))) AS c")
+ .schema.head.dataType === StringType)
+ }
+ }
+
+ test("SPARK-58798: LCT preserves collation on CHAR/VARCHAR") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val c1 = CharType(2, "UTF8_LCASE")
+ val c2 = CharType(4, "UTF8_LCASE")
+ assert(StringHelper.tightestCommonString(c1, c2).contains(CharType(4, "UTF8_LCASE")))
+ val v1 = VarcharType(3, "UTF8_LCASE")
+ val v2 = VarcharType(5, "UTF8_LCASE")
+ assert(StringHelper.tightestCommonString(v1, v2).contains(VarcharType(5, "UTF8_LCASE")))
+ assert(StringHelper.tightestCommonString(c1, v2).contains(VarcharType(5, "UTF8_LCASE")))
+ }
+ }
+
+ test("SPARK-58799: regexp/mask/split return STRING under standardSemantics") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ assert(sql("SELECT regexp_replace(cast('ab' AS CHAR(2)), 'a', 'x') AS c")
+ .schema.head.dataType === StringType)
+ assert(sql("SELECT regexp_extract(cast('ab' AS VARCHAR(2)), '(a)', 1) AS c")
+ .schema.head.dataType === StringType)
+ assert(sql("SELECT split(cast('a,b' AS CHAR(3)), ',') AS c")
+ .schema.head.dataType === ArrayType(StringType, containsNull = false))
+ assert(sql("SELECT mask(cast('ab' AS CHAR(2))) AS c")
+ .schema.head.dataType === StringType)
+ }
+ }
+
+ test("SPARK-58796: preserve vs standardSemantics R1 matrix") {
+ // preserve-only: transforming ops may keep Char/Varchar (leaky experimental path).
+ withSQLConf(SQLConf.PRESERVE_CHAR_VARCHAR_TYPE_INFO.key -> "true") {
+ assert(sql("SELECT upper(cast('ab' AS CHAR(2))) AS c")
+ .schema.head.dataType === CharType(2))
+ }
+ // standardSemantics: R1 forces STRING even if preserve is also on.
+ withSQLConf(
+ SQLConf.PRESERVE_CHAR_VARCHAR_TYPE_INFO.key -> "true",
+ SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ assert(sql("SELECT upper(cast('ab' AS CHAR(2))) AS c")
+ .schema.head.dataType === StringType)
+ }
+ }
+
+ test("SPARK-58797: standardSemantics wins over charVarcharAsString") {
+ withSQLConf(
+ SQLConf.LEGACY_CHAR_VARCHAR_AS_STRING.key -> "true",
+ SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val df = sql("SELECT CAST('ab' AS CHAR(5)) AS c")
+ assert(df.schema.head.dataType === CharType(5))
+ checkAnswer(df, Row("ab "))
+ }
+ }
+
+ test("SPARK-58796: createDataFrame allows CHAR/VARCHAR when standardSemantics") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val df = spark.range(1).map(_.toString).toDF()
+ val schema = new StructType().add("id", CharType(5))
+ val created = spark.createDataFrame(df.collectAsList(), schema)
+ assert(created.schema.head.dataType === CharType(5))
+ checkAnswer(created, Row("0 "))
+
+ // RowEncoder must retain a declared collation on the constrained type, not rebuild
+ // CharType(length) / VarcharType(length) with the default collation.
+ val collated = new StructType()
+ .add("c", CharType(5, "UTF8_LCASE"))
+ .add("v", VarcharType(5, "UTF8_LCASE"))
+ val collatedDf = spark.createDataFrame(
+ java.util.Arrays.asList(Row("ab", "cd")), collated)
+ assert(collatedDf.schema("c").dataType === CharType(5, "UTF8_LCASE"))
+ assert(collatedDf.schema("v").dataType === VarcharType(5, "UTF8_LCASE"))
+ checkAnswer(collatedDf, Row("ab ", "cd"))
+ }
+ }
+
+ test("SPARK-58803: Dataset/encoder/UDF CHAR/VARCHAR under standardSemantics") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ // createDataFrame / RowEncoder write-side: pad CHAR, reject oversize.
+ val charSchema = new StructType().add("c", CharType(3))
+ checkAnswer(
+ spark.createDataFrame(java.util.Arrays.asList(Row("ab")), charSchema),
+ Row("ab "))
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ spark.createDataFrame(java.util.Arrays.asList(Row("abcd")), charSchema).collect()
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+ val varcharSchema = new StructType().add("v", VarcharType(3))
+ checkAnswer(
+ spark.createDataFrame(java.util.Arrays.asList(Row("ab")), varcharSchema),
+ Row("ab"))
+ // Oversize by trailing blanks only: trim just enough to fit the limit.
+ checkAnswer(
+ spark.createDataFrame(java.util.Arrays.asList(Row("abc ")), varcharSchema),
+ Row("abc"))
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ spark.createDataFrame(java.util.Arrays.asList(Row("abcd")), varcharSchema).collect()
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+
+ // Explicit Encoders.CHAR / VARCHAR: typed Dataset write-side checks.
+ val charDs = spark.createDataset(Seq("ab"))(Encoders.CHAR(4))
+ assert(charDs.schema.head.dataType === CharType(4))
+ checkAnswer(charDs.toDF(), Row("ab "))
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ spark.createDataset(Seq("abcde"))(Encoders.VARCHAR(3)).collect()
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+
+ // UDF register: return type stays CHAR/VARCHAR; write-side pad / length apply.
+ spark.udf.register("std_char_udf", () => "B", CharType(3))
+ spark.udf.register("std_varchar_udf", (x: String) => x, VarcharType(3))
+ val charUdf = sql("SELECT std_char_udf() AS c")
+ assert(charUdf.schema.head.dataType === CharType(3))
+ checkAnswer(charUdf, Row("B "))
+ val varcharUdf = sql("SELECT std_varchar_udf('ab') AS v")
+ assert(varcharUdf.schema.head.dataType === VarcharType(3))
+ checkAnswer(varcharUdf, Row("ab"))
+ checkError(
+ exception = intercept[SparkException] {
+ sql("SELECT std_varchar_udf('abcd')").collect()
+ }.getCause.asInstanceOf[SparkRuntimeException],
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+
+ // Java udf(..., returnType) path and Dataset.encoder from CHAR result schema.
+ val javaUdf = functions.udf(
+ new org.apache.spark.sql.api.java.UDF0[String] {
+ override def call(): String = "a"
+ },
+ CharType(5))
+ val javaUdfDf = spark.range(1).select(javaUdf().as("c"))
+ assert(javaUdfDf.schema.head.dataType === CharType(5))
+ checkAnswer(javaUdfDf, Row("a "))
+ assert(javaUdfDf.encoder.schema.head.dataType === CharType(5))
+
+ // Dataset.to: CHAR/VARCHAR target schema allowed; Cast applies store assignment.
+ withTable("std_cv_to") {
+ sql("CREATE TABLE std_cv_to (c CHAR(10), v VARCHAR(255)) USING parquet")
+ sql("INSERT INTO std_cv_to VALUES ('spark', 'awesome')")
+ val df = sql("SELECT * FROM std_cv_to")
+ assert(df.schema("c").dataType === CharType(10))
+ assert(df.schema("v").dataType === VarcharType(255))
+ val reordered = StructType.fromDDL("v VARCHAR(255), c CHAR(10)")
+ val toDf = df.to(reordered)
+ assert(toDf.schema.map(_.dataType) === Seq(VarcharType(255), CharType(10)))
+ checkAnswer(toDf, Row("awesome", "spark "))
+ // Narrowing CHAR length is store assignment and must enforce length.
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ df.select($"c").to(new StructType().add("c", CharType(3))).collect()
+ },
+ condition = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "3"))
+ }
+
+ // DataFrameReader / DataStreamReader user schemas keep CHAR/VARCHAR.
+ val readerSchema = new StructType().add("id", CharType(5))
+ val csvInput = spark.range(1).map(_.toString)
+ val csvDf = spark.read.schema(readerSchema).csv(csvInput)
+ assert(csvDf.schema.head.dataType === CharType(5))
+ checkAnswer(csvDf, Row("0 "))
+ val csvDfDdl = spark.read.schema("id VARCHAR(5)").csv(csvInput)
+ assert(csvDfDdl.schema.head.dataType === VarcharType(5))
+ withTempPath { dir =>
+ spark.range(1).write.save(dir.toString)
+ val streamDf = spark.readStream.schema(readerSchema).load(dir.toString)
+ assert(streamDf.schema.head.dataType === CharType(5))
+ val streamDdl = spark.readStream.schema("id VARCHAR(5)").load(dir.toString)
+ assert(streamDdl.schema.head.dataType === VarcharType(5))
+ }
+ }
+ }
+
+ test("SPARK-58794: R1 promotion unifies CHAR/VARCHAR with STRING at plain-string inputs") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ withTable("std_promote") {
+ sql("CREATE TABLE std_promote (c CHAR(5), v VARCHAR(5)) USING parquet")
+ sql("INSERT INTO std_promote VALUES ('ab', 'ab')")
+
+ // Expressions requiring all their string inputs to share one type must accept a
+ // CHAR/VARCHAR argument alongside a STRING one by promoting it to STRING.
+ Seq(
+ "overlay(c PLACING 'x' FROM 1)" -> "xb ",
+ "overlay(v PLACING 'x' FROM 1)" -> "xb",
+ "string_agg(c, '-')" -> "ab ",
+ "listagg(c, '-')" -> "ab ",
+ "elt(1, c, 'x')" -> "ab ",
+ // right() is RuntimeReplaceable; its literal branches must agree with the substring
+ // branch, which R1 has already reduced to STRING.
+ "right(c, 2)" -> " ",
+ "left(c, 2)" -> "ab").foreach { case (expr, expected) =>
+ val df = sql(s"SELECT $expr AS r FROM std_promote")
+ assert(df.schema.head.dataType === StringType, s"$expr should return STRING")
+ checkAnswer(df, Row(expected))
+ }
+
+ // Transforming expressions must not inherit the input's length constraint: each of these
+ // produces a value whose length differs from the CHAR(5) input.
+ Seq(
+ "reverse(c)" -> " ba",
+ "hex(c)" -> "6162202020",
+ "array_join(array(c, c), '-')" -> "ab -ab ").foreach { case (expr, expected) =>
+ val df = sql(s"SELECT $expr AS r FROM std_promote")
+ assert(df.schema.head.dataType === StringType, s"$expr should return STRING")
+ checkAnswer(df, Row(expected))
+ }
+
+ // Promotion must not reach pass-through / LCT sites, which preserve CHAR/VARCHAR (R2/R3).
+ Seq(
+ "c", "coalesce(c, c)", "case when true then c else c end", "max(c)",
+ "element_at(array(c), 1)", "transform(array(c), x -> x)[0]",
+ "first_value(c) over (order by 1)").foreach { expr =>
+ val df = sql(s"SELECT $expr AS r FROM std_promote")
+ assert(df.schema.head.dataType === CharType(5), s"$expr should stay CHAR(5)")
+ }
+
+ // reverse() on non-string inputs is unaffected by the R1 change.
+ assert(sql("SELECT reverse(array(1, 2)) AS r").schema.head.dataType ===
+ ArrayType(IntegerType, containsNull = false))
+ }
+ }
+ }
+
+ // Every registered function that may return a CHAR(n)/VARCHAR(n) when handed one. These are the
+ // pass-through and container cases R2/R3 require to keep the type: aggregates and ordering
+ // functions that return one of their inputs unchanged, the null-handling family, element access,
+ // and the array/map/struct constructors along with the collection functions that rearrange
+ // elements without rewriting them. Anything not listed here must reduce to plain STRING (R1), so
+ // a newly added expression that leaks a length constraint fails this test rather than shipping.
Review Comment:
**Nit:**
Please narrow this guarantee to the invocation shapes the inventory actually exercises. The loop tries seven fixed templates, so a function that leaks a constrained type only at another supported arity or nested shape would not fail this test.
--
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
Re: [PR] [SPARK-58794][SQL] Add standardSemantics foundation for CHAR/VARCHAR [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #58033:
URL: https://github.com/apache/spark/pull/58033#issuecomment-5345713235
**Merge Summary:**
- merged into master https://github.com/apache/spark/commit/763b7c907cc198a367335e7fdcbc6ee255f2cf0f
- merged into branch-4.x https://github.com/apache/spark/commit/79f76f39861c544155b97f5e9981d00c816e25de
*Posted by `merge_spark_pr.py`*
--
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