Conversation
The polars lazy-frame filter pushdown treated every Datetime literal as
microseconds and emitted a naive TIMESTAMP. For TIMESTAMP_MS / TIMESTAMP_S
columns (polars Datetime('ms')) the literal landed in January 1970, so
filters like pl.col('t') > datetime(...) returned the wrong rows. For
TIMESTAMPTZ columns the UTC instant was cast to a naive TIMESTAMP and then
re-read in the session TimeZone, shifting it by the UTC offset.
Convert milliseconds and microseconds exactly, emit TIMESTAMPTZ when the
literal carries a time zone, and let polars apply nanosecond filters.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The polars lazy-frame filter pushdown (
rel.pl(lazy=True)) turns a polarsDatetimeliteral into SQL as if it were always naive microseconds:But polars serializes the literal as
[value, time_unit, time_zone], and after type coercion the literal takes the column's unit and zone. That gives silently wrong results in two cases:1.
TIMESTAMP_MS/TIMESTAMP_Scolumns (polarsDatetime("ms")). The literal is in milliseconds, so dividing by 1e6 lands it in January 1970, and the filter keeps every row:2.
TIMESTAMPTZcolumns when the sessionTimeZoneis not UTC. The UTC instant is emitted as a naiveTIMESTAMP. DuckDB then casts it back toTIMESTAMPTZin the session zone, which shifts it by the UTC offset:DuckDB's default
TimeZoneis the machine's local zone, so this hits anyone outside UTC.Fix
In
_pl_tree_to_sql'sDatetimebranch:timedelta(integer math, no float division);'...+00'::TIMESTAMPTZwhen the literal carries a time zone, so it compares as an instant;NotImplementedErrorfor nanoseconds, so polars applies that filter itself. ATIMESTAMPliteral can't hold nanoseconds. Before this change, nanosecond literals already failed the conversion (fromtimestampout of range), so they already fell back to polars.Tests
tests/fast/arrow/test_polars.py:test_polars_lazy_pushdown_timestamp_units[TIMESTAMP_S|TIMESTAMP_MS|TIMESTAMP|TIMESTAMP_NS]:==,>,>=and<counts against a datetime.test_polars_lazy_pushdown_timestamptz:<and>against 12:00 UTC withSET TimeZone = 'America/New_York'.On the base commit (f4f4ad2) the
TIMESTAMP_S,TIMESTAMP_MSandtimestamptzcases fail (assert 0 == 1,assert 2 == 1). With the fix they pass. The rest oftest_polars.pypasses too, excepttest_polars_from_json*, which fails identically before and after in my environment (prebuilt 1.5.5_duckdbwith the sourceduckdb/package onPYTHONPATH).ruff check/ruff format --check(0.13.3) are clean, andmypyreports the same output as on the base.This PR was prepared by an AI agent (Claude Code, operating the breken-ai account). The failures above were reproduced end to end and the tests were run locally, red on the base and green on the fix.