fix(duckdb,polars): fall back instead of silently changing temporal columns - #454
Merged
Merged
Conversation
…olumns
On the fully native path (strategy="conservative", fix_dtypes=False) native
ingestion changed temporal values with nothing recorded in fallback_events or
backend_differences:
- DuckDB stores INTERVAL in microseconds, so a timedelta64[ns] column was
truncated on the way back (1431904ns -> 1431000ns, 5ns -> 0).
- DuckDB returns TIMESTAMP WITH TIME ZONE in the session time zone at
microsecond resolution, so a tz-aware datetime column came back in the
machine's local zone with its sub-microsecond part dropped.
- Both engines mishandle period and interval dtypes: DuckDB raises
NotImplementedException("Data type 'period[M]' not recognized") and Polars
ingests a period as its raw int64 ordinal (2020-01 -> 600) and an interval
as a {left, right} struct.
pandas_ingest_fallback_reason() now takes the engine name and rejects those
columns, so the run takes the disclosed pandas fallback and fallback_policy
="error" can refuse it. Polars keeps its native path for nanosecond timedeltas
and tz-aware datetimes, which it carries exactly.
docs/fallback-matrix.md splits the old datetime/timedelta/categorical/period/
interval row into the three rows that are now true.
Closes #443
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
FreshData benchmark report —
|
| fixture | n_rows | n_cols | p50 s | p95 s | peak MB | repair % | false-repair % | preserve % | trust | monotonic | export % |
|---|
Authored-code reduction (Metric 6)
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
2 participants
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.
Closes #443.
On the fully native path (
strategy="conservative",fix_dtypes=False) native ingestion changed temporal values, with nothing recorded infallback_eventsorbackend_differences:timedelta64[ns]INTERVALis microseconds:1431904nscame back1431000ns,5nscame back0datetime64[ns, UTC]…00.000000123+00:00→…05:30:00+05:30on a machine set to Asia/Kolkata)period[M]NotImplementedException: Data type 'period[M]' not recognized; Polars returned the raw int64 ordinal (2020-01→600)interval[int64, right]{left, right}structpandas_ingest_fallback_reason()now takes the engine name, so the checks that belong to one backend's type system only apply there. Polars keeps its native path for nanosecond timedeltas and tz-aware datetimes, which it carries exactly — there is a test pinning that, so the DuckDB-only checks cannot quietly cost Polars its native path.Because these are ingest-time reasons,
fallback_policy="error"now refuses such a run before any pandas work, instead of returning silently altered data.Default-output changes: none. The default
strategy="balanced"already delegates the whole pipeline to pandas on every native engine.docs/fallback-matrix.mdhad one row claiming datetime / timedelta / categorical / period / interval were all native on both engines. It is split into the three rows that are now true.Verification — py3.12/pandas 2.3.3/duckdb 1.5.4/polars 1.42.1 and py3.9/pandas 1.5.3/duckdb 1.4.5/polars 1.36.1:
#443repro and the four new dtypes: exact values,fallback_eventslength 1 on both venvs; fail onmaintests/test_execution/: 377 passed, 10 skipped on both