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


##########
format/spec.md:
##########
@@ -1453,13 +1457,15 @@ Each sort field in the fields list is stored as an 
object with the following pro
 
 | V1       | V2       | V3       | Field            | JSON representation | 
Example     |
 
|----------|----------|----------|------------------|---------------------|-------------|
-| required | required | required | **`transform`**  | `JSON string`       | 
`bucket[4]` |
-| required | required | omitted  | **`source-id`**  | `JSON int`          | 1  
         |
+| required | required | required¹ | **`transform`**  | `JSON string`       | 
`bucket[4]` |
+| required | required | required¹ | **`source-id`**  | `JSON int`          | 1 
          |
 |          |          | required | **`source-ids`** | `JSON list of ints` | 
`[1,2]`     |
 | required | required | required | **`direction`**  | `JSON string`       | 
`asc`       |
 | required | required | required | **`null-order`** | `JSON string`       | 
`nulls-last`|
 
-In v3 metadata, writers must use only `source-ids` because v3 requires reader 
support for multi-arg transforms.
+Notes:
+
+1. For sort fields with a transform with a single argument, the ID of the 
source field is set on `source-id`, and `source-ids` is omitted.

Review Comment:
   In other cases in the spec, we use `[1]` or similar to reference notes. I'd 
prefer keeping it that way. I also think it would make sense to have a "notes" 
column and put this in the table. That's probably easier.
   
   Also, I don't think that this should be required. If it can be omitted, then 
it must be optional. With the current changes, I think this would make both of 
them optional and state that at least one must be present. Since that's a 
little strange, we may want to change it so that `source-id` is optional and 
`source-ids` is required. In that case, `source-ids` would always have the full 
set of IDs and `source-id` would only be present if there is only one source 
field 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-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to