fix(iceberg): fix iceberg predicate pushdown #20571
Conversation
| ScalarImpl::Timestamp(t) => Some(IcebergDatum::timestamp_nanos( | ||
| t.0.and_utc().timestamp_nanos_opt()?, | ||
| ScalarImpl::Timestamp(t) => Some(IcebergDatum::timestamp_micros( | ||
| t.0.and_utc().timestamp_micros(), |
There was a problem hiding this comment.
Are we fixing a different bug here? And do we have a test covering this branch 🤪
There was a problem hiding this comment.
Just found that timestamp is microsecond precision for both risingwave and iceberg 😆
There was a problem hiding this comment.
date <= DATE '2025-02-10' - interval '5' day
So the right side changes from timestamp_nanos to timestamp_micros.
And then will we pushdown or not predicate for it after the PR?
I don't fully get the reason of the type change here. 🤔
There was a problem hiding this comment.
Even though iceberg-rs has already handled it well.
There was a problem hiding this comment.
date <= DATE '2025-02-10' - interval '5' day
We won't push down this predicate because the lhs with data type date and the rhs is timestamp.
We can push down the predicate if we change it to date <= (DATE '2025-02-10' - interval '5' day)::date
There was a problem hiding this comment.
Just found that timestamp is microsecond precision for both risingwave and iceberg 😆
I found iceberg has a timestamp_ns type. And we've supported to convert it to RisingWave timestamp in #19221
So it seems using either one is ok here...? 🤔
There was a problem hiding this comment.
We can support timestamp_ns after this PR #20192
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Checklist
Documentation
Release note