Fix bugs in dropping partition columns by jiang95-dev · Pull Request #281 · linkedin/iceberg

jiang95-dev · 2026-10-19T02:43:01Z

Problem statement

Running RTAS to drop a partition column will cause "Cannot find source column for partition field". This is because when iceberg constructs SerializableTable, it binds all historical partition specs with the current schema, and throws an error when the spec contains missing columns. RTAS should be an atomic operation that supports dropping any columns without needing to drop the partition spec first.

Cannot find source column for partition field: 1000: part: identity(3)
org.apache.iceberg.exceptions.ValidationException: Cannot find source column for partition field: 1000: part: identity(3)
at app//org.apache.iceberg.exceptions.ValidationException.check(ValidationException.java:49)
at app//org.apache.iceberg.PartitionSpec.checkCompatibility(PartitionSpec.java:588)
at app//org.apache.iceberg.PartitionSpec$Builder.build(PartitionSpec.java:569)
at app//org.apache.iceberg.UnboundPartitionSpec.bind(UnboundPartitionSpec.java:46)
at app//org.apache.iceberg.PartitionSpecParser.fromJson(PartitionSpecParser.java:71)
at app//org.apache.iceberg.PartitionSpecParser.lambda$fromJson$1(PartitionSpecParser.java:88)
at app//org.apache.iceberg.util.JsonUtil.parse(JsonUtil.java:98)
at app//org.apache.iceberg.PartitionSpecParser.lambda$fromJson$2(PartitionSpecParser.java:88)
at app//com.github.benmanes.caffeine.cache.BoundedLocalCache.lambda$doComputeIfAbsent$14(BoundedLocalCache.java:2406)
at java.base@17.0.5/java.util.concurrent.ConcurrentHashMap.compute(ConcurrentHashMap.java:1916)
at app//com.github.benmanes.caffeine.cache.BoundedLocalCache.doComputeIfAbsent(BoundedLocalCache.java:2404)
at app//com.github.benmanes.caffeine.cache.BoundedLocalCache.computeIfAbsent(BoundedLocalCache.java:2387)
at app//com.github.benmanes.caffeine.cache.LocalCache.computeIfAbsent(LocalCache.java:108)
at app//com.github.benmanes.caffeine.cache.LocalManualCache.get(LocalManualCache.java:62)
at app//org.apache.iceberg.PartitionSpecParser.fromJson(PartitionSpecParser.java:86)
at app//org.apache.iceberg.SerializableTable.lambda$specs$1(SerializableTable.java:196)
at java.base@17.0.5/java.util.HashMap.forEach(HashMap.java:1421)
at app//org.apache.iceberg.SerializableTable.specs(SerializableTable.java:194)
at app//org.apache.iceberg.spark.source.SparkWrite$WriterFactory.createWriter(SparkWrite.java:683)
at app//org.apache.iceberg.spark.source.SparkWrite$WriterFactory.createWriter(SparkWrite.java:677)
at app//org.apache.spark.sql.execution.datasources.v2.WritingSparkTask.run(WriteToDataSourceV2Exec.scala:436)

Note, in the current branch, even in the regular path, dropping the column after dropping the partition field will fail.

Out of scope

In the normal drop partition column path, it also failed but the failure is an expected behavior. TableMetadata checks if the current spec is compatible with the current schema. It is not a bug and this PR is not fixing this.

Cannot find source column for partition field: 1000: id: identity(1)
org.apache.iceberg.exceptions.ValidationException: Cannot find source column for partition field: 1000: id: identity(1)
	at org.apache.iceberg.exceptions.ValidationException.check(ValidationException.java:49)
	at org.apache.iceberg.PartitionSpec.checkCompatibility(PartitionSpec.java:587)
	at org.apache.iceberg.TableMetadata$Builder.build(TableMetadata.java:1348)
	at org.apache.iceberg.TableMetadata.updateSchema(TableMetadata.java:547)
	at org.apache.iceberg.SchemaUpdate.commit(SchemaUpdate.java:441)
	at org.apache.iceberg.spark.SparkCatalog.commitChanges(SparkCatalog.java:538)
	at org.apache.iceberg.spark.SparkCatalog.alterTable(SparkCatalog.java:257)
	at org.apache.iceberg.spark.SparkCatalog.alterTable(SparkCatalog.java:100)
	at org.apache.iceberg.spark.SparkSessionCatalog.alterTable(SparkSessionCatalog.java:234)

