Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #26452 +/- ##
==========================================
- Coverage 82.89% 82.89% -0.01%
==========================================
Files 1154 1154
Lines 457630 457665 +35
Branches 457630 457665 +35
==========================================
+ Hits 379370 379394 +24
- Misses 55256 55263 +7
- Partials 23004 23008 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I'm wondering if it's possible to have sqlogictests that would cover this? 🤔 |
| } | ||
|
|
||
| #[test] | ||
| fn row_group_predicate_cast_timestamp_to_time() { |
There was a problem hiding this comment.
| fn row_group_predicate_cast_timestamp_to_time() { | |
| fn row_group_predicate_cast_timestamp_to_time() { | |
| // A timezone shifts where the day boundary falls but does not remove | |
| // it, so a timezone-aware timestamp is just as unsafe to prune on. | |
| for timestamp_type in [ | |
| DataType::Timestamp(TimeUnit::Second, None), | |
| DataType::Timestamp(TimeUnit::Second, Some("+05:00".into())), | |
| DataType::Timestamp(TimeUnit::Nanosecond, Some("UTC".into())), | |
| ] { | |
| assert_timestamp_to_time_cast_is_not_prunable(timestamp_type); | |
| } | |
| } | |
| /// Asserts that no comparison of `CAST`/`TRY_CAST` from `timestamp_type` to | |
| /// a time-of-day produces a pruning expression. | |
| fn assert_timestamp_to_time_cast_is_not_prunable(timestamp_type: DataType) { |
I had claude update the test to handle timestamp with tz as I wanted to be sure that this fix covered that case as well.
There was a problem hiding this comment.
Thanks — added this in 1fc4b25. The helper now runs the same CAST/TRY_CAST assertions across all Time32/Time64 units for timestamps without a timezone, with +05:00, and with UTC at nanosecond precision.
|
I got wondering just how difficult it would be to have tests for this as slt tests so I once again pointed claude at it and here is what it came up with. All 6 Parquet-backed queries in this slt fail on main and pass in this PR. |
c874f76 to
1fc4b25
Compare
1fc4b25 to
514e0e3
Compare
@Omega359 Thanks for putting this together! I added |
Which issue does this PR close?
Rationale for this change
Casting timestamp statistics to time-of-day values does not preserve ordering across midnight. This can invert the minimum and maximum used by statistics pruning and incorrectly discard files or row groups that contain matching rows.
What changes are included in this PR?
Time32and timestamp-to-Time64casts from being used for statistics pruning.CASTandTRY_CASTacross all supported time units.What is the testing strategy for this PR?
The regression test verifies equality and range predicates for
Time32andTime64, including the expected row counts and pruning metrics. The full extended workspace test suite also passes.Are there any user-facing changes?
Queries that cast timestamps to time-of-day values may perform less statistics pruning, but they no longer risk incorrect results when timestamp bounds cross a day boundary.