Copilot commented on code in PR #3623:
URL: https://github.com/apache/iceberg-rust/pull/3623#discussion_r4467979030
##########
crates/iceberg/src/arrow/schema.rs:
##########
@@ -644,15 +646,14 @@ impl ArrowSchemaVisitor for ArrowSchemaConverter {
DataType::Time64(unit) if unit == &TimeUnit::Microsecond => {
Ok(Type::Primitive(PrimitiveType::Time))
}
- DataType::Timestamp(unit, None) if unit == &TimeUnit::Microsecond
=> {
+ DataType::Timestamp(TimeUnit::Millisecond | TimeUnit::Microsecond,
None) => {
Ok(Type::Primitive(PrimitiveType::Timestamp))
Review Comment:
With field-ID-based projection, selecting an `id` column alongside an
optional `timestamp_ns` or `timestamptz_ns` column backed by
`TIMESTAMP(MILLIS)` now silently returns null timestamps instead of rejecting
the read. The projection compatibility check in `reader/projection.rs:206–213`
drops the timestamp leaf, so the transformer treats its stored values as
missing. Reject this unsupported pairing explicitly, or support its conversion
consistently, before accepting milliseconds here.
--
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]
