ZENOTME commented on code in PR #11749:
URL: https://github.com/apache/iceberg/pull/11749#discussion_r1881555120
##########
format/spec.md:
##########
@@ -454,7 +454,7 @@ 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`
| `int` (the physical type should be an `int`,
but the the logical type should be a `date`) |
Review Comment:
> TBH I also don't fully understand why we can't use date directly here,
since it's a primitive type.
+1. I'm not sure whether my understanding is correct. According to
https://github.com/apache/iceberg/issues/279#issuecomment-519620975, the day
transform returns an integer from 1970, which is equal to the date
representation in iceberg-java so that it can be treated as the date type.
However, in iceberg spec, the definition of date is `Calendar date without
timezone or time`, I think this definition is not necessarily equal to `date is
an integer from 1970`, and that's why we can't say it's a date in iceberg spec,
e.g. in some iceberg implementation, they use string to represent a date. I
guess that's what
https://github.com/apache/iceberg/pull/9345#discussion_r1439682666 means.
If the above understanding is correct, to make things clear, maybe we should:
1. Change the definition of date to `date is an integer from 1970`. AFAIK,
this also holds in iceberg-rust
2. Should not use date as result type, because that's not equal to the
definition of date type in spec.
I think using the logical type to explanation is also confused because the
spec didn't define what's logical type.
--
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
