This is an automated email from the ASF dual-hosted git repository.
JingsongLi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/paimon.git
The following commit(s) were added to refs/heads/master by this push:
new 1c3df8f8db [spark] Reject empty partition values for format tables
(#9436)
1c3df8f8db is described below
commit 1c3df8f8db760d7acc68346f137aac63559db31a
Author: Dapeng Sun(孙大鹏) <[email protected]>
AuthorDate: Fri Aug 28 13:48:54 2026 +0800
[spark] Reject empty partition values for format tables (#9436)
---
docs/docs/spark/sql-ddl.md | 6 ++
.../paimon/spark/format/PaimonFormatTable.scala | 40 +++++++-
.../CatalogManagedPartitionEdgeParityTest.scala | 114 ++++++++++++++++++++-
3 files changed, 156 insertions(+), 4 deletions(-)
diff --git a/docs/docs/spark/sql-ddl.md b/docs/docs/spark/sql-ddl.md
index e4ef5c707d..657ef33220 100644
--- a/docs/docs/spark/sql-ddl.md
+++ b/docs/docs/spark/sql-ddl.md
@@ -226,6 +226,12 @@ On a Format Table whose partitions are discovered from the
filesystem, `ADD PART
added partition before any data is written returns no rows. `DROP PARTITION`
unregisters the
partition and deletes its directory.
+A partition value that is empty or all whitespace is rejected by `ADD
PARTITION`, `DROP PARTITION`
+and `TRUNCATE PARTITION`. Such a value is written to the partition named by
+`partition.default-name` (`__DEFAULT_PARTITION__` unless configured
otherwise), the same partition
+a `NULL` is written to, so it names that partition rather than one of its own
- name it directly
+instead. Writing one through `INSERT` is unaffected and still lands there.
+
`ANALYZE TABLE` measures partitions. A Format Table has no snapshot to carry a
table-level
statistic and no column statistics, so `COMPUTE STATISTICS FOR COLUMNS` and
`FOR ALL COLUMNS` are
not supported on it; what the statement writes back to the catalog is the file
count, byte size,
diff --git
a/paimon-spark/paimon-spark-common/src/main/scala/org/apache/paimon/spark/format/PaimonFormatTable.scala
b/paimon-spark/paimon-spark-common/src/main/scala/org/apache/paimon/spark/format/PaimonFormatTable.scala
index baa7d21e40..08de978567 100644
---
a/paimon-spark/paimon-spark-common/src/main/scala/org/apache/paimon/spark/format/PaimonFormatTable.scala
+++
b/paimon-spark/paimon-spark-common/src/main/scala/org/apache/paimon/spark/format/PaimonFormatTable.scala
@@ -27,10 +27,11 @@ import org.apache.paimon.table.FormatTable
import org.apache.paimon.table.format.FormatTablePartitionManager
import org.apache.paimon.table.sink.BatchTableCommit
import org.apache.paimon.types.RowType
-import org.apache.paimon.utils.PartitionPathUtils
+import org.apache.paimon.utils.{PartitionPathUtils, StringUtils}
import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.analysis.{NoSuchPartitionException,
NoSuchPartitionsException}
+import org.apache.spark.sql.catalyst.util.CharVarcharUtils
import org.apache.spark.sql.connector.catalog.{SupportsRead, SupportsWrite,
TableCapability, TableCatalog, TruncatableTable}
import org.apache.spark.sql.connector.catalog.TableCapability.{BATCH_READ,
BATCH_WRITE, OVERWRITE_BY_FILTER, OVERWRITE_DYNAMIC}
import org.apache.spark.sql.connector.distributions.Distribution
@@ -39,7 +40,7 @@ import org.apache.spark.sql.connector.read.ScanBuilder
import org.apache.spark.sql.connector.write._
import org.apache.spark.sql.connector.write.streaming.StreamingWrite
import org.apache.spark.sql.paimon.shims.SparkShimLoader
-import org.apache.spark.sql.types.StructType
+import org.apache.spark.sql.types.{StringType, StructType}
import org.apache.spark.sql.util.CaseInsensitiveStringMap
import java.util
@@ -139,6 +140,8 @@ case class PaimonFormatTable(table: FormatTable)
return true
}
val partitionKeys = table.partitionKeys().asScala.toSeq
+ idents.foreach(
+ ident => requireNameablePartitionValues("TRUNCATE PARTITION", ident,
partitionKeys))
val specs = idents.map {
ident =>
require(
@@ -205,6 +208,35 @@ case class PaimonFormatTable(table: FormatTable)
requested.map(spec => registeredSpecs.contains(spec.asScala.toMap))
}
+ /**
+ * Rejects partition values the table cannot name. An empty or
whitespace-only string collapses to
+ * the default partition name on its way to the directory, the same name a
`NULL` gets, so the
+ * spec describes the null partition rather than one of its own. Adding it
registers a partition
+ * the value cannot round-trip to; dropping or truncating it hits the null
partition instead.
+ *
+ * `NULL` itself keeps its defined encoding and stays on that path.
+ */
+ private def requireNameablePartitionValues(
+ operation: String,
+ row: InternalRow,
+ partitionNames: Seq[String]): Unit = {
+ val fields = partitionSchema.fields.map(field => field.name -> field).toMap
+ partitionNames.take(row.numFields).zipWithIndex.foreach {
+ case (name, index) =>
+ val dataType =
CharVarcharUtils.replaceCharVarcharWithString(fields(name).dataType)
+ if (dataType == StringType && !row.isNullAt(index)) {
+ if (StringUtils.isNullOrWhitespaceOnly(row.getString(index))) {
+ val defaultPartitionName =
+ CoreOptions.fromMap(table.options()).partitionDefaultName()
+ throw new IllegalArgumentException(
+ s"$operation does not support an empty or whitespace-only string
for partition " +
+ s"column $name of Format Table ${table.fullName()}. Such a
value is written to " +
+ s"the partition named $defaultPartitionName, name it directly
to address it.")
+ }
+ }
+ }
+ }
+
private[spark] def createFormatTablePartitions(
rows: Array[InternalRow],
maps: Array[JMap[String, String]],
@@ -216,6 +248,7 @@ case class PaimonFormatTable(table: FormatTable)
val onlyValueInPath =
CoreOptions.fromMap(table.options()).formatTablePartitionOnlyValueInPath()
val partitionKeys = table.partitionKeys().asScala.toSeq
+ rows.foreach(row => requireNameablePartitionValues("ADD PARTITION", row,
partitionKeys))
val specs = rows.map(row => toPaimonPartition(row,
partitionKeys.take(row.numFields))).toSeq
// Resolve (and path-safety validate) every directory before mutating
anything.
val partitionPaths =
@@ -238,6 +271,9 @@ case class PaimonFormatTable(table: FormatTable)
partitionNames: Array[Array[String]],
rows: Array[InternalRow]): Boolean = {
val partitionKeyCount = table.partitionKeys().size()
+ rows.zip(partitionNames).foreach {
+ case (row, names) => requireNameablePartitionValues("DROP PARTITION",
row, names.toSeq)
+ }
val requested =
rows.zip(partitionNames).map { case (row, names) =>
toPaimonPartition(row, names.toSeq) }
val partitions = ArrayBuffer.empty[JMap[String, String]]
diff --git
a/paimon-spark/paimon-spark-ut/src/test/scala/org/apache/paimon/spark/sql/CatalogManagedPartitionEdgeParityTest.scala
b/paimon-spark/paimon-spark-ut/src/test/scala/org/apache/paimon/spark/sql/CatalogManagedPartitionEdgeParityTest.scala
index 14333f57e5..e92bd629e3 100644
---
a/paimon-spark/paimon-spark-ut/src/test/scala/org/apache/paimon/spark/sql/CatalogManagedPartitionEdgeParityTest.scala
+++
b/paimon-spark/paimon-spark-ut/src/test/scala/org/apache/paimon/spark/sql/CatalogManagedPartitionEdgeParityTest.scala
@@ -211,6 +211,114 @@ class CatalogManagedPartitionEdgeParityTest extends
PaimonSparkTestWithRestCatal
}
}
+ private val unnameableValues = Seq("empty" -> "", "blank" -> " ")
+
+ Seq("STRING" -> "string", "CHAR(8)" -> "char", "VARCHAR(8)" ->
"varchar").foreach {
+ case (partitionType, typeSuffix) =>
+ unnameableValues.foreach {
+ case (valueLabel, value) =>
+ test(
+ s"ADD PARTITION rejects an $valueLabel $partitionType value
without changing state") {
+ val tableName = s"edge_reject_${valueLabel}_$typeSuffix"
+ withTable(tableName) {
+ createTable(tableName, partitionType)
+
+ val error = intercept[Exception] {
+ sql(s"ALTER TABLE ${qualified(tableName)} " +
+ s"ADD PARTITION (dt = '$value', hour = '00')").collect()
+ }
+
+ assert(
+ error.getMessage.contains(
+ "ADD PARTITION does not support an empty or whitespace-only
string"))
+ assert(error.getMessage.contains("partition column dt"))
+ assert(registered(tableName).isEmpty)
+ assert(!directoryExists(tableName,
s"dt=$defaultPartitionName/hour=00"))
+ }
+ }
+ }
+ }
+
+ test("ADD PARTITION names the offending column, wherever it sits in the
spec") {
+ val tableName = "edge_empty_second_column"
+ withTable(tableName) {
+ createTable(tableName)
+
+ val error = intercept[Exception] {
+ sql(
+ s"ALTER TABLE ${qualified(tableName)} " +
+ "ADD PARTITION (dt = '20260101', hour = '')").collect()
+ }
+
+ assert(error.getMessage.contains("partition column hour"))
+ assert(registered(tableName).isEmpty)
+ assert(!directoryExists(tableName,
s"dt=20260101/hour=$defaultPartitionName"))
+ }
+ }
+
+ test("one bad spec fails an ADD PARTITION batch before any of it is
applied") {
+ val tableName = "edge_empty_batch"
+ withTable(tableName) {
+ createTable(tableName)
+
+ val error = intercept[Exception] {
+ sql(
+ s"ALTER TABLE ${qualified(tableName)} ADD " +
+ "PARTITION (dt = '20260101', hour = '00') PARTITION (dt = '', hour
= '01')").collect()
+ }
+
+ assert(
+ error.getMessage.contains(
+ "ADD PARTITION does not support an empty or whitespace-only string"))
+ assert(registered(tableName).isEmpty)
+ assert(!directoryExists(tableName, "dt=20260101/hour=00"))
+ }
+ }
+
+ test("DROP PARTITION rejects an empty string instead of dropping the default
partition") {
+ val tableName = "edge_empty_drop"
+ withTable(tableName) {
+ createTable(tableName)
+ sql(s"INSERT INTO ${qualified(tableName)} VALUES (1, 'a', '', '00')")
+
+ val error = intercept[Exception] {
+ sql(
+ s"ALTER TABLE ${qualified(tableName)} " +
+ "DROP PARTITION (dt = '', hour = '00')").collect()
+ }
+
+ assert(
+ error.getMessage.contains(
+ "DROP PARTITION does not support an empty or whitespace-only
string"))
+ assert(registered(tableName) == Set(s"$defaultPartitionName/00"))
+ assert(directoryExists(tableName, s"dt=$defaultPartitionName/hour=00"))
+ checkAnswer(
+ sql(s"SELECT id, payload, dt, hour FROM ${qualified(tableName)}"),
+ Seq(Row(1, "a", null, "00")))
+ }
+ }
+
+ test("TRUNCATE PARTITION rejects an empty string instead of emptying the
default partition") {
+ val tableName = "edge_empty_truncate"
+ withTable(tableName) {
+ createTable(tableName)
+ sql(s"INSERT INTO ${qualified(tableName)} VALUES (1, 'a', '', '00')")
+
+ val error = intercept[Exception] {
+ sql(
+ s"TRUNCATE TABLE ${qualified(tableName)} " +
+ "PARTITION (dt = '', hour = '00')").collect()
+ }
+
+ assert(
+ error.getMessage.contains(
+ "TRUNCATE PARTITION does not support an empty or whitespace-only
string"))
+ checkAnswer(
+ sql(s"SELECT id, payload, dt, hour FROM ${qualified(tableName)}"),
+ Seq(Row(1, "a", null, "00")))
+ }
+ }
+
test("an empty string partition value uses the default partition encoding") {
val tableName = "edge_empty_string"
withTable(tableName) {
@@ -267,8 +375,10 @@ class CatalogManagedPartitionEdgeParityTest extends
PaimonSparkTestWithRestCatal
private def qualified(tableName: String): String =
s"paimon.$dbName0.$tableName"
- private def createTable(tableName: String): Unit =
- sql(s"""CREATE TABLE $tableName (id INT, payload STRING, dt STRING, hour
STRING)
+ private def createTable(tableName: String): Unit = createTable(tableName,
"STRING")
+
+ private def createTable(tableName: String, partitionType: String): Unit =
+ sql(s"""CREATE TABLE $tableName (id INT, payload STRING, dt
$partitionType, hour STRING)
|USING CSV
|PARTITIONED BY (dt, hour)
|TBLPROPERTIES (