This is an automated email from the ASF dual-hosted git repository.
szehon-ho pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/main by this push:
new 4d5811f2c7 Core, Spark: Fix PartitionSet toString flattening partition
tuples (#18354)
4d5811f2c7 is described below
commit 4d5811f2c701b15be4379686b17144503ed1e685
Author: Hongyue/Steve Zhang <[email protected]>
AuthorDate: Wed Oct 7 19:55:11 2026 -0700
Core, Spark: Fix PartitionSet toString flattening partition tuples (#18354)
---
.../java/org/apache/iceberg/util/PartitionSet.java | 39 +++++++++++++---------
.../org/apache/iceberg/TestReplacePartitions.java | 16 ++++-----
.../org/apache/iceberg/util/TestPartitionSet.java | 32 ++++++++++++++++++
.../spark/extensions/TestConflictValidation.java | 8 ++---
.../spark/extensions/TestConflictValidation.java | 8 ++---
.../spark/extensions/TestConflictValidation.java | 8 ++---
.../spark/extensions/TestConflictValidation.java | 8 ++---
7 files changed, 80 insertions(+), 39 deletions(-)
diff --git a/core/src/main/java/org/apache/iceberg/util/PartitionSet.java
b/core/src/main/java/org/apache/iceberg/util/PartitionSet.java
index 184f38b324..ed327e0fbd 100644
--- a/core/src/main/java/org/apache/iceberg/util/PartitionSet.java
+++ b/core/src/main/java/org/apache/iceberg/util/PartitionSet.java
@@ -21,9 +21,12 @@ package org.apache.iceberg.util;
import java.util.AbstractSet;
import java.util.Collection;
import java.util.Iterator;
+import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.StringJoiner;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
import org.apache.iceberg.PartitionSpec;
import org.apache.iceberg.StructLike;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
@@ -191,22 +194,28 @@ public class PartitionSet extends
AbstractSet<Pair<Integer, StructLike>> {
@Override
public String toString() {
- StringJoiner result = new StringJoiner(", ", "[", "]");
- for (Map.Entry<Integer, Set<StructLike>> e : partitionSetById.entrySet()) {
- StringJoiner partitionDataJoiner = new StringJoiner(", ");
- Types.StructType structType = partitionTypeById.get(e.getKey());
- for (StructLike s : e.getValue()) {
- for (int i = 0; i < structType.fields().size(); i++) {
- StringBuilder partitionStringBuilder = new StringBuilder();
- partitionStringBuilder.append(structType.fields().get(i).name());
- partitionStringBuilder.append("=");
- partitionStringBuilder.append(s.get(i, Object.class));
- partitionDataJoiner.add(partitionStringBuilder.toString());
- }
- }
- result.add(partitionDataJoiner.toString());
+ return partitionSetById.entrySet().stream()
+ .flatMap(this::toStrings)
+ .collect(Collectors.joining(", ", "[", "]"));
+ }
+
+ private Stream<String> toStrings(Map.Entry<Integer, Set<StructLike>> entry) {
+ Types.StructType partitionType = partitionTypeById.get(entry.getKey());
+ return entry.getValue().stream().map(struct -> toString(partitionType,
struct));
+ }
+
+ private static String toString(Types.StructType partitionType, StructLike
struct) {
+ if (struct == null) {
+ return "null";
+ }
+
+ List<Types.NestedField> fields = partitionType.fields();
+ StringJoiner joiner = new StringJoiner(", ", "{", "}");
+ for (int pos = 0; pos < fields.size(); pos += 1) {
+ joiner.add(fields.get(pos).name() + "=" + struct.get(pos, Object.class));
}
- return result.toString();
+
+ return joiner.toString();
}
@Override
diff --git a/core/src/test/java/org/apache/iceberg/TestReplacePartitions.java
b/core/src/test/java/org/apache/iceberg/TestReplacePartitions.java
index a004cd0b01..1335fe33cd 100644
--- a/core/src/test/java/org/apache/iceberg/TestReplacePartitions.java
+++ b/core/src/test/java/org/apache/iceberg/TestReplacePartitions.java
@@ -399,7 +399,7 @@ public class TestReplacePartitions extends TestBase {
.isInstanceOf(ValidationException.class)
.hasMessage(
"Found conflicting files that can contain records matching
partitions "
- + "[data_bucket=0, data_bucket=1]: [/path/to/data-a.parquet]");
+ + "[{data_bucket=0}, {data_bucket=1}]:
[/path/to/data-a.parquet]");
}
@TestTemplate
@@ -421,7 +421,7 @@ public class TestReplacePartitions extends TestBase {
.isInstanceOf(ValidationException.class)
.hasMessage(
"Found conflicting files that can contain records matching
partitions "
- + "[data_bucket=null, data_bucket=1]:
[/path/to/data-null-partition.parquet]");
+ + "[{data_bucket=null}, {data_bucket=1}]:
[/path/to/data-null-partition.parquet]");
}
@TestTemplate
@@ -444,7 +444,7 @@ public class TestReplacePartitions extends TestBase {
.isInstanceOf(ValidationException.class)
.hasMessage(
"Found conflicting files that can contain records matching
partitions "
- + "[id_null=null, data_bucket=1, id_null=null, data_bucket=0]:
"
+ + "[{id_null=null, data_bucket=1}, {id_null=null,
data_bucket=0}]: "
+ "[/path/to/data-a-void-partition.parquet]");
}
@@ -473,7 +473,7 @@ public class TestReplacePartitions extends TestBase {
.isInstanceOf(ValidationException.class)
.hasMessage(
"Found conflicting files that can contain records matching
partitions "
- + "[data_bucket=0, data_bucket=1]: [/path/to/data-a.parquet]");
+ + "[{data_bucket=0}, {data_bucket=1}]:
[/path/to/data-a.parquet]");
}
@TestTemplate
@@ -566,7 +566,7 @@ public class TestReplacePartitions extends TestBase {
.isInstanceOf(ValidationException.class)
.hasMessage(
"Found conflicting files that can contain records matching
partitions "
- + "[data_bucket=0, data_bucket=1]: [/path/to/data-b.parquet]");
+ + "[{data_bucket=0}, {data_bucket=1}]:
[/path/to/data-b.parquet]");
}
@TestTemplate
@@ -666,7 +666,7 @@ public class TestReplacePartitions extends TestBase {
.isInstanceOf(ValidationException.class)
.hasMessage(
"Found new conflicting delete files that can apply to records
matching "
- + "[data_bucket=0]: [/path/to/data-a-deletes.parquet]");
+ + "[{data_bucket=0}]: [/path/to/data-a-deletes.parquet]");
}
@TestTemplate
@@ -702,7 +702,7 @@ public class TestReplacePartitions extends TestBase {
.isInstanceOf(ValidationException.class)
.hasMessage(
"Found new conflicting delete files that can apply to records
matching "
- + "[data_bucket=0]: ["
+ + "[{data_bucket=0}]: ["
+ dv2.location()
+ ", "
+ dv1.location()
@@ -818,7 +818,7 @@ public class TestReplacePartitions extends TestBase {
.isInstanceOf(ValidationException.class)
.hasMessage(
"Found conflicting deleted files that can apply to records
matching "
- + "[data_bucket=0]: [/path/to/data-a.parquet]");
+ + "[{data_bucket=0}]: [/path/to/data-a.parquet]");
}
@TestTemplate
diff --git a/core/src/test/java/org/apache/iceberg/util/TestPartitionSet.java
b/core/src/test/java/org/apache/iceberg/util/TestPartitionSet.java
index 533e590423..164306e672 100644
--- a/core/src/test/java/org/apache/iceberg/util/TestPartitionSet.java
+++ b/core/src/test/java/org/apache/iceberg/util/TestPartitionSet.java
@@ -80,4 +80,36 @@ public class TestPartitionSet {
assertThat(set.remove(BY_DATA_CATEGORY_BUCKET_SPEC.specId(), Row.of("a",
1))).isTrue();
assertThat(set).isEmpty();
}
+
+ @Test
+ public void testToString() {
+ PartitionSet set = PartitionSet.create(SPECS);
+
+ assertThat(set.toString()).isEqualTo("[]");
+
+ set.add(BY_DATA_SPEC.specId(), Row.of("aaa"));
+ assertThat(set.toString()).isEqualTo("[{data=aaa}]");
+
+ set.add(BY_DATA_SPEC.specId(), CustomRow.of("bbb"));
+ set.add(BY_DATA_CATEGORY_BUCKET_SPEC.specId(), Row.of("ccc", 2));
+ assertThat(set.toString())
+ .as("each partition is rendered as its own group and ordering is not
guaranteed in a set")
+ .contains("{data=aaa}", "{data=bbb}", "{data=ccc, category_bucket=2}");
+ }
+
+ @Test
+ public void testToStringWithUnpartitionedSpec() {
+ PartitionSet set = PartitionSet.create(SPECS);
+ set.add(UNPARTITIONED_SPEC.specId(), Row.of());
+
+ assertThat(set.toString()).isEqualTo("[{}]");
+ }
+
+ @Test
+ public void testToStringWithNullPartition() {
+ PartitionSet set = PartitionSet.create(SPECS);
+ set.add(BY_DATA_SPEC.specId(), null);
+
+ assertThat(set.toString()).isEqualTo("[null]");
+ }
}
diff --git
a/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
b/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
index b5ba7eec1b..ce7fd18894 100644
---
a/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
+++
b/spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
@@ -264,7 +264,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found conflicting files that can contain records matching
partitions [id=1]");
+ "Found conflicting files that can contain records matching
partitions [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
@@ -299,7 +299,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found new conflicting delete files that can apply to records
matching [id=1]");
+ "Found new conflicting delete files that can apply to records
matching [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
@@ -333,7 +333,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found conflicting deleted files that can apply to records
matching [id=1]");
+ "Found conflicting deleted files that can apply to records
matching [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
@@ -380,7 +380,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found conflicting files that can contain records matching
partitions [id=1]");
+ "Found conflicting files that can contain records matching
partitions [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
diff --git
a/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
b/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
index b5ba7eec1b..ce7fd18894 100644
---
a/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
+++
b/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
@@ -264,7 +264,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found conflicting files that can contain records matching
partitions [id=1]");
+ "Found conflicting files that can contain records matching
partitions [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
@@ -299,7 +299,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found new conflicting delete files that can apply to records
matching [id=1]");
+ "Found new conflicting delete files that can apply to records
matching [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
@@ -333,7 +333,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found conflicting deleted files that can apply to records
matching [id=1]");
+ "Found conflicting deleted files that can apply to records
matching [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
@@ -380,7 +380,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found conflicting files that can contain records matching
partitions [id=1]");
+ "Found conflicting files that can contain records matching
partitions [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
diff --git
a/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
b/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
index b5ba7eec1b..ce7fd18894 100644
---
a/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
+++
b/spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
@@ -264,7 +264,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found conflicting files that can contain records matching
partitions [id=1]");
+ "Found conflicting files that can contain records matching
partitions [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
@@ -299,7 +299,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found new conflicting delete files that can apply to records
matching [id=1]");
+ "Found new conflicting delete files that can apply to records
matching [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
@@ -333,7 +333,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found conflicting deleted files that can apply to records
matching [id=1]");
+ "Found conflicting deleted files that can apply to records
matching [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
@@ -380,7 +380,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found conflicting files that can contain records matching
partitions [id=1]");
+ "Found conflicting files that can contain records matching
partitions [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
diff --git
a/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
index b5ba7eec1b..ce7fd18894 100644
---
a/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
+++
b/spark/v4.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java
@@ -264,7 +264,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found conflicting files that can contain records matching
partitions [id=1]");
+ "Found conflicting files that can contain records matching
partitions [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
@@ -299,7 +299,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found new conflicting delete files that can apply to records
matching [id=1]");
+ "Found new conflicting delete files that can apply to records
matching [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
@@ -333,7 +333,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found conflicting deleted files that can apply to records
matching [id=1]");
+ "Found conflicting deleted files that can apply to records
matching [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
@@ -380,7 +380,7 @@ public class TestConflictValidation extends
ExtensionsTestBase {
.overwritePartitions())
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith(
- "Found conflicting files that can contain records matching
partitions [id=1]");
+ "Found conflicting files that can contain records matching
partitions [{id=1}]");
// Validating from latest snapshot should succeed
table.refresh();
