cloud-fan commented on code in PR #58541:
URL: https://github.com/apache/spark/pull/58541#discussion_r3960068718
##########
sql/core/src/test/scala/org/apache/spark/sql/StatisticsCollectionSuite.scala:
##########
@@ -158,6 +158,50 @@ class StatisticsCollectionSuite extends
StatisticsCollectionTestBase with Shared
}
}
+ test("SPARK-59273: collect CHAR/VARCHAR column statistics") {
+ withSQLConf(SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true") {
+ val tableName = "char_varchar_column_stats"
+ withTable(tableName) {
+ sql(s"CREATE TABLE $tableName(c CHAR(3), v VARCHAR(3)) USING parquet")
+ sql(s"INSERT INTO $tableName VALUES ('a', 'x'), ('bb', 'yz'), (NULL,
NULL)")
+ sql(s"ANALYZE TABLE $tableName COMPUTE STATISTICS FOR COLUMNS c, v")
+
+ val columnStats = getCatalogTable(tableName).stats.get.colStats
+ assert(columnStats.keySet === Set("c", "v"))
+ assert(columnStats("c").distinctCount.contains(BigInt(2)))
+ assert(columnStats("v").distinctCount.contains(BigInt(2)))
+ assert(columnStats("c").nullCount.contains(BigInt(1)))
+ assert(columnStats("v").nullCount.contains(BigInt(1)))
+ }
+ }
+ }
+
+ test("SPARK-59273: CBO plans CHAR/VARCHAR predicates after ANALYZE") {
+ withSQLConf(
+ SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true",
+ SQLConf.CBO_ENABLED.key -> "true") {
+ val tableName = "char_varchar_cbo_stats"
+ withTable(tableName) {
+ sql(s"CREATE TABLE $tableName(c CHAR(3), v VARCHAR(3)) USING parquet")
+ sql(s"INSERT INTO $tableName VALUES ('a', 'x'), ('bb', 'yz'), (NULL,
NULL)")
+ sql(s"ANALYZE TABLE $tableName COMPUTE STATISTICS FOR COLUMNS c, v")
+
+ // CBO FilterEstimation used to MatchError on CharType/VarcharType
after ANALYZE.
Review Comment:
**Nit (P3):** `MatchError` is the exception type, so `used to MatchError` is
ungrammatical. Please make the thrown action explicit.
```suggestion
// CBO FilterEstimation used to throw a MatchError on
CharType/VarcharType after ANALYZE.
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/ValueInterval.scala:
##########
@@ -53,7 +53,7 @@ object ValueInterval {
min: Option[Any],
max: Option[Any],
dataType: DataType): ValueInterval = dataType match {
- case StringType | BinaryType => new DefaultValueInterval()
+ case _: StringType | BinaryType => new DefaultValueInterval()
Review Comment:
**Non-blocking (P2):** This widens the interval path used by
`JoinEstimation`, and the patch also widens its UTF8 row-size accounting, but
the owning `JoinEstimationSuite` mixed-type matrix still has only `StringType`
and `BinaryType`. Please add both `CharType` and `VarcharType` keys there and
assert the nonzero NDV-derived row count, propagated key statistics, and
explicit string-overhead `sizeInBytes`; otherwise these two changed branches
can regress together while the new filter tests stay green.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
