Skip to content

Commit 51f54b0

Browse files
srielaudtenedor
authored andcommitted
[SPARK-59273][SQL] Complete CHAR/VARCHAR support at core execution boundaries
### What changes were proposed in this pull request? When `spark.sql.charVarchar.standardSemantics.enabled` is true, `CharType` and `VarcharType` are first-class `StringType` subtypes. Several execution-boundary matchers still used exact `StringType` (or an explicit CHAR/VARCHAR reject), so those paths failed or skipped constrained string columns. This patch treats CHAR/VARCHAR as the string family at: - JDBC getters/setters and JDBC array element conversion - JDBC schema inference (`CHAR`/`VARCHAR` keep first-class types when standard semantics is on, even if `charVarcharAsString` is also set) - File partition value decoding - `RowToColumnConverter` - `DataFrame.na.fill` for string replacement values - `ANALYZE TABLE ... FOR COLUMNS` (string-family stats) Read-side CHAR padding and VARCHAR overflow still come from existing CAST / `ApplyCharTypePadding` paths rather than being reimplemented in each converter. ### Why are the changes needed? With first-class CHAR/VARCHAR, JDBC scans/writes, file-only partition discovery, columnar conversion, `na.fill("...")`, and column stats currently throw or silently ignore those columns. That blocks enabling standard semantics. JIRA: https://issues.apache.org/jira/browse/SPARK-59273 (subtask of SPARK-58794) ### Does this PR introduce _any_ user-facing change? Yes, when `spark.sql.charVarchar.standardSemantics.enabled` is true (still default false): - JDBC read/write of CHAR/VARCHAR (including arrays) no longer fails with an unsupported JDBC type. - File partition columns declared as CHAR/VARCHAR can be decoded; CHAR is padded on scan and oversize VARCHAR fails with `EXCEED_LIMIT_LENGTH`. - Columnar row-to-column conversion accepts CHAR/VARCHAR. - `df.na.fill("x")` fills null CHAR/VARCHAR columns (CHAR values are padded by CAST). - `ANALYZE TABLE ... FOR COLUMNS` collects string-family stats on CHAR/VARCHAR instead of rejecting them. ### How was this patch tested? Added/extended unit tests: - `JDBCSuite`: read CHAR/VARCHAR and arrays; write CHAR/VARCHAR; standard semantics wins over `charVarcharAsString` in schema inference - `ParquetV1PartitionDiscoverySuite` / `ParquetV2PartitionDiscoverySuite`: CHAR/VARCHAR partition values and oversize VARCHAR - `RowToColumnConverterSuite`: CHAR/VARCHAR and nested CHAR arrays - `DataFrameNaFunctionsSuite`: `na.fill` on CHAR/VARCHAR - `StatisticsCollectionSuite`: `ANALYZE TABLE ... FOR COLUMNS` on CHAR/VARCHAR ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Cursor Grok 4.6 Closes #58541 from srielau/serge-rielau_data/SPARK-59273. Authored-by: Serge Rielau <serge@rielau.com> Signed-off-by: Daniel Tenedorio <daniel.tenedorio@databricks.com>
1 parent 4525eb9 commit 51f54b0

17 files changed

Lines changed: 226 additions & 19 deletions

