kevinjqliu commented on code in PR #16446:
URL: https://github.com/apache/iceberg/pull/16446#discussion_r3289951142


##########
format/spec.md:
##########
@@ -514,14 +514,18 @@ Partition field IDs must be reused if an existing 
partition spec contains an equ
 | **`truncate[W]`** | Value truncated to width `W` (see below)                 
    | `int`, `long`, `decimal`, `string`, `binary`                              
                                | Source type |
 | **`year`**        | Extract a date or timestamp year, as years from 1970     
    | `date`, `timestamp`, `timestamptz`, `timestamp_ns`, `timestamptz_ns`      
                                | `int`       |
 | **`month`**       | Extract a date or timestamp month, as months from 
1970-01-01 | `date`, `timestamp`, `timestamptz`, `timestamp_ns`, 
`timestamptz_ns`                                      | `int`       |
-| **`day`**         | Extract a date or timestamp day, as days from 1970-01-01 
    | `date`, `timestamp`, `timestamptz`, `timestamp_ns`, `timestamptz_ns`      
                                | `int`       |
+| **`day`**         | Extract a date or timestamp day, as days from 1970-01-01 
    | `date`, `timestamp`, `timestamptz`, `timestamp_ns`, `timestamptz_ns`      
                                | `date` [1]  |
 | **`hour`**        | Extract a timestamp hour, as hours from 1970-01-01 
00:00:00  | `timestamp`, `timestamptz`, `timestamp_ns`, `timestamptz_ns`        
                                      | `int`       |
 | **`void`**        | Always produces `null`                                   
    | Any                                                                       
                                | Source type or `int` |
 
 All transforms must return `null` for a `null` input value.
 
 The `void` transform may be used to replace the transform in an existing 
partition field so that the field is effectively dropped in v1 tables. See 
partition evolution below.
 
+Notes:
+
+1. The physical representation is a 4-byte integer counting days from 
`1970-01-01`. Writers may produce Avro fields annotated both with and without a 
logical `date` annotation, and readers must accept both forms.

Review Comment:
   ```suggestion
   1. Readers must also accept `int` values for the `day` transform, 
interpreting each integer as a date represented by the number of days since 
`1970-01-01`.
   ```
   
   nit: could we keep this "Partition Transforms" section from mentioning file 
format specifics? 



##########
format/spec.md:
##########
@@ -514,14 +514,18 @@ Partition field IDs must be reused if an existing 
partition spec contains an equ
 | **`truncate[W]`** | Value truncated to width `W` (see below)                 
    | `int`, `long`, `decimal`, `string`, `binary`                              
                                | Source type |
 | **`year`**        | Extract a date or timestamp year, as years from 1970     
    | `date`, `timestamp`, `timestamptz`, `timestamp_ns`, `timestamptz_ns`      
                                | `int`       |
 | **`month`**       | Extract a date or timestamp month, as months from 
1970-01-01 | `date`, `timestamp`, `timestamptz`, `timestamp_ns`, 
`timestamptz_ns`                                      | `int`       |
-| **`day`**         | Extract a date or timestamp day, as days from 1970-01-01 
    | `date`, `timestamp`, `timestamptz`, `timestamp_ns`, `timestamptz_ns`      
                                | `int`       |
+| **`day`**         | Extract a date or timestamp day, as days from 1970-01-01 
    | `date`, `timestamp`, `timestamptz`, `timestamp_ns`, `timestamptz_ns`      
                                | `date` [1]  |
 | **`hour`**        | Extract a timestamp hour, as hours from 1970-01-01 
00:00:00  | `timestamp`, `timestamptz`, `timestamp_ns`, `timestamptz_ns`        
                                      | `int`       |
 | **`void`**        | Always produces `null`                                   
    | Any                                                                       
                                | Source type or `int` |
 
 All transforms must return `null` for a `null` input value.
 
 The `void` transform may be used to replace the transform in an existing 
partition field so that the field is effectively dropped in v1 tables. See 
partition evolution below.
 
+Notes:
+
+1. The physical representation is a 4-byte integer counting days from 
`1970-01-01`. Writers may produce Avro fields annotated both with and without a 
logical `date` annotation, and readers must accept both forms.

Review Comment:
   And move the Avro specific wording to https://iceberg.apache.org/spec/#avro
   something like
   
   ```
   #### Manifest Partition Field Encoding
   
   Manifest files store partition values in the `partition` tuple.
   
   For partition fields produced by the `day` transform, the Avro field may be
   encoded either as `"int"` or as `{ "type": "int", "logicalType": "date" }`.
   
   Readers must accept both forms. Writers should encode `day` partition fields 
as
   `{ "type": "int", "logicalType": "date" }`.
   ```



-- 
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