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 514387a484 Spark 3.5, 4.0: Fix rewrite failure on tables created with
a sort order (#18510)
514387a484 is described below
commit 514387a4845f5a41492bb15599f4cdcb08c0b7b9
Author: raashed <[email protected]>
AuthorDate: Thu Oct 29 04:46:01 2026 +0530
Spark 3.5, 4.0: Fix rewrite failure on tables created with a sort order
(#18510)
---
.../org/apache/iceberg/spark/SparkWriteConf.java | 5 +++-
.../apache/iceberg/spark/TestSparkWriteConf.java | 25 +++++++++++++++++
.../spark/actions/TestRewriteDataFilesAction.java | 31 ++++++++++++++++++++++
.../org/apache/iceberg/spark/SparkWriteConf.java | 5 +++-
.../apache/iceberg/spark/TestSparkWriteConf.java | 26 ++++++++++++++++++
.../spark/actions/TestRewriteDataFilesAction.java | 31 ++++++++++++++++++++++
6 files changed, 121 insertions(+), 2 deletions(-)
diff --git
a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/SparkWriteConf.java
b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/SparkWriteConf.java
index 2f98aec4e7..0698e6327f 100644
---
a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/SparkWriteConf.java
+++
b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/SparkWriteConf.java
@@ -168,8 +168,11 @@ public class SparkWriteConf {
confParser.intConf().option(SparkWriteOptions.OUTPUT_SORT_ORDER_ID).parseOptional();
if (explicitId != null) {
+ // order id 0 is reserved for unsorted order, which is a valid output
for any table
+ // whether or not the table contains a sort order with that id
Preconditions.checkArgument(
- table.sortOrders().containsKey(explicitId),
+ explicitId == SortOrder.unsorted().orderId()
+ || table.sortOrders().containsKey(explicitId),
"Cannot use output sort order id %s because the table does not
contain a sort order with that id",
explicitId);
return explicitId;
diff --git
a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java
b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java
index 89daf195ca..7d24d29be3 100644
---
a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java
+++
b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java
@@ -54,6 +54,8 @@ import java.util.Map;
import org.apache.iceberg.DistributionMode;
import org.apache.iceberg.FileFormat;
import org.apache.iceberg.ParameterizedTestExtension;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.SortOrder;
import org.apache.iceberg.Table;
import org.apache.iceberg.TableProperties;
import org.apache.iceberg.UpdateProperties;
@@ -598,6 +600,29 @@ public class TestSparkWriteConf extends
TestBaseWithCatalog {
assertThat(writeConfNoOption.outputSortOrderId(SparkWriteRequirements.EMPTY)).isEqualTo(0);
}
+ @TestTemplate
+ void sortOrderWriteConfWithUnsortedId() {
+ Schema schema = validationCatalog.loadTable(tableIdent).schema();
+ validationCatalog.dropTable(tableIdent);
+ Table table =
+ validationCatalog
+ .buildTable(tableIdent, schema)
+ .withSortOrder(SortOrder.builderFor(schema).asc("id").build())
+ .create();
+
assertThat(table.sortOrders()).doesNotContainKey(SortOrder.unsorted().orderId());
+
+ SparkWriteConf writeConf =
+ new SparkWriteConf(
+ spark,
+ table,
+ ImmutableMap.of(
+ SparkWriteOptions.OUTPUT_SORT_ORDER_ID,
+ String.valueOf(SortOrder.unsorted().orderId())));
+
+ assertThat(writeConf.outputSortOrderId(SparkWriteRequirements.EMPTY))
+ .isEqualTo(SortOrder.unsorted().orderId());
+ }
+
private void testWriteProperties(List<Map<String, String>> propertiesSuite) {
withSQLConf(
propertiesSuite.get(0),
diff --git
a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java
b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java
index 5bc3293e91..b893484c1d 100644
---
a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java
+++
b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java
@@ -1685,6 +1685,37 @@ public class TestRewriteDataFilesAction extends TestBase
{
dataFilesShouldHaveSortOrderIdMatching(table, c3SortOrder);
}
+ @TestTemplate
+ void zOrderWhenTableDeclaredSortOrderAtCreation() throws IOException {
+ Table table =
+ TABLES.create(
+ SCHEMA,
+ PartitionSpec.unpartitioned(),
+ SortOrder.builderFor(SCHEMA).asc("c2").build(),
+ ImmutableMap.of(TableProperties.FORMAT_VERSION,
String.valueOf(formatVersion)),
+ tableLocation);
+
assertThat(table.sortOrders()).doesNotContainKey(SortOrder.unsorted().orderId());
+
+ writeRecords(4, 100);
+ table.refresh();
+
+ List<Object[]> originalData = currentData();
+
+ RewriteDataFiles.Result result =
+ basicRewrite(table)
+ .zOrder("c2", "c3")
+ .option(SizeBasedFileRewritePlanner.REWRITE_ALL, "true")
+ .execute();
+
+ assertThat(result.rewriteResults()).as("Should have 1
fileGroup").hasSize(1);
+
+ table.refresh();
+
+ assertEquals("We shouldn't have changed the data", originalData,
currentData());
+ dataFilesShouldHaveSortOrderIdMatching(table, SortOrder.unsorted());
+ shouldHaveACleanCache(table);
+ }
+
@TestTemplate
public void testAutoSortShuffleOutput() throws IOException {
Table table = createTable(20, LARGE_SCALE);
diff --git
a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/SparkWriteConf.java
b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/SparkWriteConf.java
index fc98bfeaed..105039167a 100644
---
a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/SparkWriteConf.java
+++
b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/SparkWriteConf.java
@@ -171,8 +171,11 @@ public class SparkWriteConf {
confParser.intConf().option(SparkWriteOptions.OUTPUT_SORT_ORDER_ID).parseOptional();
if (explicitId != null) {
+ // order id 0 is reserved for unsorted order, which is a valid output
for any table
+ // whether or not the table contains a sort order with that id
Preconditions.checkArgument(
- table.sortOrders().containsKey(explicitId),
+ explicitId == SortOrder.unsorted().orderId()
+ || table.sortOrders().containsKey(explicitId),
"Cannot use output sort order id %s because the table does not
contain a sort order with that id",
explicitId);
return explicitId;
diff --git
a/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java
b/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java
index c5cfbe62b1..03cd807dee 100644
---
a/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java
+++
b/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java
@@ -55,6 +55,8 @@ import java.util.Map;
import org.apache.iceberg.DistributionMode;
import org.apache.iceberg.FileFormat;
import org.apache.iceberg.ParameterizedTestExtension;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.SortOrder;
import org.apache.iceberg.Table;
import org.apache.iceberg.TableProperties;
import org.apache.iceberg.UpdateProperties;
@@ -654,6 +656,30 @@ public class TestSparkWriteConf extends
TestBaseWithCatalog {
assertThat(writeConfNoOption.outputSortOrderId(SparkWriteRequirements.EMPTY)).isEqualTo(0);
}
+ @TestTemplate
+ void sortOrderWriteConfWithUnsortedId() {
+ Schema schema = validationCatalog.loadTable(tableIdent).schema();
+ validationCatalog.dropTable(tableIdent);
+ Table table =
+ validationCatalog
+ .buildTable(tableIdent, schema)
+ .withSortOrder(SortOrder.builderFor(schema).asc("id").build())
+ .create();
+
assertThat(table.sortOrders()).doesNotContainKey(SortOrder.unsorted().orderId());
+
+ SparkWriteConf writeConf =
+ new SparkWriteConf(
+ spark,
+ table,
+ new CaseInsensitiveStringMap(
+ ImmutableMap.of(
+ SparkWriteOptions.OUTPUT_SORT_ORDER_ID,
+ String.valueOf(SortOrder.unsorted().orderId()))));
+
+ assertThat(writeConf.outputSortOrderId(SparkWriteRequirements.EMPTY))
+ .isEqualTo(SortOrder.unsorted().orderId());
+ }
+
private void testWriteProperties(List<Map<String, String>> propertiesSuite) {
withSQLConf(
propertiesSuite.get(0),
diff --git
a/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java
b/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java
index bc8d922f62..2be79e8295 100644
---
a/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java
+++
b/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java
@@ -1687,6 +1687,37 @@ public class TestRewriteDataFilesAction extends TestBase
{
dataFilesShouldHaveSortOrderIdMatching(table, c3SortOrder);
}
+ @TestTemplate
+ void zOrderWhenTableDeclaredSortOrderAtCreation() throws IOException {
+ Table table =
+ TABLES.create(
+ SCHEMA,
+ PartitionSpec.unpartitioned(),
+ SortOrder.builderFor(SCHEMA).asc("c2").build(),
+ ImmutableMap.of(TableProperties.FORMAT_VERSION,
String.valueOf(formatVersion)),
+ tableLocation);
+
assertThat(table.sortOrders()).doesNotContainKey(SortOrder.unsorted().orderId());
+
+ writeRecords(4, 100);
+ table.refresh();
+
+ List<Object[]> originalData = currentData();
+
+ RewriteDataFiles.Result result =
+ basicRewrite(table)
+ .zOrder("c2", "c3")
+ .option(SizeBasedFileRewritePlanner.REWRITE_ALL, "true")
+ .execute();
+
+ assertThat(result.rewriteResults()).as("Should have 1
fileGroup").hasSize(1);
+
+ table.refresh();
+
+ assertEquals("We shouldn't have changed the data", originalData,
currentData());
+ dataFilesShouldHaveSortOrderIdMatching(table, SortOrder.unsorted());
+ shouldHaveACleanCache(table);
+ }
+
@TestTemplate
public void testAutoSortShuffleOutput() throws IOException {
Table table = createTable(20, LARGE_SCALE);
