Document field-id requirements for partition specs #1141

jun-he · 2020-06-29T06:41:06Z

rdblue · 2020-07-05T20:42:59Z

site/docs/spec.md


+#### Partition Field ID handling
+
+A partition field id is an integer (starting at 1000) used to identify a partition field.


The requirement isn't to start at 1000, it is that the IDs should not conflict with the IDs required for the other fields in data files. I think starting at 1000 is a good recommendation to avoid that.

rdblue · 2020-07-05T20:46:11Z

site/docs/spec.md

+
+A partition field id is an integer (starting at 1000) used to identify a partition field.
+
+Since iceberg release 0.8.0, partition fields are present in every partition field of partition specs in a table metadata.


The spec shouldn't reference Iceberg Java releases. The requirements here are for different versions of tables:

For v1 tables, partition field metadata should include a field-id for each partition field, but this is not required.

For v2 tables, partition field metadata must include a field-id for each partition field.

Then for compatibility within v1, this should recommend not removing fields but replacing them with always null. We also need to add that transform to the spec.

rdblue · 2020-07-05T20:47:49Z

site/docs/spec.md

+
+Additionally, in table metadata format v2, partition fields are required to have unique field IDs to support partition spec evolution.
+
+For tables without partition field IDs, iceberg will generate an auto-increment unique field id starting at 1000 for every partition field.


This should state that old versions of the "reference implementation will generate..."

I think this should also give some context at the top about what breaks if compatibility recommendations are not followed for v1.

jun-he · 2020-07-11T23:27:59Z

Thanks @rdblue and update the PR.

jun-he · 2020-07-29T04:32:44Z

@rdblue can you let me know your comments? Thanks.

jun-he · 2020-12-19T21:46:13Z

@rdblue should we merge this?

rdblue · 2021-01-05T00:34:18Z

site/docs/spec.md

 | **`month`**       | Extract a date or timestamp month, as months from 1970-01-01 | `date`, `timestamp(tz)`                                                                                   | `int`       |
 | **`day`**         | Extract a date or timestamp day, as days from 1970-01-01     | `date`, `timestamp(tz)`                                                                                   | `date`      |
 | **`hour`**        | Extract a timestamp hour, as hours from 1970-01-01 00:00:00  | `timestamp(tz)`                                                                                           | `int`       |
+| **`alwaysNull`**  | Always produces `null` (the void transform)                  | Any                                                                                                       | `void`      |


The transform name for the spec is void, not alwaysNull. That's just the name we use in Java where void is a reserved keyword.

Also, the result type is the source type, but values are always null.

Thanks for the comments. Fixed.

rdblue · 2021-01-05T00:40:14Z

site/docs/spec.md


 In some cases partition specs are stored using only the field list instead of the object format that includes the spec ID, like the deprecated `partition-spec` field in table metadata. The object format should be used unless otherwise noted in this spec.

+#### Partition Field ID Handling


This is in the appendix that covers JSON serialization for partition specs. It is good to document the field-id name here and note that it is required in v2 and optional in v1, but a lot of this content should be noted in the Partitioning section of the spec.

rdblue · 2021-01-05T00:43:00Z

site/docs/spec.md

+
+A partition field ID is an integer used to identify a partition field.
+These IDs should not conflict with the IDs required for the other fields in data files.
+To avoid that, iceberg assigns partition field IDs starting at 1000.


The requirement is that IDs are unique and do not conflict with field IDs for columns in manifest files. The reference implementation assigns partition field IDs starting at 1,000.

rdblue · 2021-01-05T00:44:30Z

site/docs/spec.md

+* For v1 tables, partition field metadata should include a field id for each partition field, but this is not required.
+* For v2 tables, partition field metadata must include a field id for each partition field. Partition field IDs are unique across partition specs to support the partition spec evolution for a given table.
+
+To remove partition fields from the partition spec in an existing v1 table, it is recommended not removing fields but replacing their transforms with `alwaysNull`.


This should be in a secondary section about working with v1 tables. That should explain the problem: that field IDs were reused, which causes problems in metadata tables. To avoid the problem, don't reorder or delete partition fields. Instead, add new fields at the end and replace fields with void. Also note that this is not needed fro v2.

jun-he · 2021-01-05T08:21:11Z

@rdblue thanks for the comments. I updated the document accordingly. Can you take another look? Thanks.

rdblue · 2021-01-05T21:19:09Z

site/docs/spec.md

 | **`month`**       | Extract a date or timestamp month, as months from 1970-01-01 | `date`, `timestamp(tz)`                                                                                   | `int`       |
 | **`day`**         | Extract a date or timestamp day, as days from 1970-01-01     | `date`, `timestamp(tz)`                                                                                   | `date`      |
 | **`hour`**        | Extract a timestamp hour, as hours from 1970-01-01 00:00:00  | `timestamp(tz)`                                                                                           | `int`       |
