Skip to content

[SPARK-58803][SQL] Dataset/encoder/UDF CHAR/VARCHAR under standardSemantics - #58059

Closed
srielau wants to merge 9 commits into
apache:masterfrom
srielau:SPARK-58803
Closed

[SPARK-58803][SQL] Dataset/encoder/UDF CHAR/VARCHAR under standardSemantics#58059
srielau wants to merge 9 commits into
apache:masterfrom
srielau:SPARK-58803

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: srielau/spark@SPARK-58794...SPARK-58803

SPARK-58803: prove Dataset / encoder / UDF CHAR/VARCHAR surfaces under spark.sql.charVarchar.standardSemantics.enabled.

The foundation already gates these APIs via charVarcharFirstClassTypes (failIfHasCharVarchar, RowEncoderCharEncoder/VarcharEncoder, CatalystTypeConverters Char/Varchar converters) and applies write-side pad/trim/EXCEED_LIMIT_LENGTH. This PR adds the end-to-end coverage that belongs with that API contract:

  • createDataFrame / Encoders.CHAR / Encoders.VARCHAR: pad + oversize
  • spark.udf.register and Java udf(..., returnType): typed results + length enforcement
  • Dataset.to: CHAR/VARCHAR target schema + narrowing assignment
  • DataFrameReader / DataStreamReader user schemas keep CHAR/VARCHAR
  • RowEncoderSuite / UDFSuite: standardSemantics path (not only preserveCharVarcharTypeInfo)

Why are the changes needed?

Without dedicated tests, Dataset/UDF/reader CHAR/VARCHAR under standardSemantics is only incidentally covered. SPARK-58803 is the API-surface closure for Phase 2.

Does this PR introduce any user-facing change?

No new behavior beyond what #58033 already enables when the flag is on; this PR adds tests.

How was this patch tested?

./build/sbt "sql/testOnly org.apache.spark.sql.BasicCharVarcharTestSuite -- -z SPARK-58803"
./build/sbt "sql/testOnly org.apache.spark.sql.UDFSuite -- -z \"char/varchar as UDF return type\""
./build/sbt "catalyst/testOnly org.apache.spark.sql.catalyst.encoders.RowEncoderSuite -- -z SPARK-58803"
./build/sbt "catalyst/testOnly org.apache.spark.sql.catalyst.encoders.RowEncoderSuite -- -z char/varchar"

All of the above succeeded locally.

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.
Covers SPARK-58802: prove the fixed-point and single-pass analyzers agree
under spark.sql.charVarchar.standardSemantics.enabled (D19). The single-pass
resolver has no Char/Varchar-specific logic -- it inherits Expression.dataType
and shared TypeCoercion -- so parity is verification of the foundation code
rather than a separate change, and the matrix belongs with the code it proves.
Adds dual-run assertions for 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.
…antics
Allowing CharType/VarcharType in createDataFrame, Encoders.CHAR/VARCHAR,
UDF return types, Dataset.to, and DataFrameReader/DataStreamReader schemas
was already gated by charVarcharFirstClassTypes in the SPARK-58794 foundation,
with write-side pad/trim/EXCEED_LIMIT_LENGTH via CharEncoder and
CatalystTypeConverters. This subtask proves those API surfaces end-to-end
under spark.sql.charVarchar.standardSemantics.enabled and extends
RowEncoderSuite / UDFSuite coverage beyond the preserve-only path.
@srielau

Copy link
Copy Markdown
ContributorAuthor

Folding this into #58033. I verified with a throwaway probe that every path here already works on the foundation branch with no code from this PR: createDataFrame from an RDD pads and rejects oversize, .as[String]/.map see the padded value, collect() returns the padded string, groupBy keys on it, and a Scala UDF with a String parameter fed a CHAR(5) column receives the padded value.

What actually unlocks these surfaces is the change to failIfHasCharVarchar in #58033 (on master it rejected CHAR/VARCHAR outright, never consulting preserveCharVarcharTypeInfo). So these tests belong with that gate change rather than in a PR of their own. Carried over as 5a51ba2.

@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