-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[spark] fix drop partition when partition value is null or empty string #6662
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
cc @JingsongLi @Zouxxyy @YannByron Can you take a look when you have time? Thanks! |
JingsongLi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean HiveCatalog? Can we fix HiveCatalog.dropPartitions to deal with default values?
|
@JingsongLi Thanks for looking at this. I think this issue occurs for all catalogs. The root cause lies in Spark’s current partition-dropping logic, which incorrectly transforms partition values using InternalRowPartitionComputer—not in the catalog implementation itself. Here's how the issue unfolds: When executing ALTER TABLE T DROP PARTITION(pt=''), PaimonPartitionManagement#toPaimonPartitions uses InternalRowPartitionComputer to convert the Spark Row into a partition spec of type Map<String, String>. During this conversion, the empty string ('') is replaced with the default partition value, resulting in (pt='DEFAULT_PARTITION'). This spec is then passed to FileStoreCommitImpl#dropPartitions to delete the partition. Inside dropPartitions, the partition spec (pt='DEFAULT_PARTITION') is converted back into a Paimon internal row via paimon/paimon-common/src/main/java/org/apache/paimon/utils/InternalRowPartitionComputer.java Lines 135 to 153 in 4a7cdf4
As a result, data with pt = '' is not deleted; instead, data with pt = null gets deleted (if it exists), leading to incorrect behavior. This also happens in |
|
@zhongyujiang Are you referring to the issue of mixing empty strings and null? |
|
This may be a tricky issue, as Hive's previous definition was to treat empty strings and null as equivalent. |
@JingsongLi Is not that the values are "mixed", but that the data with partition column values equal to an empty string cannot be deleted when using Reproduce: test("Paimon Partition Management: partition values are empty string") {
spark.sql(s"""
|CREATE TABLE T (pt STRING, data STRING)
|PARTITIONED BY (pt)
|""".stripMargin)
sql("INSERT INTO T VALUES('', 'a'), ('2', 'b')")
sql("ALTER TABLE T DROP PARTITION (pt = '')")
spark.sql("SELECT * FROM T").show(false)
//+---+----+
//|pt |data|
//+---+----+
//|2 |b |
//| |a |
//+---+----+
}
Oh, I wasn't aware of that. I noticed that Paimon currently places data with partition values of both null and empty string into the same partition—the default partition. Is Paimon intended to be consistent with Hive in this behavior? |
My answer is, if possible, it's best for us to maintain the status quo. Can this solve your business problem? |
I think that in Paimon's manifest, In other words, in Paimon's metadata, these are currently considered two different partitions. Just that data files of both types are stored under the same path Are you saying that when using HiveCatalog, these two partitions get merged into a single partition when synced to the HMS? And when doing partition deletion, would data from both null and empty string be deleted together? I’m not clear on this point.
The issue we’re encountering now is that data with partCol = '' cannot be deleted using either DELETE FROM or ALTER TABLE ... DROP PARTITION. Instead, it erroneously deletes data where partCol = null, which is not the expected behavior. So I fixed the push down of partition values to ensure that truncatePartitions works correctly in this PR. |
|
Actually, Spark's Lines 31 to 45 in 69b8159
|
|
@JingsongLi Do you have time to take another look? Thanks! |
|
I merged your first commit. |
|
Maybe can we provide a |
Part of apache#6662 (cherry picked from commit 1a1ff56)
Part of apache#6662 (cherry picked from commit 1a1ff56)
Purpose
Currently, when using Spark to delete Paimon partitions, deletion fails or incorrectly removes mismatched data if the partition value is NULL or an empty string. This issue occurs across
ALTER TABLE ... DROP PARTITION,DELETE FROM, andTRUNCATE PARTITIONALTER TABLE T DROP PARTITION and DELETE FROM
Dropping a partition with partitionField = '' erroneously deletes data where partitionField = null.
The root cause is that during deletion, Spark SQL passes partition values that are replaced by
InternalRowPartitionComputerwith the default partition value. However, the actual partition filter should use the original (real) partition values for matching, because the manifest files store the real partition values—not the default ones.This PR resolves the issue by preserving the original partition values (instead of replacing them with the default partition value) when computing partitions for data deletion:
InternalRowPartitionComputer preserveNullOrEmptyValueTRUNCATE PARTITION
Throws a NullPointerException (NPE) when the partition value is null.
Fixed by safely handling null values.
Tests
Added in the PR.
API and Format
No.
Documentation
No.