rdblue commented on code in PR #9661:
URL: https://github.com/apache/iceberg/pull/9661#discussion_r1486949549


##########
format/spec.md:
##########
@@ -1130,14 +1142,10 @@ Each partition field in the fields list is stored as an 
object. See the table fo
 |**`hour`**|`JSON string: "hour"`|`"hour"`|
 |**`Partition Field`** [1,2]|`JSON object: {`<br />&nbsp;&nbsp;`"source-id": 
<id int>,`<br />&nbsp;&nbsp;`"field-id": <field id int>,`<br 
/>&nbsp;&nbsp;`"name": <name string>,`<br />&nbsp;&nbsp;`"transform": 
<transform JSON>`<br />`}`|`{`<br />&nbsp;&nbsp;`"source-id": 1,`<br 
/>&nbsp;&nbsp;`"field-id": 1000,`<br />&nbsp;&nbsp;`"name": "id_bucket",`<br 
/>&nbsp;&nbsp;`"transform": "bucket[16]"`<br />`}`|
 
-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.
-
-The `field-id` property was added for each partition field in v2. In v1, the 
reference implementation assigned field ids sequentially in each spec starting 
at 1,000. See Partition Evolution for more details.
-
 Notes:
-
-1. For partition fields with a transform with a single argument, the ID of the 
source field is set on `source-id`, and `source-ids` is omitted.
-2. For partition fields with a transform of multiple arguments, the IDs of the 
source fields are set on `source-ids`. To preserve backward compatibility, 
`source-id` is set to -1.
+1. 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.
+2. The `field-id` property was added for each partition field in v2. In v1, 
the reference implementation assigned field ids sequentially in each spec 
starting at 1,000. See Partition Evolution for more details.
+3. For tables of version < V3, the ID of the source field of each partition 
field is set in `source-id`. For tables of version >= V3, the ID(s) of the 
source field(s) is set on `source-ids`, and `source-id` is omitted. See 
Appendix E for more details.

Review Comment:
   Rather than moving the first two clarifications into notes, I'd probably 
remove notes and just add a paragraph on source-id vs source-ids.
   
   For the paragraph on source IDs, I think it should be something like this:
   
   > Transforms that accept multiple arguments specify source field IDs using 
`source-ids` instead of `source-id`.
   >
   > Writers producing v1 and v2 metadata should continue to produce the 
`source-id` field for older readers that require it by setting it to the first 
ID from the `source-ids` list. Older versions of the reference implementation 
can read tables with unknown transforms and will ignore multi-arg transforms, 
but other implementations may break if they encounter unknown transform names.
   >
   > Writers producing v3 metadata should omit the `source-id` field because v3 
readers are required to support multi-arg transforms and accept the 
`source-ids` field.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to