+| **`void`**        | Always produces `null` (the void transform)                  | Any                                                                                                       | `null`      |


null isn't really a type. I understand wanting to make it clear what is happening, but I think "Source type" like identity is probably more accurate. It is clear that the function result is always null, though.

rdblue · 2021-01-05T21:20:08Z

site/docs/spec.md


+#### Partition Field ID handling
+
+A partition field ID is an integer used to identify a partition field. 


How about "... identify a partition field in Iceberg manifest files".

rdblue · 2021-01-05T21:22:10Z

site/docs/spec.md

+
+About compatibility between v1 and v2 tables:
+
+* For backward compatibility, if field ids are missing in a table metadata, iceberg will sequentially generate ids for each field starting at 1000 based on its position in the list of fields.


This should not refer to what "iceberg" does. There are things required by this spec and there are implementations. Both are "iceberg".

rdblue · 2021-01-05T21:23:31Z

site/docs/spec.md

+#### Partition Field ID
+
+A partition field ID is an integer used to identify a partition field.
+They are unique and do not conflict with field IDs for columns in manifest files. 


This should be "must not" because it is a requirement, not a guarantee.

rdblue · 2021-01-05T21:31:18Z

site/docs/spec.md

+* For backward compatibility, if field ids are missing in a table metadata, iceberg will sequentially generate ids for each field starting at 1000 based on its position in the list of fields.
+* For forward compatibility, if field ids are not supported but present in the metadata, old versions of the reference implementation will ignore those field ids and then regenerate an auto-increment field id starting at 1000 for every partition field.
+
+While working with a v1 table, field IDs might be reused if removing partition fields from its partition spec. 


I think what this is trying to say is that old versions of the reference implementation did not keep track of field IDs. When creating a manifest, each field of the partition spec was assigned an ID starting at 1,000, and there were no guarantees about ID reuse across files. But as long as the spec was not evolved, IDs were consistent.

That has a few implications:

Older writers may erase partition field IDs when writing to a v1 table. This does not happen to v2 tables because writers will fail to read or write a v2 table.

