feat: skip per-dim discovery scans when query is an unfiltered scan#193
Merged
Merged
Conversation
When a query is a pure unfiltered scan over a single registered
template (Projection / Sort / TableScan / SubqueryAlias only), the
result's coordinate extent is exactly the registered Dataset's.
Reuse its coord arrays directly instead of running one DISTINCT scan
per dim. Filtered, aggregated, joined, limited, multi-table queries
fall back to the existing per-dim discovery path unchanged.
The detector walks the logical plan and returns the scanned table
name; coord values are then sourced from that registered Dataset, not
from any user-supplied template= argument (which is only for metadata
recovery). That keeps the fast path correct when a user with multiple
registered Datasets passes a metadata template that differs from the
query's actual source.
Construction cost on NCEP air_temperature (chunks={'time':24}):
- before: peak 77 MB, wall 3.4 s (3x full-source scans for 3 dims)
- after : peak 0.1 MB, wall 7 ms
Also fixes xarray-sql#171 (stable dim order on the lazy round-trip):
the discovery path's .distinct().sort() forced ascending; the
template-coord fast path returns coords in source order, so a source
with descending lat (e.g. air_temperature: 75 -> 15) round-trips
descending instead of being flipped.
alxmrs
approved these changes
Jun 28, 2026
alxmrs
left a comment
alxmrs
left a comment
Collaborator
There was a problem hiding this comment.
A few notes -- great idea. I can't wait to benchmark this optimization.
| ctx.register_table("air", table) | ||
| ctx._registered_datasets["air"] = air_dataset_small | ||
|
|
||
| reads.clear() |
| table = read_xarray_table( | ||
| air_dataset_small, | ||
| chunks={"time": 6}, | ||
| _iteration_callback=lambda block, proj: reads.append(block), |
Collaborator
There was a problem hiding this comment.
Can we make this append consistent with the above version (block or proj)
Comment on lines
+282
to
+284
| out = ctx.sql('SELECT * FROM "air"').to_dataset( | ||
| dims=["time", "lat", "lon"], template=other | ||
| ) |
| ) | ||
|
|
||
|
|
||
| def test_round_trip_preserves_descending_lat_on_lazy_path(air_dataset_small): |
| _PURE_SCAN_NODES = {"Projection", "Sort", "TableScan", "SubqueryAlias"} | ||
|
|
||
|
|
||
| def _unfiltered_scan_table(inner_df: Any) -> str | None: |
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.
For an unfiltered scan over a single registered table (Projection / Sort / TableScan / SubqueryAlias only) the result's coord extent is exactly the source's.
_build_lazy_scannow reuses the registered Dataset's coord arrays directly instead of running one DISTINCT scan per dim. Filtered / aggregated / joined / limited / multi-table queries fall back to the existing discovery path unchanged. Also closes #171 (lazy round-trip now preserves source dim order, e.g. descending lat).Results
to_dataset()construction on NCEPair_temperature(chunks={'time':24}):171 / 171 tests pass; detector audited across 8 query shapes.