You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "emkornfield (via GitHub)" <gi...@apache.org> on 2025/03/21 06:40:57 UTC
[PR] Spec: Add details on GZIP compressed metadata files [iceberg]
emkornfield opened a new pull request, #12598:
URL: https://github.com/apache/iceberg/pull/12598
I am not sure if this is the best place to put the details (open to suggestions on creating another section elsewhere or if there are thoughts on a better section to add the sentences to).
The logic documented is reverse engineered from [TableMetadataParser.Codec](http://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/TableMetadataParser.java#L68). Which is a little bit odd, since in theory it allows for file names like `v0gz.metadata.json.foo` to correctly return as GZIP.
I assume the file name needs to end in `.metadata.json` based on [FileFormat.fromFileName](http://github.com/apache/iceberg/blob/e47e99a0dd3a500134633df640522c8c8fb56a2b/api/src/main/java/org/apache/iceberg/FileFormat.java#L60) which I assume would end up crashing if a null value was returned and from the specification mandating these file names for commits (even though this isn't strictly necessary for catalog based commits).
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2067029653
##########
format/spec.md:
##########
@@ -1761,6 +1764,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Legacy naming for GZIP compressed Metadata JSON files
+
+Some implementations have written GZIP compressed metadata JSON files with the suffix `metadata.json.gz`. The reference Java implementation will interpret files with this naming convention as GZIP files for backwards compatibility.
Review Comment:
I commented on https://github.com/apache/iceberg/pull/12598#discussion_r2067028228 I think people want to retroactively change a decision that has been canonicalized in Iceberg for quite some time.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#issuecomment-2848467074
> LGTM. I think this is the correct balance to specify the current behavior from the reference implementation while not adding additional requirements to the spec.
>
> One nit comment is to add the details of this comment ([#12598 (comment)](https://github.com/apache/iceberg/pull/12598#discussion_r2067028228)) to the PR description. This was a learning for me and i think it would be good for future readers to reference.
I added more details into the PR description, please let me know if there are any salient points you would like to see beyond what was added.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2073919400
##########
format/spec.md:
##########
@@ -1761,6 +1763,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Naming for GZIP compressed Metadata JSON files
+
+Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`.
Review Comment:
Code like https://github.com/apache/iceberg-python/blob/82e025ee73f04e6174bb1b4d853804d609ca5b67/pyiceberg/serializers.py#L35
As far as I can tell no OSS implementation always checks for GZIP magic bits
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "aihuaxu (via GitHub)" <gi...@apache.org>.
aihuaxu commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2074733827
##########
format/spec.md:
##########
@@ -1761,6 +1763,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Naming for GZIP compressed Metadata JSON files
+
+Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`.
Review Comment:
Got it. Thanks for pointing out.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2085033248
##########
format/spec.md:
##########
@@ -1761,6 +1763,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Naming for GZIP compressed Metadata JSON files
+
+Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`.
Review Comment:
The current logic appears to treat both `.gz.metadata.json` and `metadata.json.gz` as valid. Should we make this explicit in the spec to help other tools make consistent decisions? The current description is a bit ambiguous, and new tools may not know which format to prefer. I'm fine resolving this in a follow-up if that is preferred.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#issuecomment-2867823819
I'm +1 to merging this but deferring to @emkornfield
Strictly basing off the PR guidelines [here](https://iceberg.apache.org/contribute/?h=vote#merging-pull-requests), I suppose starting a vote is the right path forward since this PR is clarifying the existing behavior and is not grammar/spelling/minor formatting fixes.
"""
Changes to files under the format directory and open-api/rest-catalog* are considered specification changes. Unless already covered under an Iceberg improvement proposal, specification changes require their own vote (e.g. bug fixes or specification clarifications). The vote follows the ASF code modification model and no lazy consensus modifier. **Grammar, spelling and minor formatting fixes are exempted from this rule.** Draft specifications (new independent specifications that are going through the Iceberg improvement process) do not require a vote but authors should provide notice on the developer mailing list about substantive changes (the final draft will be subject to a vote).
"""
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2062591347
##########
format/spec.md:
##########
@@ -1473,7 +1473,10 @@ The following table describes the possible values for the some of the field with
### Table Metadata and Snapshots
-Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+
+A metadata JSON file must end in `.metadata.json`. A metadata JSON file may be compressed with [GZIP](https://datatracker.ietf.org/doc/html/rfc1952). A GZIP compressed file must end with `.gz.metadata.json`.
Review Comment:
Hi, I have two concerns about this change:
- `.gz.metadata.json` is quite uncommon and can't be read by most existing tools. Would it be better to support `.metadata.json.gz` and treat `.gz.metadata.json` as legacy for backward compatibility?
- `gzip` is becoming increasingly outdated due to its lack of support for modern CPUs. New algorithms like `zstd` are gaining popularity, so should we consider allowing users to use `.metadata.json.zst` as well?
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2070529135
##########
format/spec.md:
##########
@@ -1473,7 +1473,10 @@ The following table describes the possible values for the some of the field with
### Table Metadata and Snapshots
-Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+
+A metadata JSON file name must end in `.metadata.json`. A metadata JSON file may be compressed with [GZIP](https://datatracker.ietf.org/doc/html/rfc1952). A GZIP compressed file name must end with `.gz.metadata.json`.
Review Comment:
As Russell [points out](https://github.com/apache/iceberg/pull/12598#discussion_r2070480860) the requirement for file naming is already codified in specification under both protocols. So the first sentence here is simply a summary and gives context to naming for GZIP files names.
I don't want to generalize the naming scheme here because as I've pointed out, my goal is simply to codify existing behavior (gzip) that should have been put in the specification from the beginning. I think exact naming belongs in the specification for two reasons:
1. We are already doing so for .metadata.json files, and is required for Filesystem based naming at the very least. This can potentially be revisited for V4 once filesystem commits are fully deprecated, but guidance is needed.
2. Even though there are multiple ways to detect GZIP files, the defacto way IIUC in the two most popular implementation of Iceberg (Java and Python) appear to use filename so would break if the naming convention is broken. Python only supports this schema (it doesn't due the java backwards compatibility).
I think V4 is a good place to consider relaxing all of these constraints, as it is probably low value work to make this a blocker for the V3 spec.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#issuecomment-2856732662
> Since this is just documenting how implementations work, do we need to vote on it? It doesn't change requirements.
I think this covers that GZIP is supported which is probably a bug fix. I think last time this was discussed people wanted to ensure every [Spec change (even if trivial) had a vote](https://iceberg.apache.org/contribute/#merging-pull-requests). This might be a good test case to revisit this policy but I don't want to block on that, I'll start a vote and maybe someone else who is interested can see if the community is willing to relax this requirement
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#issuecomment-2856399957
Since this is just documenting how implementations work, do we need to vote on it? It doesn't change requirements.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2105074290
##########
format/spec.md:
##########
@@ -1761,6 +1763,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Naming for GZIP compressed Metadata JSON files
+
+Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`.
Review Comment:
> with the suffix `metadata.json.gz`
maybe "with the suffix `.metadata.json.gz`"? missing a dot?
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2070444650
##########
format/spec.md:
##########
@@ -1473,7 +1473,10 @@ The following table describes the possible values for the some of the field with
### Table Metadata and Snapshots
-Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+
+A metadata JSON file name must end in `.metadata.json`. A metadata JSON file may be compressed with [GZIP](https://datatracker.ietf.org/doc/html/rfc1952). A GZIP compressed file name must end with `.gz.metadata.json`.
Review Comment:
Why should there be any requirements for file naming, except for the FS table scheme that is deprecated? While we need to know what compression is used, it should be part of the implementation details section, not part of the spec itself. This can easily be a responsibility of the catalog or determined by file magic bytes.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2070510902
##########
format/spec.md:
##########
@@ -1761,6 +1764,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Legacy naming for GZIP compressed Metadata JSON files
+
+Some implementations have written GZIP compressed metadata JSON files with the suffix `metadata.json.gz`. The reference Java implementation will interpret files with this naming convention as GZIP files for backwards compatibility.
Review Comment:
Lets consolidate discussion on https://github.com/apache/iceberg/pull/12598#discussion_r2070444650 thread
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2070508585
##########
format/spec.md:
##########
@@ -1473,7 +1473,10 @@ The following table describes the possible values for the some of the field with
### Table Metadata and Snapshots
-Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+
+A metadata JSON file name must end in `.metadata.json`. A metadata JSON file may be compressed with [GZIP](https://datatracker.ietf.org/doc/html/rfc1952). A GZIP compressed file name must end with `.gz.metadata.json`.
Review Comment:
Lets consolidate discussion on https://github.com/apache/iceberg/pull/12598#discussion_r2070480860
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "sfc-gh-rspitzer (via GitHub)" <gi...@apache.org>.
sfc-gh-rspitzer commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2073971905
##########
format/spec.md:
##########
@@ -1761,6 +1763,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Naming for GZIP compressed Metadata JSON files
+
+Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`.
Review Comment:
Something that does also kind of bug me is that our utility class would fail for metadata.json.gz
https://github.com/apache/iceberg/blob/main/api/src/main/java/org/apache/iceberg/FileFormat.java#L60
but that's more just something that bugs me in the code and it's not actually used by anything but table importing (on my last check)
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2064063258
##########
format/spec.md:
##########
@@ -1473,7 +1473,10 @@ The following table describes the possible values for the some of the field with
### Table Metadata and Snapshots
-Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+
+A metadata JSON file must end in `.metadata.json`. A metadata JSON file may be compressed with [GZIP](https://datatracker.ietf.org/doc/html/rfc1952). A GZIP compressed file must end with `.gz.metadata.json`.
Review Comment:
+1 to treating `.metadata.json.gz` as the "default" and `.gz.metadata.json` for backwards compatibility
looking at the broader ecosystem, `.json.gz` is used more often. for example, its in the duckdb documentations
https://duckdb.org/docs/stable/data/json/loading_json.html#parameters
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2070529135
##########
format/spec.md:
##########
@@ -1473,7 +1473,10 @@ The following table describes the possible values for the some of the field with
### Table Metadata and Snapshots
-Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+
+A metadata JSON file name must end in `.metadata.json`. A metadata JSON file may be compressed with [GZIP](https://datatracker.ietf.org/doc/html/rfc1952). A GZIP compressed file name must end with `.gz.metadata.json`.
Review Comment:
As Russell [points out](https://github.com/apache/iceberg/pull/12598#discussion_r2070480860) the requirement for file naming is already codified in specification under both protocols. So the first sentence here is simply a summary and gives context to naming for GZIP files names.
I don't want to generalize the naming scheme here because as I've pointed out, my goal is simply to codify existing behavior (gzip) that should have been put in the specification from the beginning. I think exact naming belongs in the specification for two reasons:
1. We are already doing so for .metadata.json files, and is required for Filesystem based naming at the very least. This can potentially be revisited for V4 once filesystem commits are fully deprecated, but guidance is needed.
2. Even though there are multiple ways to detect GZIP files, the defacto way IIUC in the two most popular implementation of Iceberg (Java and Python) appear to use filename so would break if the naming convention is broken. Python only supports this schema (it doesn't due the java backwards compatibility).
I think V4 is a good place to consider relaxing all of these constraints.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2070548308
##########
format/spec.md:
##########
@@ -1473,7 +1473,10 @@ The following table describes the possible values for the some of the field with
### Table Metadata and Snapshots
-Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+
+A metadata JSON file name must end in `.metadata.json`. A metadata JSON file may be compressed with [GZIP](https://datatracker.ietf.org/doc/html/rfc1952). A GZIP compressed file name must end with `.gz.metadata.json`.
Review Comment:
Ah, @RussellSpitzer is right that it does imply a requirement for file names. I don't think my intent was to require a specific format -- in fact we violate it to add `gz` -- but it is precedent for specific naming requirements in the spec.
My preference for how to move forward doesn't change though. We don't need a hard requirement in the spec and we can't guarantee one given existing implementations. An implementation note is appropriate for recommendations and to document existing behavior, but we don't need to increase the requirements on writers.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#issuecomment-2858977571
:/ I think it's very close to a "no-vote necessary" change. I think when we are at this threshold it is up to the author to decide whether they think they want to fall back to a community notice. I would lean towards "no-vote" since it's just a "metadata may be gzipped" line in the spec.
The additions in Implementation notes do not require a vote imho, since they really aren't up for debate, it's just what actually is the case.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra merged PR #12598:
URL: https://github.com/apache/iceberg/pull/12598
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2070480860
##########
format/spec.md:
##########
@@ -1761,6 +1764,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Legacy naming for GZIP compressed Metadata JSON files
+
+Some implementations have written GZIP compressed metadata JSON files with the suffix `metadata.json.gz`. The reference Java implementation will interpret files with this naming convention as GZIP files for backwards compatibility.
Review Comment:
I think we are a bit to ambiguous here in the spec right now or at least have not been specific in what we are leaving up to an implementation.
For example in our description of metadata tables it definitely seems like we are mandating a postfix metadata.json. It also feels like we are specifying other parts like the version and the random bits.
https://github.com/apache/iceberg/blob/main/format/spec.md?plain=1#L1065 ?
I'd prefer we either specifically say we think file names end in xxx or we say that actual choice of file names is up to the implementation.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2070451684
##########
format/spec.md:
##########
@@ -1761,6 +1764,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Legacy naming for GZIP compressed Metadata JSON files
+
+Some implementations have written GZIP compressed metadata JSON files with the suffix `metadata.json.gz`. The reference Java implementation will interpret files with this naming convention as GZIP files for backwards compatibility.
Review Comment:
I think this is the only place where filenames should be mentioned, but this note doesn't seem sufficient to me. First, we should document the behavior expected by Hadoop/FS tables that rely on a predictable file name. Second, this is where we should document that although there are no requirements for naming, the convention is encouraged to be `<unique-name>.<compression-ext>.metadata.json`. Then note that some implementations have put the compression extension at the end, like `<unique-name>.metadata.json.gz`.
Update: looks like I got the proposed naming wrong and that the new convention is to put the compression suffix at the end?
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2076448013
##########
format/spec.md:
##########
@@ -1761,6 +1763,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Naming for GZIP compressed Metadata JSON files
+
+Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`.
Review Comment:
@RussellSpitzer what do you think we should do moving forward? Should we check magic bytes? We could use `Pattern.compile("\\.gz\\b").matcher(filename).find()`?
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2071879722
##########
format/spec.md:
##########
@@ -1473,7 +1473,10 @@ The following table describes the possible values for the some of the field with
### Table Metadata and Snapshots
-Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+
+A metadata JSON file name must end in `.metadata.json`. A metadata JSON file may be compressed with [GZIP](https://datatracker.ietf.org/doc/html/rfc1952). A GZIP compressed file name must end with `.gz.metadata.json`.
Review Comment:
Sounds good. updated.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2102878522
##########
format/spec.md:
##########
@@ -1761,6 +1763,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Naming for GZIP compressed Metadata JSON files
+
+Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`.
Review Comment:
Sorry @rdblue I forgot to get back to this! I think it's ok that our util is a little different, we can fix it though later. I think checking bytes is probably the right way to actually detect the encryption. For the utility it probably should just be looking for JSON. I don't think it makes sense to also look for metadata
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#issuecomment-2901725915
@emkornfield Did we ever finish this vote?
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2076448013
##########
format/spec.md:
##########
@@ -1761,6 +1763,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Naming for GZIP compressed Metadata JSON files
+
+Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`.
Review Comment:
@RussellSpitzer what do you think we should do moving forward? Should we check magic bytes? We could use `Pattern.compile("\\.gz\\b").matcher(filename).find()`?
It looks like implementations should be using `.gz.metadata.json` for compatibility, but we should identify the format using the regex. And when we read the metadata.json file we should probably use a stream that auto-decompresses based on magic bytes.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#issuecomment-2832775953
I'll start a vote
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#issuecomment-2835959921
This looks fine to me, do we have any other outstanding issues or can we vote on this?
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2070508585
##########
format/spec.md:
##########
@@ -1473,7 +1473,10 @@ The following table describes the possible values for the some of the field with
### Table Metadata and Snapshots
-Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+
+A metadata JSON file name must end in `.metadata.json`. A metadata JSON file may be compressed with [GZIP](https://datatracker.ietf.org/doc/html/rfc1952). A GZIP compressed file name must end with `.gz.metadata.json`.
Review Comment:
Lets consolidate discussion on https://github.com/apache/iceberg/pull/12598#discussion_r2070480860
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2062984701
##########
format/spec.md:
##########
@@ -1473,7 +1473,10 @@ The following table describes the possible values for the some of the field with
### Table Metadata and Snapshots
-Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+
+A metadata JSON file must end in `.metadata.json`. A metadata JSON file may be compressed with [GZIP](https://datatracker.ietf.org/doc/html/rfc1952). A GZIP compressed file must end with `.gz.metadata.json`.
Review Comment:
Thank you @emkornfield, I have copied my comments to ML.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2067028228
##########
format/spec.md:
##########
@@ -1473,7 +1473,10 @@ The following table describes the possible values for the some of the field with
### Table Metadata and Snapshots
-Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+
+A metadata JSON file must end in `.metadata.json`. A metadata JSON file may be compressed with [GZIP](https://datatracker.ietf.org/doc/html/rfc1952). A GZIP compressed file must end with `.gz.metadata.json`.
Review Comment:
Thanks for the feedback, as noted above this is documenting existing behavior. If we want to change it I think this is a larger discussion. Regardless of what the ecosystem uses to to denote gzip compressed metadata, the fact of the matter is Iceberg chose a different convention. This is documenting what Java does, and what [Python](https://github.com/apache/iceberg-python/blob/82e025ee73f04e6174bb1b4d853804d609ca5b67/pyiceberg/serializers.py#L35) expects.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2062731626
##########
format/spec.md:
##########
@@ -1473,7 +1473,10 @@ The following table describes the possible values for the some of the field with
### Table Metadata and Snapshots
-Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+
+A metadata JSON file must end in `.metadata.json`. A metadata JSON file may be compressed with [GZIP](https://datatracker.ietf.org/doc/html/rfc1952). A GZIP compressed file must end with `.gz.metadata.json`.
Review Comment:
@Xuanwo both of these are good points, I don't have historical context on why these conventions where chosen but this is documenting existing behavior, so I think changes should be discussed on the mailing list as a follow-up.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2071941003
##########
format/spec.md:
##########
@@ -1761,6 +1764,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Naming for GZIP compressed Metadata JSON files
Review Comment:
note for reviewers, this is part of the `Appendix F: Implementation Notes`
The section above references https://iceberg.apache.org/spec/#assignment-of-snapshot-ids-and-current-snapshot-id
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2070451684
##########
format/spec.md:
##########
@@ -1761,6 +1764,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Legacy naming for GZIP compressed Metadata JSON files
+
+Some implementations have written GZIP compressed metadata JSON files with the suffix `metadata.json.gz`. The reference Java implementation will interpret files with this naming convention as GZIP files for backwards compatibility.
Review Comment:
I think this is the only place where filenames should be mentioned, but this note doesn't seem sufficient to me. First, we should document the behavior expected by Hadoop/FS tables that rely on a predictable file name. Second, this is where we should document that although there are no requirements for naming, the convention is encouraged to be `<unique-name>.<compression-ext>.metadata.json`. Then note that some implementations have put the compression extension at the end, like `<unique-name>.metadata.json.gz`.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2070548308
##########
format/spec.md:
##########
@@ -1473,7 +1473,10 @@ The following table describes the possible values for the some of the field with
### Table Metadata and Snapshots
-Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+Table metadata is serialized as a JSON object according to the following table. Snapshots are not serialized separately. Instead, they are stored in the table metadata JSON.
+
+A metadata JSON file name must end in `.metadata.json`. A metadata JSON file may be compressed with [GZIP](https://datatracker.ietf.org/doc/html/rfc1952). A GZIP compressed file name must end with `.gz.metadata.json`.
Review Comment:
Ah, @RussellSpitzer is right that it does imply a requirement for file names. I don't think my intent was to require a specific format -- in fact we violate it to add `gz` -- but it is precedent for specific naming requirements in the spec.
My preference for how to move forward doesn't change though. We don't need a hard requirement here and we can't guarantee one given existing implementations. An implementation note is appropriate for recommendations and to document existing behavior, but we don't need to increase the requirements on writers.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2073972434
##########
format/spec.md:
##########
@@ -1761,6 +1763,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Naming for GZIP compressed Metadata JSON files
+
+Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`.
Review Comment:
Wrong account, but thats me^
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#issuecomment-2817406506
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#issuecomment-2903372715
> @emkornfield Did we ever finish this vote?
Sorry for the delay, just closed the vote as passing
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2064219250
##########
format/spec.md:
##########
@@ -1761,6 +1764,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Legacy naming for GZIP compressed Metadata JSON files
+
+Some implementations have written GZIP compressed metadata JSON files with the suffix `metadata.json.gz`. The reference Java implementation will interpret files with this naming convention as GZIP files for backwards compatibility.
Review Comment:
Isn't this backwards of what is suggested in the mailing list discussion? `.gz.metadata.json` is the legacy format and `.metadata.json.gz` is the proposed representation?
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "aihuaxu (via GitHub)" <gi...@apache.org>.
aihuaxu commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2073903346
##########
format/spec.md:
##########
@@ -1761,6 +1763,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Naming for GZIP compressed Metadata JSON files
+
+Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`.
Review Comment:
What would make the the implementation not able to read `metadata.json.gz` and require to be `.gz.metadata.json`?
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2073921351
##########
format/spec.md:
##########
@@ -1761,6 +1763,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Naming for GZIP compressed Metadata JSON files
+
+Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`.
Review Comment:
Corresponding java code: https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/TableMetadataParser.java#L68
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#issuecomment-2742504894
CC @danielcweeks I think we had an offline conversation about this a while ago.
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2008093312
##########
format/spec.md:
##########
@@ -1761,6 +1764,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Legacy naming for GZIP compressed Metadata JSON files
+
+Some implementations have written GZIP compressed metadata JSON files with the suffix `metadata.json.gz`. The reference Java implementation will interpret files with this naming convention as GZIP files.
Review Comment:
reference
https://github.com/apache/iceberg/blob/74051b44f933bf3f33bb9ab9b04ed987e16b40dc/core/src/main/java/org/apache/iceberg/TableMetadataParser.java#L68-L74
##########
format/spec.md:
##########
@@ -1761,6 +1764,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Legacy naming for GZIP compressed Metadata JSON files
+
+Some implementations have written GZIP compressed metadata JSON files with the suffix `metadata.json.gz`. The reference Java implementation will interpret files with this naming convention as GZIP files.
Review Comment:
I think we should also mention that `.metadata.json.gz` is supported for backwards compatibility
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2008110463
##########
format/spec.md:
##########
@@ -1761,6 +1764,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Legacy naming for GZIP compressed Metadata JSON files
+
+Some implementations have written GZIP compressed metadata JSON files with the suffix `metadata.json.gz`. The reference Java implementation will interpret files with this naming convention as GZIP files.
Review Comment:
it does imply that! I think its better to be explicit
nit:
```suggestion
Some implementations have written GZIP compressed metadata JSON files with the suffix `metadata.json.gz`. The reference Java implementation will interpret files with this naming convention as GZIP files for backwards compatibility.
```
--
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: Add details on GZIP compressed metadata files [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #12598:
URL: https://github.com/apache/iceberg/pull/12598#discussion_r2008106428
##########
format/spec.md:
##########
@@ -1761,6 +1764,10 @@ The reference Java implementation uses a type 4 uuid and XORs the 4 most signifi
Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to omitted or `null`. This has never been formalized in the spec, but for compatibility, other implementations can accept `-1` as `null`. Java will no longer write `-1` and will use `null` for "no current snapshot" for all tables with a version greater than or equal to V3.
+### Legacy naming for GZIP compressed Metadata JSON files
+
+Some implementations have written GZIP compressed metadata JSON files with the suffix `metadata.json.gz`. The reference Java implementation will interpret files with this naming convention as GZIP files.
Review Comment:
I thought that was implied here? Could you suggest some language here?
--
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