Skip to content

Add IntervalFilterSpec for validity-interval pushdown - #40

Merged
timkpaine merged 2 commits into
mainfrom
feat/interval-filter-spec
Sep 5, 2026
Merged

Add IntervalFilterSpec for validity-interval pushdown#40
timkpaine merged 2 commits into
mainfrom
feat/interval-filter-spec

Conversation

@hintse

@hintse hintse commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

FilterSpec maps one output column to one source column, so it cannot express the overlap predicate needed for validity-interval / history-of-change sources, where each row is valid over [start, end] and a request date range [lo, hi] selects covering windows via start <= hi AND end >= lo.

This adds a first-class IntervalFilterSpec that pushes the correct overlap predicate.

What it does

IntervalFilterSpec(
    start_col="valid_from",
    end_col="valid_to",
    closed="both",      # "both" | "left" | "right" | "none"
    value_mapping=...,  # offsets the request bounds before pushdown (as FilterSpec does)
)

For a request range [lo, hi] extracted from the pushed predicate, it filters the source with end_col >= lo and start_col <= hi (operators adjusted for closed), mapping the bounds through value_mapping first. The overlap is resolved entirely at the source row, so the surviving rows are exactly the overlapping windows and the range predicates push through to SQL/TickStore.

Date bounds compared against a Datetime source column reuse the existing _extend_dates_to_full_datetimes widening, so intraday rows on the boundary day are not silently dropped.

Notable shared-path change

The post-combine result_lf.filter(predicate) is now restricted (via the existing restrict_expr_to_columns) to columns present in the combined output before being applied. An interval spec's request/output column (e.g. date) is supplied by another joined frame and is not a column of the interval source itself, so restricting keeps it from leaking into the source-level filter. This is a no-op for FilterSpec, whose output column combine produces.

Testing

test_pushdown_combine.py: 92 passed (no regression). Consumer-facing tests (overlap correctness, pushdown verification, closed variants, value_mapping, Datetime sources, missing columns) live downstream.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Test Results

1 608 tests  +18   1 560 ✅ +18   50s ⏱️ ±0s
    2 suites ± 0      48 💤 ± 0 
    2 files   ± 0       0 ❌ ± 0 

Results for commit 308e717. ± Comparison against base commit 9218944.

♻️ This comment has been updated with latest results.

@hintse
hintse force-pushed the feat/interval-filter-spec branch 4 times, most recently from 07bc1e1 to 5c0833a Compare September 4, 2026 09:21
FilterSpec maps one output column to one source column, which cannot
express the overlap predicate needed for validity-interval sources where
each row is valid over [start, end]. IntervalFilterSpec pushes the correct
overlap (start_col <= hi AND end_col >= lo, adjusted for `closed`) onto the
source, mapping the request bounds through value_mapping and optionally
pruning windows longer than max_span. The overlap is resolved entirely at
the source row, so surviving rows are exactly the overlapping windows.

The post-combine predicate is now restricted (via the existing
restrict_expr_to_columns) to columns present in the output, so an interval
spec's virtual request column does not leak into the source-level filter.
This is a no-op for FilterSpec, whose output column combine produces.

Signed-off-by: Hin Tse <5867507+hintse@users.noreply.github.com>
@hintse
hintse force-pushed the feat/interval-filter-spec branch from 5c0833a to 68ebbb3 Compare September 4, 2026 09:29
@hintse
hintse marked this pull request as ready for review September 4, 2026 11:22

@ptomecek ptomecek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition — the two one-sided range predicates are exactly what pushdown can consume, and the Date-source path looks correct for all four closed modes. One correctness issue on Datetime sources with the half-open modes (details inline), plus a couple of smaller robustness notes. It'd also be good to land at least one in-repo test for IntervalFilterSpec so the module's contract is guarded here rather than only downstream.

Comment thread polars_io_tools/io_sources/pushdown_combine.py Outdated
Comment thread polars_io_tools/io_sources/pushdown_combine.py
Comment thread polars_io_tools/io_sources/pushdown_combine.py
Comment thread polars_io_tools/io_sources/pushdown_combine.py
@hintse
hintse force-pushed the feat/interval-filter-spec branch from 68ebbb3 to 7e00b2f Compare September 4, 2026 14:22
- Fix half-open closed modes on Datetime sources: a date request bound is
  now resolved to a full-day datetime comparison point directly, so
  left/right/none no longer shift the bound a full day and drop windows
  that overlap the boundary day. Endpoint open/closed-ness only selects
  the operator.
- Validate closed in __post_init__ (ValueError for anything outside
  {both,left,right,none}) so a typo fails fast instead of a late KeyError.
- Document conservative behavior: disjoint requests collapse to their
  outer hull, and a dict value_mapping missing a bound leaves that side
  unconstrained (over-selects, never drops an overlapping row).
- Add upstream tests: defaults, closed validation, overlap, pushdown
  verification, closed x {Date, Datetime} incl. intraday boundary, and
  the hull-collapse case.

Signed-off-by: Hin Tse <5867507+hintse@users.noreply.github.com>
@hintse
hintse force-pushed the feat/interval-filter-spec branch from 7e00b2f to 308e717 Compare September 4, 2026 14:40
@timkpaine
timkpaine merged commit e143a91 into main Sep 5, 2026
6 checks passed
@timkpaine
timkpaine deleted the feat/interval-filter-spec branch September 5, 2026 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants