-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[Bug] Fix Iceberg metadata unreadable by Snowflake - add Avro schema/partition-spec metadata to manifest files #9497
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,6 +72,7 @@ | |
| import org.apache.paimon.types.RowType; | ||
| import org.apache.paimon.utils.DataFilePathFactories; | ||
| import org.apache.paimon.utils.FileStorePathFactory; | ||
| import org.apache.paimon.utils.JsonSerdeUtil; | ||
| import org.apache.paimon.utils.ManifestReadThreadPool; | ||
| import org.apache.paimon.utils.Pair; | ||
| import org.apache.paimon.utils.Preconditions; | ||
|
|
@@ -189,8 +190,6 @@ public IcebergCommitCallback(FileStoreTable table, String commitUser) { | |
| metadataCommitterFactory == null ? null : metadataCommitterFactory.create(table); | ||
|
|
||
| this.fileStorePathFactory = table.store().pathFactory(); | ||
| this.manifestFile = IcebergManifestFile.create(table, pathFactory); | ||
| this.manifestList = IcebergManifestList.create(table, pathFactory); | ||
|
|
||
| this.formatVersion = | ||
| table.coreOptions().toConfiguration().get(IcebergOptions.FORMAT_VERSION); | ||
|
|
@@ -200,6 +199,21 @@ public IcebergCommitCallback(FileStoreTable table, String commitUser) { | |
| "Unsupported iceberg format version! Only version 2 or version 3 is valid, but current version is ", | ||
| formatVersion); | ||
|
|
||
| // Compute Iceberg schema and partition spec for Avro manifest metadata. | ||
| // Snowflake and other Iceberg readers require these in the manifest file header. | ||
| IcebergSchema icebergSchema = IcebergSchema.create(table.schema()); | ||
| List<IcebergPartitionField> partitionFields = | ||
| getPartitionFields(table.schema().partitionKeys(), icebergSchema); | ||
| IcebergPartitionSpec partitionSpec = new IcebergPartitionSpec(partitionFields); | ||
| Map<String, String> avroMetadata = new HashMap<>(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Iceberg v2/v3 manifests require a |
||
| avroMetadata.put("schema", icebergSchema.toJson()); | ||
| avroMetadata.put("partition-spec", JsonSerdeUtil.toJson(partitionSpec)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Iceberg's |
||
| avroMetadata.put("partition-spec-id", String.valueOf(IcebergPartitionSpec.SPEC_ID)); | ||
| avroMetadata.put("format-version", String.valueOf(formatVersion)); | ||
| this.manifestFile = IcebergManifestFile.create(table, pathFactory, avroMetadata); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] This only adds metadata to manifests created after the upgrade. |
||
|
|
||
| this.manifestList = IcebergManifestList.create(table, pathFactory); | ||
|
|
||
| this.indexFileHandler = table.store().newIndexFileHandler(); | ||
| this.needAddDvToIceberg = needAddDvToIceberg(); | ||
| } | ||
|
|
||
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.
[P1] This still writes field ID 0 into the Iceberg schema.
Schema.Builderassigns the first Paimon column ID 0, andIcebergDataField(DataField)preserves it. I verified that a manifest produced by this PR has"id" : 0in itsschemaheader, which is the incompatibility reported in #9012. Adding the header therefore does not demonstrate that Snowflake can read the table. Please introduce a consistent positive-ID mapping everywhere Iceberg IDs are emitted (schema, partition source IDs, metrics maps, and any physical schema IDs), and cover it with a compatibility regression test.