You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "stevenzwu (via GitHub)" <gi...@apache.org> on 2025/08/21 23:48:58 UTC
[PR] Spec: clarify the partition-spec metadata for Avro manifest file [iceberg]
stevenzwu opened a new pull request, #13895:
URL: https://github.com/apache/iceberg/pull/13895
In the field, we have seen some writer produce non-conform metadata in Manifest Avro file.
```
partition-spec: {"spec-id":0,"fields":[]}
```
Probably the spec wording caused the mis-interpretation that it is the JSON serialization of the whole partition spec. But the spec and the Java reference implementation meant only the partition fields array of the partition spec, as the spec id was encoded as a separate metadata field in the Avro file.
```
.meta("partition-spec", PartitionSpecParser.toJsonFields(spec))
.meta("partition-spec-id", String.valueOf(spec.specId()))
```
This PR is to clarify this Avro metadata field.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: clarify the partition-spec metadata for Avro manifest file [iceberg]
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao merged PR #13895:
URL: https://github.com/apache/iceberg/pull/13895
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: clarify the partition-spec metadata for Avro manifest file [iceberg]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #13895:
URL: https://github.com/apache/iceberg/pull/13895#discussion_r2298781163
##########
format/spec.md:
##########
@@ -609,14 +609,14 @@ A manifest stores files for a single partition spec. When a table’s partition
A manifest file must store the partition spec and other metadata as properties in the Avro file's key-value metadata:
-| v1 | v2 | Key | Value |
-|------------|------------|---------------------|------------------------------------------------------------------------------|
-| _required_ | _required_ | `schema` | JSON representation of the table schema at the time the manifest was written |
-| _optional_ | _required_ | `schema-id` | ID of the schema used to write the manifest as a string |
-| _required_ | _required_ | `partition-spec` | JSON fields representation of the partition spec used to write the manifest |
-| _optional_ | _required_ | `partition-spec-id` | ID of the partition spec used to write the manifest as a string |
-| _optional_ | _required_ | `format-version` | Table format version number of the manifest as a string |
-| | _required_ | `content` | Type of content files tracked by the manifest: "data" or "deletes" |
+| v1 | v2 | Key | Value |
+|------------|------------|---------------------|---------------------------------------------------------------------------------------------------------------------------------------------|
+| _required_ | _required_ | `schema` | JSON representation of the table schema at the time the manifest was written |
+| _optional_ | _required_ | `schema-id` | ID of the schema used to write the manifest as a string |
+| _required_ | _required_ | `partition-spec` | JSON representation of only the partition fields array of the partition spec used to write the manifest. See [Appeneix C](#partition-specs) |
Review Comment:
```suggestion
| _required_ | _required_ | `partition-spec` | JSON representation of only the partition fields array of the partition spec used to write the manifest. See [Appendix C](#partition-specs) |
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: clarify the partition-spec metadata for Avro manifest file [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #13895:
URL: https://github.com/apache/iceberg/pull/13895#discussion_r2298646328
##########
format/spec.md:
##########
@@ -609,14 +609,14 @@ A manifest stores files for a single partition spec. When a table’s partition
A manifest file must store the partition spec and other metadata as properties in the Avro file's key-value metadata:
-| v1 | v2 | Key | Value |
-|------------|------------|---------------------|------------------------------------------------------------------------------|
-| _required_ | _required_ | `schema` | JSON representation of the table schema at the time the manifest was written |
-| _optional_ | _required_ | `schema-id` | ID of the schema used to write the manifest as a string |
-| _required_ | _required_ | `partition-spec` | JSON fields representation of the partition spec used to write the manifest |
-| _optional_ | _required_ | `partition-spec-id` | ID of the partition spec used to write the manifest as a string |
-| _optional_ | _required_ | `format-version` | Table format version number of the manifest as a string |
-| | _required_ | `content` | Type of content files tracked by the manifest: "data" or "deletes" |
+| v1 | v2 | Key | Value |
+|------------|------------|---------------------|----------------------------------------------------------------------------------------------------|
+| _required_ | _required_ | `schema` | JSON representation of the table schema at the time the manifest was written |
+| _optional_ | _required_ | `schema-id` | ID of the schema used to write the manifest as a string |
+| _required_ | _required_ | `partition-spec` | JSON representation of the partition fields array of the partition spec used to write the manifest |
Review Comment:
manifest reader in iceberg-core only use the Avro metadata as backup, as manifest file entry (from manifest list) contains the `partition_spec_id`.
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/ManifestReader.java#L126-L130
Good call-out for the unbounded transform part. maybe I can add a link to the spec
https://iceberg.apache.org/spec/#partition-specs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: clarify the partition-spec metadata for Avro manifest file [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #13895:
URL: https://github.com/apache/iceberg/pull/13895#issuecomment-3221855611
> I've also seen this in the past. I don't see any harm in making this more explicit 👍
Could you share who was generating the files? We had a user report this but never figured out the origin?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: clarify the partition-spec metadata for Avro manifest file [iceberg]
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on PR #13895:
URL: https://github.com/apache/iceberg/pull/13895#issuecomment-3224380993
Merged. Thanks @stevenzwu for the PR! Thanks @singhpk234 @Fokko @RussellSpitzer for the review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: clarify the partition-spec metadata for Avro manifest file [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #13895:
URL: https://github.com/apache/iceberg/pull/13895#discussion_r2298369716
##########
format/spec.md:
##########
@@ -609,14 +609,14 @@ A manifest stores files for a single partition spec. When a table’s partition
A manifest file must store the partition spec and other metadata as properties in the Avro file's key-value metadata:
-| v1 | v2 | Key | Value |
-|------------|------------|---------------------|------------------------------------------------------------------------------|
-| _required_ | _required_ | `schema` | JSON representation of the table schema at the time the manifest was written |
-| _optional_ | _required_ | `schema-id` | ID of the schema used to write the manifest as a string |
-| _required_ | _required_ | `partition-spec` | JSON fields representation of the partition spec used to write the manifest |
-| _optional_ | _required_ | `partition-spec-id` | ID of the partition spec used to write the manifest as a string |
-| _optional_ | _required_ | `format-version` | Table format version number of the manifest as a string |
-| | _required_ | `content` | Type of content files tracked by the manifest: "data" or "deletes" |
+| v1 | v2 | Key | Value |
+|------------|------------|---------------------|----------------------------------------------------------------------------------------------------|
+| _required_ | _required_ | `schema` | JSON representation of the table schema at the time the manifest was written |
+| _optional_ | _required_ | `schema-id` | ID of the schema used to write the manifest as a string |
+| _required_ | _required_ | `partition-spec` | JSON representation of the partition fields array of the partition spec used to write the manifest |
Review Comment:
[doubt-1] how is the spec-id inferred when the partition-spec-id is empty (since its optional in v1)
[discuss] is it worth calling out that the ParitionSpec if first converted to unbounded partition spec whats put here is the unbounded version of partition field (where transform is just string) rather than Transform object ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org