liurenjie1024 commented on code in PR #2069:
URL: https://github.com/apache/iceberg-rust/pull/2069#discussion_r2739292067
##########
crates/integrations/datafusion/src/physical_plan/expr_to_predicate.rs:
##########
@@ -266,6 +270,17 @@ fn scalar_value_to_datum(value: &ScalarValue) ->
Option<Datum> {
ScalarValue::LargeBinary(Some(v)) => Some(Datum::binary(v.clone())),
ScalarValue::Date32(Some(v)) => Some(Datum::date(*v)),
ScalarValue::Date64(Some(v)) => Some(Datum::date((*v / MILLIS_PER_DAY)
as i32)),
+ // Timestamp conversions - convert to microseconds (Iceberg's native
timestamp unit)
+ ScalarValue::TimestampSecond(Some(v), _) => {
+ Some(Datum::timestamp_micros(v * MICROS_PER_SECOND))
+ }
+ ScalarValue::TimestampMillisecond(Some(v), _) => {
+ Some(Datum::timestamp_micros(v * MICROS_PER_MILLI))
+ }
+ ScalarValue::TimestampMicrosecond(Some(v), _) =>
Some(Datum::timestamp_micros(*v)),
+ ScalarValue::TimestampNanosecond(Some(v), _) => {
Review Comment:
We should not convert nano seconds here since it will lose precision? For
example, let's say the predict is like `c1 >= 100 nano seconds`, this will be
converted to `c1 >= 0 micro seconds`, where `c1`'s data type is microseconds
timestamp in iceberg. In this case if c1 = 0, it will not pass first condition,
while it will pass second condition.
##########
crates/sqllogictest/src/engine/datafusion.rs:
##########
@@ -96,6 +96,7 @@ impl DataFusionEngine {
// Create partitioned test table (unpartitioned tables are now created
via SQL)
Self::create_partitioned_table(&catalog, &namespace).await?;
Self::create_binary_table(&catalog, &namespace).await?;
+ Self::create_timestamp_table(&catalog, &namespace).await?;
Review Comment:
We already supported creating table, see
https://github.com/apache/iceberg-rust/blob/045c0b2e298fee03e6d658bba78092a7fcf65747/crates/sqllogictest/testdata/slts/df_test/basic_queries.slt#L23,
we don't need to create table using rust code.
--
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]
