Add support for DataType::Timestamp casts in unwrap_cast_in_comparison optimizer pass#4148
Conversation
| ] { | ||
| let utc = Some("+0:00".to_string()); | ||
| // No timezone, utc timezone | ||
| let (lit_tz_none, lit_tz_utc) = match time_unit { |
There was a problem hiding this comment.
A note on the timezone handling is that expect_cast added in #4147 and used in this PR explicitly verifies that calling the arrow cast kernel produces the same result as the code in this PR.
| // pushed down / pruned | ||
| let expected = | ||
| "Projection: test.col_int32\n Filter: CAST(test.col_ts_nano_none AS Timestamp(Nanosecond, Some(\"+00:00\"))) < TimestampNanosecond(1666612093000000000, Some(\"+00:00\"))\ | ||
| "Projection: test.col_int32\ |
There was a problem hiding this comment.
It is finally fixed! Note there is no cast on the test.col_ts_nano_none column
|
If your pr is ready, please ping me. |
alamb
force-pushed
the
alamb/unwrap_timestamp_casts
branch
from
0f64cfc to
8b67ded
Compare
|
This PR is now ready for review @liukun4515 As an aside, I found working with this code to be quite straightforward -- thank you to @liukun4515 for the nice structure and tests |
|
@liukun4515 if you have a moment to review this PR I would appreciate it (as I have another PR with support for unsigned type support backed up behind it). 🙏 |
very happy to hear that |
|
Benchmark runs are scheduled for baseline = 129654c and contender = bcd8af8. bcd8af8 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
3 participants
Draft as it builds on tests in #4147Which issue does this PR close?
Closes #3938
Rationale for this change
I want predicates that compare timestamp columns and
now()to be fully optimized / pruned. See #3938 and https://github.com/influxdata/influxdb_iox/issues/5875 for more detailsWhat changes are included in this PR?
DataType::Timestampinunwrap_cast_in_comparisonoptimizer passAre these changes tested?
Yes (I wrote a whole new set of tests just for this :) )-- #4147
Are there any user-facing changes?
Hopefully faster queries