Changes

This bug has already been fixed in the upstream. This PR backports 7 related upstream PRs, fixes the merge conflicts, and adds RTAS related tests to verify the fix. Note this PR is not a backport PR but borrows the idea from the upstream fix.

The main idea is to use UnknownType when a partition source column is missing from the current schema, and let the transform determine the result type. Identity and truncate resolve to UnknownType, while transforms with fixed result types, such as bucket, retain their known types.

For time-travel reads, partition specs are rebound to the selected snapshot’s schema so predicates on historical columns can be resolved correctly.

Backport commits explanation

Commit / Upstream PR	Purpose	Why it is included
`99f1c0b03` — apache/iceberg#12012	Introduces `UnknownType`.	Iceberg 1.5.2 lacks the type used by the upstream solution to represent missing partition source types. Only the relevant type definition, parsing, size estimation, and serialization support are retained.
`d7bd0c974` — apache/iceberg#12455	Adds Avro support for unknown types and timestamp nanoseconds.	Partition types are used in Avro manifest schemas, so `UnknownType` needs an Avro representation. Only the unknown/null conversion and optionality changes are retained; timestamp-nanosecond support is excluded.
`4f350f540` — apache/iceberg#11868	Allows historical specs to reference source columns missing from the current schema.	This is the primary fix for the `SerializableTable.specs()` failure. It permits missing source columns during JSON parsing and excludes inactive fields from the current table’s unified partition type.
`a6868412c` — apache/iceberg#13301	Resolves partition specs against the selected snapshot’s schema during time travel.	Not required for the RTAS write itself, but required for historical predicates such as `WHERE dropped_column = ...`.
`61a97c3c7` — apache/iceberg#14356	Handles unknown types in comparisons, conversions, manifest writing, and delete processing.	Completes downstream handling so dropping a source column does not simply move the failure to a subsequent operation, particularly DELETE.
`fdc803d9b` — apache/iceberg#16523	Uses snapshot-bound specs throughout distributed planning.	Complements #13301: both the driver’s evaluator cache and executor-side manifest readers must use the historical schema. Our time-travel tests still failed with #13301 alone and passed after this fix.
`71256790d` — apache/iceberg#17262	Preserves fixed transform result types when the source type is missing.	Not essential to the identity-partition RTAS reproduction, but avoids degrading fixed-result transforms such as bucket to unknown when their result type remains integer.

What the final adaptation commit retains

36c1cb5ea — Adapt upstream partition fixes to Iceberg 1.5.2 and add RTAS regressions

The final commit resolves cross-version conflicts and leaves the following core changes in the resulting branch:

Minimal UnknownType support: type definition, type ID, primitive-name parsing, size estimation, and serialization coverage.
Historical spec parsing: PartitionSpecParser allows missing source fields when reconstructing specs. Normal strict spec construction remains available; missing-field tolerance does not make an invalid current partition spec valid.
Transform-aware partition types: missing source types become UnknownType; identity/truncate propagate unknown, while fixed-result transforms retain their known result types.
Current partition metadata: fields whose source columns no longer exist are excluded from the current table’s unified partition type.
Avro and delete-path compatibility: unknown/null schema conversion, optionality, comparators, byte-buffer conversions, null writing, and conservative manifest-summary handling.
Time-travel schema binding: local and distributed scan paths use specs rebound to the selected snapshot’s schema, including executor-side manifest readers. The distributed changes are adapted to Spark 3.4 as well as 3.5.
Regression coverage: RTAS source-column removal, subsequent reads/appends, historical reads and filters in LOCAL/DISTRIBUTED planning modes, ordinary partition-field/source-column removal followed by DELETE, and core type/schema-evolution checks.

The final tree does not introduce variant support, timestamp-nanosecond support, Spark 4.x modules, or the custom historical-type recovery implementation. Those unrelated newer-version changes appeared temporarily in intermediate conflict resolutions and were removed by the final adaptation commit.

