colinmarc commented on code in PR #1569:
URL: https://github.com/apache/iceberg-rust/pull/1569#discussion_r2650531519
##########
crates/integrations/datafusion/src/physical_plan/expr_to_predicate.rs:
##########
@@ -214,18 +218,46 @@ fn scalar_value_to_datum(value: &ScalarValue) ->
Option<Datum> {
ScalarValue::LargeUtf8(Some(v)) => Some(Datum::string(v.clone())),
ScalarValue::Date32(Some(v)) => Some(Datum::date(*v)),
ScalarValue::Date64(Some(v)) => Some(Datum::date((*v / MILLIS_PER_DAY)
as i32)),
+ ScalarValue::TimestampSecond(Some(v), tz) => {
+ interpret_timestamptz_micros(v.checked_mul(MICROS_PER_SECOND)?,
tz.as_deref())
+ }
+ ScalarValue::TimestampMillisecond(Some(v), tz) => {
+
interpret_timestamptz_micros(v.checked_mul(MICROS_PER_MILLISECOND)?,
tz.as_deref())
+ }
+ ScalarValue::TimestampMicrosecond(Some(v), tz) => {
+ interpret_timestamptz_micros(*v, tz.as_deref())
+ }
+ ScalarValue::TimestampNanosecond(Some(v), Some(_)) =>
Some(Datum::timestamptz_nanos(*v)),
+ ScalarValue::TimestampNanosecond(Some(v), None) =>
Some(Datum::timestamp_nanos(*v)),
Review Comment:
> I think this code captures that logic? Note that the scalar value in
[Datafustion](https://github.com/apache/datafusion/blob/e64de0f2fed35497951fadb831adae3453e7a10c/datafusion/common/src/scalar/mod.rs#L309)
captures an epoch offset and an optional timezone. The Some(_) matches on the
line above and returns timestamp_tz (no normalization is needed because the
offset is from the epoch).
Correct; Datafusion has the same internal representation, where the storage
has an optional timezone but the timestamp is UTC. Internal to `iceberg-rust`
there's no way to pass on the timestamp information currently (all the
constructors just take the UTC stamp).
> Actually, looking more closely, I think the method is just doing extra work
Yeah, it was roundtripping unecessarily. Fixed.
--
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]