Metadata tables need consistent field IDs across manifest files. To do this for v1 tables evolve the spec according to the recommendations (don't delete, replace with void; only add to the end; renames are okay).

jun-he · 2021-01-06T07:31:30Z

@rdblue thanks for the review. Improved the doc accordingly.

rdblue · 2021-01-07T01:28:04Z

site/docs/spec.md

+* For backward compatibility, if field ids are missing in a table metadata, the reference implementation will sequentially generate ids for each field starting at `1000` based on its position in the list of fields.
+* For forward compatibility, if field ids are not supported but present in the metadata, old versions of the reference implementation will ignore those field ids and then regenerate an auto-increment field id starting at 1000 for every partition field.
+
+While working with a v1 table, old versions of the reference implementation did not keep track of field IDs. 


The v1 spec did not require consistent field IDs, so they were assigned when creating each manifest file.

@rdblue thanks for the comment. Updated it accordingly.

yyanyy

Thank you for continuing the efforts of updating this! Some minor comments on wording, maybe it's just me not being super familiar with this area

yyanyy · 2021-01-13T00:33:24Z

site/docs/spec.md

+But as long as the partition spec will not be evolved, IDs will be consistent.
+
+This has a few implications:
+* Older writers may erase partition field IDs when writing to a v1 table. This does not happen to v2 tables because writers will fail to read or write a v2 table.


Nit: "... will fail to read or write a v2 table when partition field ID doesn't exist"?

Thanks for the comment. Updated it for clarification. It failed because of version mismatch. So old parsers will fail to read or write a v2 table metadata.

yyanyy · 2021-01-13T00:42:11Z

site/docs/spec.md

 | **`month`**       | Extract a date or timestamp month, as months from 1970-01-01 | `date`, `timestamp(tz)`                                                                                   | `int`       |
 | **`day`**         | Extract a date or timestamp day, as days from 1970-01-01     | `date`, `timestamp(tz)`                                                                                   | `date`      |
 | **`hour`**        | Extract a timestamp hour, as hours from 1970-01-01 00:00:00  | `timestamp(tz)`                                                                                           | `int`       |
+| **`void`**        | Always produces `null` (the void transform)                  | Any                                                                                                       | Source type |


Nit: might be good to have an explanation here on why this is needed (not likely needed in v2, mostly needed in v1 when dropping a partition field without changing the auto-assigned partition field ids?)

This section is to describe all available partition transforms instead of explaining their use cases.
I added a new section Void Transform Details with some details.

yyanyy · 2021-01-13T00:45:10Z

site/docs/spec.md

+This has a few implications:
+* Older writers may erase partition field IDs when writing to a v1 table. This does not happen to v2 tables because writers will fail to read or write a v2 table.
+* Metadata tables need consistent field IDs across manifest files. To achieve it, for v1 tables, please evolve the partition spec according to the recommendations, 
+i.e. don't reorder or delete partition fields; replace fields with with `void` transform; add new fields to the end. Note that renames are OK and also note that this does not apply for v2 tables.


Nit: might want to clarify why it doesn't apply for v2 tables; it seems that it's not needed because v2 table is capable of auto identifying the latest partition field Id across manifests to always assign a new one to the new field during a partition spec update table commit? If that's the case, it might be good to mention it here

Updated it with the reason (i.e. v2 table tracks and persists the last assigned partition field id).

yyanyy · 2021-01-13T00:52:35Z

site/docs/spec.md

+
+This has a few implications:
+* Older writers may erase partition field IDs when writing to a v1 table. This does not happen to v2 tables because writers will fail to read or write a v2 table.
+* Metadata tables need consistent field IDs across manifest files. To achieve it, for v1 tables, please evolve the partition spec according to the recommendations, 


Nit: I wonder if we want to expand a little bit on "Metadata tables need consistent field IDs across manifest files", which I think is to ensure correctness when looking up metadata tables, the same ID always refers to the same partition field/no ID reuses, and this mapping between ID and partition field is immutable even after partition spec evolution.

Yep, that is correct. New section Void Transform Details now include those details.

…ting

rdblue linked an issue Jul 5, 2020 that may be closed by this pull request

Spec: Document field-id requirements for partition specs #1025

Closed

rdblue reviewed Jul 5, 2020

View reviewed changes

rdblue added this to the Format Version 2 milestone Jul 7, 2020

jun-he requested a review from rdblue July 23, 2020 04:38

rdblue reviewed Jan 5, 2021

View reviewed changes

jun-he force-pushed the jun/add-field-id-doc branch from 365c7ec to 7ce502c Compare January 5, 2021 08:20

github-actions bot added the docs label Jan 5, 2021

rdblue reviewed Jan 5, 2021

View reviewed changes

jun-he requested a review from rdblue January 6, 2021 07:30

rdblue reviewed Jan 7, 2021

View reviewed changes

jun-he requested a review from rdblue January 8, 2021 06:41

yyanyy reviewed Jan 13, 2021

View reviewed changes

jun-he added 4 commits January 30, 2021 12:19

Document field-id requirements for partition specs and fix doc format…

ab4e207

…ting

address the comments

95b4c19

improve the doc according to the review comments

abbcf3e

address the comment

591dfad

improve the doc and add a new Void Transform Details section

832a5c8

jun-he force-pushed the jun/add-field-id-doc branch from 39a3eff to 832a5c8 Compare January 30, 2021 20:21

rdblue mentioned this pull request May 31, 2021

Update spec for v2 changes #2654

Merged

rdblue closed this in #2654 Jun 16, 2021


		#### Partition Field ID handling

		A partition field id is an integer (starting at 1000) used to identify a partition field.


		A partition field id is an integer (starting at 1000) used to identify a partition field.

		Since iceberg release 0.8.0, partition fields are present in every partition field of partition specs in a table metadata.


		Additionally, in table metadata format v2, partition fields are required to have unique field IDs to support partition spec evolution.

		For tables without partition field IDs, iceberg will generate an auto-increment unique field id starting at 1000 for every partition field.


		In some cases partition specs are stored using only the field list instead of the object format that includes the spec ID, like the deprecated `partition-spec` field in table metadata. The object format should be used unless otherwise noted in this spec.

		#### Partition Field ID Handling


		#### Partition Field ID handling

		A partition field ID is an integer used to identify a partition field.


		About compatibility between v1 and v2 tables:

		* For backward compatibility, if field ids are missing in a table metadata, iceberg will sequentially generate ids for each field starting at 1000 based on its position in the list of fields.

Document field-id requirements for partition specs #1141

Document field-id requirements for partition specs #1141

Uh oh!

Conversation

jun-he commented Jun 29, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jun-he commented Jul 11, 2020

Uh oh!

jun-he commented Jul 29, 2020

Uh oh!

jun-he commented Dec 19, 2020

Uh oh!

rdblue Jan 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jun-he commented Jan 5, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jun-he commented Jan 6, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yyanyy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rdblue Jan 5, 2021 •

edited

Loading