Repository navigation
feat(expr): add timestamp timezone replacement - #10182
Conversation
|
I need to look at this carefully |
Merging this PR will degrade performance by 5.73%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | mul_u64_nonnull_neon |
28.5 µs | 39.6 µs | -27.91% |
| ❌ | WallTime | multiply_shapes_neon[(32768, PerRowPerRow)] |
32.3 µs | 38.8 µs | -16.86% |
| ❌ | WallTime | mul_i64_nonnull_neon |
32.6 µs | 38.7 µs | -15.86% |
| ❌ | Simulation | sum_i64 |
193.9 µs | 224.9 µs | -13.76% |
| ❌ | Simulation | sum_v2_i64 |
193.8 µs | 224.4 µs | -13.61% |
| ❌ | WallTime | scalar_subtract_neon |
10.8 µs | 12.4 µs | -13.16% |
| ❌ | WallTime | bitpack_blocked_compress_avx2 |
6.8 µs | 7.6 µs | -11.13% |
| ⚡ | Simulation | decode_primitives[i64, (1000, 32)] |
37.7 µs | 24.5 µs | +53.82% |
| ⚡ | WallTime | compare_u64_avx2 |
3.8 µs | 3.4 µs | +11.69% |
| ⚡ | Simulation | or_true_constant |
20.3 µs | 18.2 µs | +11.31% |
| 🆕 | Simulation | varbinview_fill_null_inlined |
N/A | 276.8 µs | N/A |
| 🆕 | Simulation | varbinview_fill_null_outlined |
N/A | 289.1 µs | N/A |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing dk/replace-time-zone (92ddab2) with develop (53c7659)2
Footnotes
-
434 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
develop(30e84dc) during the generation of this report, so 53c7659 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
Polars predicate pushdown can serialize unsigned comparison literals as typed scalar variants that the converter rejects. Decode recognized scalar variants through the existing literal type mapping, including UInt8, UInt16, UInt32, and UInt64. Add file-scan regression cases for comparisons on all four unsigned widths. Split from the original combined PR; datetime changes are in #10183 (stacked on #10182), and `is_not_null` changes are in #10184. Validation: 4 targeted regression case(s) passed with this PR’s converter. With only `polars_.py` restored to the base revision and the same tests retained, all 4 failed with Polars `ComputeError` wrapping the unsupported-expression `ValueError` or `NotImplementedError`. The tests compare complete dataframes from Polars alone and a Vortex-backed Polars scan, and check explicit row expectations. No lint or broader suite was run. --------- Signed-off-by: Daniel King <dan@spiraldb.com> Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
Signed-off-by: Daniel King <dan@spiraldb.com>
Avoid Debug formatting in the ReplaceTimeZoneOptions Display impl, collapse the nested timestamp-options check, and apply rustfmt and ruff formatting. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E92wUoS3LaqWVJmmS7vKVq
e2695d2 to
993b177
Compare
Polars serializes every timezone-aware datetime literal as replace_time_zone(<naive literal>), and stats pruning only matches bare literal operands. Without folding, `col >= replace_time_zone(lit)` falsified to `max(col) < min(replace_time_zone(lit))`, which zone maps cannot bind, so no zones were pruned. Fold literal inputs in simplify, mirroring Cast, and share the scalar path with execute. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E92wUoS3LaqWVJmmS7vKVq
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Write values into a BufferMut and validity into a BitBufferMut instead of collecting Vec<Option<i64>>, rebuilding it with from_option_iter, and casting it to the nullable i64 it already was. Add a test that runs the array path with per-row policies and null inputs and policies. Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E92wUoS3LaqWVJmmS7vKVq
|
I have applied a couple of fixes here. This should be good now |
Timestamp expressions need to reinterpret local wall times in another timezone, including daylight-saving transitions. Add native
replace_time_zoneto Vortex's Rust and Python expression APIs, preserving timestamp units and propagating nulls.Support timezone removal, per-row
raise/earliest/latest/nullambiguity policies, andraise/nullpolicies for nonexistent times. Serialize options through protobuf and register the function for expression deserialization. Polars expression mapping is in the stacked datetime PR #10183.Validation: regenerated bindings with
cargo run -p xtask -- generate-proto. Added Rust cases for timestamp units, DST folds/gaps, overflow, invalid inputs, and option serialization, plus Python constant/null cases. Tests and linting were not run.