Testing Done

Added a test in TestCreateTableAsSelect to test that RTAS can drop a partition column, and still be able to read, and do filtering in time travel.

* API: Add `UnknownType` * Make the CI happy * Thanks Eduard! * Add tests for the other transforms as well * Thanks Honah! * Thanks Russell! * Thanks Eduard! (cherry picked from commit a04220a) Backport note: conflicting files temporarily use upstream versions; 1.5.2 compatibility is restored in the final adaptation commit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

(cherry picked from commit 3c8366a) Backport note: conflicting files temporarily use upstream versions; 1.5.2 compatibility is restored in the final adaptation commit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

…urrent-schema (#11868) Co-authored-by: Nynke Gaikema <nynkegaikema@MacBook-Air-van-Nynke.local> (cherry picked from commit 8134815) Backport note: conflicting files temporarily use upstream versions; 1.5.2 compatibility is restored in the final adaptation commit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

…13301) (cherry picked from commit 867835e) Backport note: conflicting files temporarily use upstream versions; 1.5.2 compatibility is restored in the final adaptation commit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

(cherry picked from commit c22bac8) Backport note: conflicting files temporarily use upstream versions; 1.5.2 compatibility is restored in the final adaptation commit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

…ing mode (#16523) (cherry picked from commit 4057f59dbee6878b3986f7cc39b1f30a59ee9ee3) Backport note: conflicting files temporarily use upstream versions; 1.5.2 compatibility is restored in the final adaptation commit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

…wn (#17262) PartitionSpec.partitionType() degrades a partition field's type to `unknown` whenever the source column has been dropped from the schema, even for transforms like bucket and the date/time transforms whose result type does not depend on the source type. Substitute unknown as the source type instead and let the transform derive its result type, matching the existing behavior of javaClasses(). Identity and truncate still fall back to unknown because their result type is the source type. Follow-up to the discussion in apache/iceberg#16958 (comment) (cherry picked from commit e973bc0fcd05514466fd0e201c34ece2b396c4fd) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Resolve the cross-version conflicts deferred by the preceding upstream commits. Keep the UnknownType and Avro support needed by the partition fixes without importing unrelated timestamp-nanosecond/variant features, newer reader APIs, Spark 4.x modules, or migrated test infrastructure. Preserve the 1.5.2 implementation and local default-value behavior. Adapt the upstream time-travel planner fixes to Spark 3.4 as well as 3.5. Add RTAS/drop-source read and append tests, historical predicates in LOCAL and DISTRIBUTED planning modes, ordinary ALTER/drop-source/DELETE coverage, and core partition/schema-evolution regressions. Upstream PRs: apache/iceberg#12012, #12455, #11868, #13301, #14356, #16523, #17262. Intermediate commits intentionally retain incoming versions of conflicting files and need not compile individually; this commit restores the validated final tree. Validation: API suite, targeted core partition/parser/Avro/schema-evolution tests, Spark 3.4 compilation, Spark 3.5 CTAS/RTAS suite and ALTER/drop-source/DELETE tests, plus formatting checks. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

mkuchenbecker

This backport seems overly broad without further explanation and the changes to make it work are 30% of the total lines shanged.

Its pulling in variant and other code. it seems the issue is more localized; is there a smaller change that could be made? the core issue seems to be loading the old column data but I may be underestimateing

mkuchenbecker · 2026-10-19T16:14:18Z

In the normal drop partition column path, it also failed but the failure is an expected behavior. TableMetadata checks if the current spec is compatible with the current schema. It is not a bug and this PR is not fixing this.

This seems like a bug to give a stacktrace when a deterministic error is sufficent.

jiang95-dev · 2026-10-20T20:17:20Z

This backport seems overly broad without further explanation and the changes to make it work are 30% of the total lines shanged.

Its pulling in variant and other code. it seems the issue is more localized; is there a smaller change that could be made? the core issue seems to be loading the old column data but I may be underestimateing

I'm not sure if it is a github rendering bug. The PR did not add variant or timestampNano support. The final commit cut all the unrelated changes. Can you take a look again?

Also I've added in the PR description the explanations to the 7 commits, and also what are the core changes in the last commit.

jiang95-dev · 2026-10-20T20:26:22Z

In the normal drop partition column path, it also failed but the failure is an expected behavior. TableMetadata checks if the current spec is compatible with the current schema. It is not a bug and this PR is not fixing this.

This seems like a bug to give a stacktrace when a deterministic error is sufficent.

Are you saying we should change the way we surface this disallowed operation? Like throwing "spec mismatch the current schema"?

mkuchenbecker · 2026-10-23T01:07:32Z

-        return sanitizeTimestamp(DateTimeUtil.nanosToMicros((long) value / 1000), now);
      case STRING:
        return sanitizeString((CharSequence) value, now, today);
-      case UNKNOWN:


Unkonwn should be handled here, no?

mkuchenbecker · 2026-10-23T01:08:34Z


  private static String sanitizeSimpleString(CharSequence value) {
    // hash the value and return the hash as hex
-    return String.format(Locale.ROOT, "(hash-%08x)", HASH_FUNC.apply(value));


why did se change the format string from the backport?

mkuchenbecker · 2026-10-23T01:08:57Z

-        variantValue.writeTo(variantBuffer, variantMetadata.sizeInBytes());
-        return variantBuffer;
      case UNKNOWN:
-        // underlying type not known


why is the comment on unknown removed?

mkuchenbecker · 2026-10-23T01:09:08Z

-      case VARIANT:
-        return Variant.from(tmp);
      case UNKNOWN:
-        // underlying type not known


same comment here.

mkuchenbecker · 2026-10-23T01:09:30Z

    @Override
    public String toString() {
-      return String.format(Locale.ROOT, "fixed[%d]", length);
+      return String.format("fixed[%d]", length);


unclear why this was changed from backport

mkuchenbecker · 2026-10-23T01:10:09Z

+      private boolean isOptional = true;
+      private String name = null;


why can't these be final

mkuchenbecker · 2026-10-23T01:12:12Z

    return visit(schema, new FindTypeVisitor(predicate));
  }

-  public static Type find(Type type, Predicate<Type> predicate) {


Why are we removing the visitor?

mkuchenbecker · 2026-10-23T01:14:25Z

-      Types.NestedField sourceColumn = findSourceColumn(sourceName);
-      String columnName = schema.findColumnName(sourceColumn.fieldId());
-      return day(sourceColumn, columnName + "_day");
+      return day(sourceName, sourceName + "_day");


I don't understand why we are removing the source column resolution. Can we explain?

mkuchenbecker · 2026-10-23T01:17:14Z

    assertThat(identity.toHumanString(timestamptz, ts.value()))
        .as("Should produce timestamp with time zone adjusted to UTC")
-        .isEqualTo("2017-12-01T18:12:55.038194+00:00");
+        .isEqualTo("2017-12-01T18:12:55.038194Z");


why is this different than OSS? TZ precision change?

Fokko and others added 8 commits October 18, 2026 19:28

github-actions Bot added SPARK CORE API labels Oct 19, 2026

jiang95-dev marked this pull request as ready for review October 19, 2026 02:54

mkuchenbecker reviewed Oct 19, 2026

View reviewed changes

mkuchenbecker reviewed Oct 23, 2026

View reviewed changes

fix a minor bug in the reconcilation commit

4826ceb

jiang95-dev changed the title ~~Backport to fix bugs in dropping partition columns~~ Fix bugs in dropping partition columns Oct 23, 2026

		private boolean isOptional = true;
		private String name = null;

Conversation

jiang95-dev commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem statement

Out of scope

Changes

Backport commits explanation

What the final adaptation commit retains

Testing Done

Uh oh!

mkuchenbecker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkuchenbecker commented Oct 19, 2026

Uh oh!

jiang95-dev commented Oct 20, 2026

Uh oh!

jiang95-dev commented Oct 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkuchenbecker Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jiang95-dev commented Oct 19, 2026 •

edited

Loading

mkuchenbecker Oct 23, 2026 •

edited

Loading