srielau commented on code in PR #58255:
URL: https://github.com/apache/spark/pull/58255#discussion_r3855418762
##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala:
##########
@@ -408,7 +408,14 @@ private[client] class Shim_v2_0 extends Shim with Logging {
}
}
- if (!SQLConf.get.metastorePartitionPruningFastFallback ||
+ // CHAR/VARCHAR partition keys are excluded from the metastore filter (see
+ // SupportedAttribute), because Hive compares them with its own
trailing-blank rules. Under
+ // standard semantics that would leave such a query fetching every
partition, so prune on the
+ // client instead, where Spark's own comparison semantics apply.
+ val charVarcharPartitionKey = SQLConf.get.charVarcharStandardSemantics &&
+ catalogTable.partitionSchema.exists(f =>
CharVarcharUtils.hasCharVarchar(f.dataType))
+
+ if ((!SQLConf.get.metastorePartitionPruningFastFallback &&
!charVarcharPartitionKey) ||
Review Comment:
Agreed. Client-side prune now keys off predicates that mention a
CHAR/VARCHAR partition attribute, not the table's partition schema. The
MetaException path still honors `metastorePartitionPruningFastFallback`. The
guard is now `useClientSidePrune = fastFallback ||
referencesCharVarcharPartitionKey`.
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveCharVarcharTestSuite.scala:
##########
@@ -91,6 +93,41 @@ class HiveCharVarcharTestSuite extends CharVarcharTestSuite
with TestHiveSinglet
}
}
}
+
+ test("SPARK-58794: CHAR partition filters use the same Hive prune path as
STRING") {
+ // Keep the relation a HiveTableRelation, otherwise the scan is converted
to a file index
+ // and never reaches HiveShim's metastore filter conversion.
+ withSQLConf(
+ SQLConf.CHAR_VARCHAR_STANDARD_SEMANTICS.key -> "true",
+ SQLConf.HIVE_METASTORE_PARTITION_PRUNING.key -> "true",
+ HiveUtils.CONVERT_METASTORE_PARQUET.key -> "false") {
+ val partitionValues = Seq("a", "b", "c", "d", "e")
+
+ def partitionsFetched(partitionType: String, literal: String): Long = {
+ var fetched = 0L
+ withTable("std_hive_part") {
+ sql(
+ s"""CREATE TABLE std_hive_part (i INT, p $partitionType)
+ |USING $format PARTITIONED BY (p)""".stripMargin)
+ partitionValues.foreach { v =>
+ sql(s"INSERT INTO std_hive_part PARTITION (p='$v') VALUES (1)")
+ }
+ HiveCatalogMetrics.reset()
+ checkAnswer(sql(s"SELECT i FROM std_hive_part WHERE p = $literal"),
Row(1))
+ fetched = HiveCatalogMetrics.METRIC_PARTITIONS_FETCHED.getCount
+ }
+ fetched
+ }
+
+ val stringFetched = partitionsFetched("STRING", "'a'")
+ // Standard semantics compare CHAR without PAD SPACE, so the literal
carries the pad.
+ val charFetched = partitionsFetched("CHAR(5)", "'a '")
+ assert(stringFetched < partitionValues.length,
+ s"STRING baseline did not prune: fetched $stringFetched of
${partitionValues.length}")
+ assert(charFetched === stringFetched,
+ s"CHAR fetched $charFetched partitions but STRING fetched
$stringFetched")
Review Comment:
Done. The test is now SPARK-59001, asserts `METRIC_PARTITIONS_FETCHED == 1`
(with `checkToRDD = false` so the metric is not doubled), covers VARCHAR, and
checks `p = 'a'` (no CHAR match) against the padded literal.
##########
sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala:
##########
@@ -2219,6 +2256,26 @@ class FileSourceCharVarcharTestSuite extends
CharVarcharTestSuite with SharedSpa
checkAnswer(sql("SELECT * FROM t"), Row("12"))
}
}
+ // Catalog write/read and file inference both keep CHAR/VARCHAR.
+ withTable("std_parquet") {
+ sql(s"CREATE TABLE std_parquet (c CHAR(5), v VARCHAR(5)) USING
$format")
+ sql("INSERT INTO std_parquet VALUES ('ab', 'cd')")
+ assert(spark.table("std_parquet").schema.map(_.dataType) ===
+ Seq(CharType(5), VarcharType(5)))
+ checkAnswer(
+ sql("SELECT concat('<', c, '>'), concat('<', v, '>') FROM
std_parquet"),
+ Row("<ab >", "<cd>"))
+ }
+ withTempPath { dir =>
+ val path = dir.getCanonicalPath
+ sql("SELECT CAST('ab' AS CHAR(4)) AS
c").write.mode("overwrite").format(format).save(path)
+ val inferred = spark.read.format(format).load(path)
+ assert(inferred.schema.head.dataType === CharType(4))
+ checkAnswer(inferred.selectExpr("concat('<', c, '>')"), Row("<ab >"))
+ val catalog = spark.read.schema("c
VARCHAR(4)").format(format).load(path)
+ assert(catalog.schema.head.dataType === VarcharType(4))
+ checkAnswer(catalog, Row("ab "))
+ }
Review Comment:
Dropped. Those catalog/inference checks do not exercise Empty2Null, text, or
Hive prune; they belong with the remaining format round-trips on SPARK-58814.
--
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]