File tree

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/EstimationUtils.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ object EstimationUtils {
107107
8 + attributes.map { attr =>
108108
if (attrStats.get(attr).map(_.avgLen.isDefined).getOrElse(false)) {
109109
attr.dataType match {
110-
case StringType =>
110+
case _: StringType =>
111111
// UTF8String: base + offset + numBytes
112112
attrStats(attr).avgLen.get + 8 + 4
113113
case _ =>

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/FilterEstimation.scala

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ case class FilterEstimation(plan: Filter) extends Logging {
293293
attr.dataType match {
294294
case _: NumericType | DateType | TimestampType | BooleanType =>
295295
evaluateBinaryForNumeric(op, attr, literal, update)
296-
case StringType | BinaryType =>
296+
case _: StringType | BinaryType =>
297297
// TODO: It is difficult to support other binary comparisons for String/Binary
298298
// type without min/max and advanced statistics like histogram.
299299
logDebug("[CBO] No range comparison statistics for String/Binary type " + attr)
@@ -329,7 +329,7 @@ case class FilterEstimation(plan: Filter) extends Logging {
329329
// We currently don't store min/max for binary/string type. For other types, if min/max are
330330
// missing, treat the range as unknown (instead of "all nulls") and fall back to NDV/histogram.
331331
val valueInRange = attr.dataType match {
332-
case StringType | BinaryType =>
332+
case _: StringType | BinaryType =>
333333
true
334334
case _ if !colStat.hasMinMaxStats =>
335335
true
@@ -353,7 +353,7 @@ case class FilterEstimation(plan: Filter) extends Logging {
353353
// We update ColumnStat structure after apply this equality predicate:
354354
// Set distinctCount to 1, nullCount to 0, and min/max values (if exist) to the literal value.
355355
val newStats = attr.dataType match {
356-
case StringType | BinaryType =>
356+
case _: StringType | BinaryType =>
357357
colStat.copy(distinctCount = Some(1), nullCount = Some(0))
358358
case _ =>
359359
colStat.copy(distinctCount = Some(1), min = Some(literal.value),
@@ -443,7 +443,7 @@ case class FilterEstimation(plan: Filter) extends Logging {
443443
}
444444

445445
// We assume the whole set since there is no min/max information for String/Binary type
446-
case StringType | BinaryType =>
446+
case _: StringType | BinaryType =>
447447
if (ndv.toDouble == 0) {
448448
return Some(0.0)
449449
}
@@ -686,7 +686,7 @@ case class FilterEstimation(plan: Filter) extends Logging {
686686
}
687687

688688
attrLeft.dataType match {
689-
case StringType | BinaryType =>
689+
case _: StringType | BinaryType =>
690690
// TODO: It is difficult to support other binary comparisons for String/Binary
691691
// type without min/max and advanced statistics like histogram.
692692
logDebug("[CBO] No range comparison statistics for String/Binary type " + attrLeft)

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/ValueInterval.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ object ValueInterval {
5353
min: Option[Any],
5454
max: Option[Any],
5555
dataType: DataType): ValueInterval = dataType match {
56-
case StringType | BinaryType => new DefaultValueInterval()
56+
case _: StringType | BinaryType => new DefaultValueInterval()
5757
case _ if min.isEmpty || max.isEmpty => new NullValueInterval()
5858
case _ =>
5959
NumericValueInterval(

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/statsEstimation/FilterEstimationSuite.scala

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,13 @@ class FilterEstimationSuite extends StatsEstimationTestBase {
7373
val colStatString = ColumnStat(distinctCount = Some(10), min = None, max = None,
7474
nullCount = Some(0), avgLen = Some(2), maxLen = Some(2))
7575

76+
val attrChar = AttributeReference("cchar", CharType(2))()
77+
val colStatChar = ColumnStat(distinctCount = Some(10), min = None, max = None,
78+
nullCount = Some(0), avgLen = Some(2), maxLen = Some(2))
79+
val attrVarchar = AttributeReference("cvarchar", VarcharType(2))()
80+
val colStatVarchar = ColumnStat(distinctCount = Some(10), min = None, max = None,
81+
nullCount = Some(0), avgLen = Some(2), maxLen = Some(2))
82+
7683
// column cint2 has values: 7, 8, 9, 10, 11, 12, 13, 14, 15, 16
7784
// Hence, distinctCount:10, min:7, max:16, nullCount:0, avgLen:4, maxLen:4
7885
// This column is created to test "cint < cint2
@@ -121,6 +128,8 @@ class FilterEstimationSuite extends StatsEstimationTestBase {
121128
attrDecimal -> colStatDecimal,
122129
attrDouble -> colStatDouble,
123130
attrString -> colStatString,
131+
attrChar -> colStatChar,
132+
attrVarchar -> colStatVarchar,
124133
attrInt2 -> colStatInt2,
125134
attrInt3 -> colStatInt3,
126135
attrInt4 -> colStatInt4,
@@ -570,6 +579,29 @@ class FilterEstimationSuite extends StatsEstimationTestBase {
570579
expectedRowCount = 10)
571580
}
572581

582+
test("SPARK-59273: CHAR/VARCHAR equality, IN, and range fall back like STRING") {
583+
Seq(attrChar -> colStatChar, attrVarchar -> colStatVarchar).foreach {
584+
case (attr, colStat) =>
585+
validateEstimatedStats(
586+
Filter(EqualTo(attr, Literal("A2")), childStatsTestPlan(Seq(attr), 10L)),
587+
Seq(attr -> colStat.copy(distinctCount = Some(1), nullCount = Some(0))),
588+
expectedRowCount = 1)
589+
validateEstimatedStats(
590+
Filter(InSet(attr, Set("A0")), childStatsTestPlan(Seq(attr), 10L)),
591+
Seq(attr -> colStat.copy(distinctCount = Some(1), nullCount = Some(0))),
592+
expectedRowCount = 1)
593+
validateEstimatedStats(
594+
Filter(LessThan(attr, Literal("A2")), childStatsTestPlan(Seq(attr), 10L)),
595+
Seq(attr -> colStat),
596+
expectedRowCount = 10)
597+
}
598+
validateEstimatedStats(
599+
Filter(GreaterThan(attrChar, attrVarchar),
600+
childStatsTestPlan(Seq(attrChar, attrVarchar), 10L)),
601+
Seq(attrChar -> colStatChar, attrVarchar -> colStatVarchar),
602+
expectedRowCount = 10)
603+
}
604+
573605
test("cint IN (1, 2, 3, 4, 5)") {
574606
// This is a corner test case. We want to test if we can handle the case when the number of
575607
// valid values in IN clause is greater than the number of distinct values for a given column.

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/statsEstimation/JoinEstimationSuite.scala

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,10 @@ class JoinEstimationSuite extends StatsEstimationTestBase {
499499
nullCount = Some(0), avgLen = Some(16), maxLen = Some(16)),
500500
AttributeReference("cstring", StringType)() -> ColumnStat(distinctCount = Some(1),
501501
min = None, max = None, nullCount = Some(0), avgLen = Some(3), maxLen = Some(3)),
502+
AttributeReference("cchar", CharType(3))() -> ColumnStat(distinctCount = Some(1),
503+
min = None, max = None, nullCount = Some(0), avgLen = Some(3), maxLen = Some(3)),
504+
AttributeReference("cvarchar", VarcharType(3))() -> ColumnStat(distinctCount = Some(1),
505+
min = None, max = None, nullCount = Some(0), avgLen = Some(3), maxLen = Some(3)),
502506
AttributeReference("cbinary", BinaryType)() -> ColumnStat(distinctCount = Some(1),
503507
min = None, max = None, nullCount = Some(0), avgLen = Some(3), maxLen = Some(3)),
504508
AttributeReference("cdate", DateType)() -> ColumnStat(distinctCount = Some(1),
@@ -531,6 +535,10 @@ class JoinEstimationSuite extends StatsEstimationTestBase {
531535
rowCount = Some(1),
532536
attributeStats = AttributeMap(Seq(key1 -> columnInfo1(key1), key2 -> columnInfo1(key1))))
533537
assert(join.stats == expectedStats)
538+
if (key1.dataType.isInstanceOf[StringType]) {
539+
// UTF8 rows include the average payload length plus base and offset overhead.
540+
assert(join.stats.sizeInBytes == 38)
541+
}
534542
}
535543
}
536544
}

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/statsEstimation/StatsEstimationTestBase.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ trait StatsEstimationTestBase extends SparkFunSuite {
4646

4747
def getColSize(attribute: Attribute, colStat: ColumnStat): Long = attribute.dataType match {
4848
// For UTF8String: base + offset + numBytes
49-
case StringType => colStat.avgLen.getOrElse(attribute.dataType.defaultSize.toLong) + 8 + 4
49+
case _: StringType => colStat.avgLen.getOrElse(attribute.dataType.defaultSize.toLong) + 8 + 4
5050
case _ => colStat.avgLen.getOrElse(attribute.dataType.defaultSize)
5151
}
5252

sql/core/src/main/scala/org/apache/spark/sql/classic/DataFrameNaFunctions.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ final class DataFrameNaFunctions private[sql](df: DataFrame)
249249
val projections = outputAttributes.map { col =>
250250
val typeMatches = (targetType, col.dataType) match {
251251
case (NumericType, dt) => dt.isInstanceOf[NumericType]
252-
case (StringType, dt) => dt == StringType
252+
case (StringType, _: StringType) => true
253253
case (BooleanType, dt) => dt == BooleanType
254254
case _ =>
255255
throw new IllegalArgumentException(s"$targetType is not matched at fillValue")

sql/core/src/main/scala/org/apache/spark/sql/execution/Columnar.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ private object RowToColumnConverter {
298298
case LongType | TimestampType | TimestampNTZType | _: DayTimeIntervalType | _: TimeType =>
299299
LongConverter
300300
case DoubleType => DoubleConverter
301-
case StringType => StringConverter
301+
case _: StringType => StringConverter
302302
case _: GeographyType | _: GeometryType => BinaryViewConverter
303303
case CalendarIntervalType => CalendarConverter
304304
case VariantType => VariantConverter

sql/core/src/main/scala/org/apache/spark/sql/execution/command/AnalyzeColumnCommand.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,6 @@ case class AnalyzeColumnCommand(
142142
case DoubleType | FloatType => true
143143
case BooleanType => true
144144
case _: DatetimeType => true
145-
case _: CharType | _: VarcharType => false
146145
case BinaryType | _: StringType => true
147146
case _ => false
148147
}

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/PartitioningUtils.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,7 @@ object PartitioningUtils extends SQLConfHelper {
550550
zoneId: ZoneId): Any = desiredType match {
551551
case _ if value == DEFAULT_PARTITION_NAME => null
552552
case NullType => null
553-
case StringType => UTF8String.fromString(unescapePathName(value))
553+
case _: StringType => UTF8String.fromString(unescapePathName(value))
554554
case ByteType => Integer.parseInt(value).toByte
555555
case ShortType => Integer.parseInt(value).toShort
556556
case IntegerType => Integer.parseInt(value)

0 commit comments

Comments
 (0)