Skip to content

[SPARK-58802][SQL] Dual-run coverage for CHAR/VARCHAR standardSemantics - #58057

Closed
srielau wants to merge 8 commits into
apache:masterfrom
srielau:SPARK-58802
Closed

[SPARK-58802][SQL] Dual-run coverage for CHAR/VARCHAR standardSemantics#58057
srielau wants to merge 8 commits into
apache:masterfrom
srielau:SPARK-58802

Conversation

@srielau

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Depends on #58033 (SPARK-58794 foundation). Please review/merge that first; rebase this onto master afterward. Delta vs foundation is a single commit: srielau/spark@SPARK-58794...SPARK-58802

Expands HybridAnalyzer dual-run coverage under spark.sql.charVarchar.standardSemantics.enabled so fixed-point and single-pass analyzers are proven to agree on (D19):

  • CAST / try_cast type introduction (R3)
  • LCT: COALESCE / CASE / NULL / CHAR+VARCHAR / CHAR+STRING / IN / UNION / INTERSECT (R2)
  • R1 transforming exprs: upper/lower/||/concat/substr/trim/regexp_replace/mask/split, including R1 after LCT
  • Nested ARRAY/STRUCT of CHAR
  • Collated CAST
  • Bare column references from a CHAR/VARCHAR table

The single-pass resolver has no Char/Varchar-specific logic; it inherits Expression.dataType and shared TypeCoercion. Dual-run with sample rate 1.0 is the proof of parity.

Why are the changes needed?

Parent design decision D19: single-pass must match fixed-point on LCT/CAST/R1 before calling the standardSemantics foundation complete. #58033 only had a small smoke test.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

./build/sbt "sql/testOnly org.apache.spark.sql.BasicCharVarcharTestSuite -- -z SPARK-58802"

Result: 1 test succeeded (dual-run matrix under standardSemantics).

Introduce spark.sql.charVarchar.standardSemantics.enabled and wire
first-class types, real CAST, LCT, R1 STRING results, write-side typing,
and read-side pad/oversize checks (SPARK-58796 through SPARK-58801).
…dence
- Preserve collationId when tightestCommonString returns CHAR/VARCHAR.
- Apply R1 (transforming string functions return plain STRING) to
regexp_replace/regexp_extract/regexp_extract_all/split and mask.
- Make charVarchar.standardSemantics take precedence over the legacy
charVarcharAsString flag in cast replacement and read-side padding.
- addPaddingForScan falls back to attr.dataType when the raw-type metadata
is absent, so first-class CHAR/VARCHAR attributes still get enforced.
- Add tests for LCT collation, R1 on regexp/mask/split, and the
preserveCharVarcharTypeInfo vs standardSemantics flag matrix.
…low-ups
- Add charvarchar-standard-semantics.sql covering CAST/LCT/R1/UNION/IN/scan,
including try_cast, regexp/mask/split, and nested types.
- Preserve declared collation Option in tightestCommonString so default LCT
still renders as char(n)/varchar(n) (not collate UTF8_BINARY).
- Revert attr.dataType fallback in addPaddingForScan: metadata is the
not-yet-padded marker required for ApplyCharTypePadding idempotence.
- Warn once when readSideCharPadding=false under standardSemantics; document
that read-side checks are identical to write-side by design.
- Pin dual-run Analyzer++ parity coverage for CAST/LCT/R1/UNION.
…, binding policy
Three CI failures from the standardSemantics foundation:
- Guard the multi-byte VARCHAR test's CJK literals with scalastyle:off/on
nonascii, matching the convention used by the variant and collation suites.
- Stop the Thrift server from wrapping CHAR/VARCHAR values in quotes. The
column-oriented fast path in RowSetUtils matches only the default-collation
StringType singleton, so first-class CHAR/VARCHAR (and collated strings) fall
through to the generic branch that calls toHiveString with nested = true.
Treat string types like the geospatial types already handled there.
- Give spark.sql.charVarchar.standardSemantics.enabled a binding policy. It is
PERSISTED: the flag decides which types a view body resolves to, so a view
created under standard semantics keeps computing CHAR/VARCHAR no matter who
calls it, the same reasoning that applies to ANSI mode.
… RowEncoder
Pass the full CharType/VarcharType into CharEncoder/VarcharEncoder (like
GeographyEncoder) so createDataFrame retains a declared collation instead of
rebuilding a default-collation type from length alone. Also short-circuit the
CheckAnalysis leaf guard, fix the VARCHAR read-side javadoc link, and rephrase
a scan-padding test comment.
Expand the HybridAnalyzer dual-run matrix under
spark.sql.charVarchar.standardSemantics.enabled so fixed-point and
single-pass analyzers are proven to agree on CAST/try_cast, LCT
(COALESCE/CASE/NULL/IN/UNION/INTERSECT), R1 transforming expressions
(including regexp/mask/split), nested CHAR types, collated CAST, and
bare column references from tables (D19).
@srielau

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of #58033.

These are tests that verify the foundation change, not a change of their own: the single-pass resolver has no CHAR/VARCHAR-specific logic, so parity under spark.sql.charVarchar.standardSemantics.enabled follows from Expression.dataType and shared TypeCoercion in #58033. Reviewing the matrix apart from the code it exercises made both harder to judge, so the coverage now lives alongside that code and SPARK-58802 is satisfied there.

@srielausrielau closed this Aug 18, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@srielau