Skip to content

Commit a5faf1a

Browse files
authored
feat: skip per-dim discovery scans when query is an unfiltered scan (#193)
1 parent c656dda commit a5faf1a

2 files changed

Lines changed: 206 additions & 14 deletions

File tree

tests/test_ds.py

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,110 @@ def test_order_by_direction_sets_dim_order(air_dataset_small):
202202
np.testing.assert_allclose(out["air_avg"].values, expected.values)
203203

204204

205+
def test_unfiltered_scan_skips_dim_discovery(air_dataset_small):
206+
"""``SELECT * FROM <registered>`` does no per-dim discovery scans.
207+
208+
The pre-fix :func:`_build_lazy_scan` ran one full source scan per dim
209+
via ``inner_df.select(col(d)).distinct().sort(...)`` -- a single-column
210+
projection per dim, multiplied across the dim count. With a registered
211+
template and an unfiltered plan, :func:`_maybe_template_coords` returns
212+
the template's coord arrays directly and the discovery loop is skipped
213+
entirely. Verified by the iteration callback's projection list: no read
214+
where the projection is a single dim column.
215+
"""
216+
from xarray_sql.reader import read_xarray_table
217+
218+
reads: list = []
219+
table = read_xarray_table(
220+
air_dataset_small,
221+
chunks={"time": 6},
222+
_iteration_callback=lambda block, proj: reads.append(list(proj)),
223+
)
224+
ctx = XarrayContext()
225+
ctx.register_table("air", table)
226+
ctx._registered_datasets["air"] = air_dataset_small
227+
228+
reads.clear()
229+
out = ctx.sql('SELECT * FROM "air"').to_dataset()
230+
single_dim_reads = [
231+
proj for proj in reads if set(proj) <= {"time", "lat", "lon"}
232+
]
233+
assert not single_dim_reads, (
234+
f"fast path should skip per-dim discovery scans, got "
235+
f"{single_dim_reads!r}"
236+
)
237+
# The fast path must not break correctness on first access.
238+
out.compute() # safety: the Dataset still resolves
239+
240+
241+
def test_filtered_scan_still_uses_dim_discovery(air_dataset_small):
242+
"""A ``WHERE`` clause forbids the template-coord fast path.
243+
244+
With a filter the result's coord extent is a subset of the source, so
245+
reusing template coords would silently widen the output. The dispatch
246+
in :func:`_maybe_template_coords` falls back to per-dim discovery in
247+
that case, which is observable as a positive read count on the source.
248+
"""
249+
from xarray_sql.reader import read_xarray_table
250+
251+
reads: list = []
252+
table = read_xarray_table(
253+
air_dataset_small,
254+
chunks={"time": 6},
255+
_iteration_callback=lambda block, proj: reads.append(block),
256+
)
257+
ctx = XarrayContext()
258+
ctx.register_table("air", table)
259+
ctx._registered_datasets["air"] = air_dataset_small
260+
261+
reads.clear()
262+
ctx.sql('SELECT * FROM "air" WHERE lat > 30').to_dataset()
263+
assert reads, "filtered scan must hit the source to discover coord extent"
264+
265+
266+
def test_fast_path_uses_scanned_tables_coords_not_user_template(
267+
air_dataset_small,
268+
):
269+
"""Fast path sources coord values from the scanned table, not ``template=``.
270+
271+
With multiple registered Datasets, a user may pass ``template=other``
272+
for metadata recovery while the query scans a different registered
273+
table. The coord values must come from the **scanned** table's
274+
registered Dataset; using ``other``'s coords would silently widen the
275+
output if their coord ranges differ.
276+
"""
277+
other = air_dataset_small.isel(lat=slice(0, 5))
278+
assert other.sizes["lat"] != air_dataset_small.sizes["lat"]
279+
ctx = XarrayContext()
280+
ctx.from_dataset("air", air_dataset_small, chunks={"time": 24})
281+
ctx.from_dataset("other", other, chunks={"time": 24})
282+
out = ctx.sql('SELECT * FROM "air"').to_dataset(
283+
dims=["time", "lat", "lon"], template=other
284+
)
285+
# Scanned table is "air", so lat must match air's full lat axis.
286+
np.testing.assert_array_equal(
287+
out["lat"].values, air_dataset_small["lat"].values
288+
)
289+
290+
291+
def test_round_trip_preserves_descending_lat_on_lazy_path(air_dataset_small):
292+
"""Lazy round-trip preserves source dim order (xarray-sql#171).
293+
294+
NCEP ``air_temperature`` ships descending lat (75.0 -> 15.0). The
295+
discovery path's ``.distinct().sort()`` previously flipped lat to
296+
ascending on the lazy result. The template-coord fast path returns the
297+
source's coord arrays as-is, so the descending order survives.
298+
"""
299+
ds = air_dataset_small
300+
assert (np.diff(ds["lat"].values) < 0).all(), (
301+
"test relies on a descending-lat fixture"
302+
)
303+
ctx = XarrayContext()
304+
ctx.from_dataset("air", ds, chunks={"time": 24})
305+
lazy = ctx.sql('SELECT * FROM "air"').to_dataset()
306+
np.testing.assert_array_equal(lazy["lat"].values, ds["lat"].values)
307+
308+
205309
def test_chunks_argument_controls_partitioning(synthetic_dataset):
206310
"""``chunks`` controls eager-vs-chunked and inherits the source grid.
207311

xarray_sql/ds.py

Lines changed: 102 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -432,33 +432,115 @@ def _materialize(
432432
return xr.Dataset(data_vars=data_vars, coords=coords_arg)
433433

434434

435+
_PURE_SCAN_NODES = {"Projection", "Sort", "TableScan", "SubqueryAlias"}
436+
437+
438+
def _unfiltered_scan_table(inner_df: Any) -> str | None:
439+
"""Return the scanned table name iff the query is a pure unfiltered scan.
440+
441+
A pure scan only contains ``Projection``, ``Sort``, ``TableScan``,
442+
``SubqueryAlias`` nodes and exactly one ``TableScan``. Anything else
443+
(``Filter``, ``Aggregate``, ``Join``, ``Union``, ``Limit``, multi-table
444+
joins, ...) returns ``None`` so the caller falls back to per-dim
445+
discovery. The returned name is the registered table the caller can
446+
look up to source coord arrays from.
447+
"""
448+
try:
449+
lp = inner_df.logical_plan()
450+
except Exception:
451+
return None
452+
table_name: str | None = None
453+
stack = [lp]
454+
while stack:
455+
node = stack.pop()
456+
try:
457+
variant = node.to_variant()
458+
except Exception:
459+
return None
460+
cls = type(variant).__name__
461+
if cls not in _PURE_SCAN_NODES:
462+
return None
463+
if cls == "TableScan":
464+
try:
465+
this = variant.table_name()
466+
except (AttributeError, TypeError):
467+
return None
468+
if not isinstance(this, str):
469+
return None
470+
if table_name is not None and table_name != this:
471+
return None # multi-table scan; not a single source
472+
table_name = this
473+
stack.extend(node.inputs())
474+
return table_name
475+
476+
477+
def _maybe_template_coords(
478+
templates: dict[str, xr.Dataset] | None,
479+
dimension_columns: list[str],
480+
inner_df: Any,
481+
) -> dict[str, np.ndarray] | None:
482+
"""Use the scanned table's registered coord arrays directly when safe.
483+
484+
Returns coord arrays sourced from the registered Dataset for the
485+
scanned table iff the query is an unfiltered scan over that single
486+
table and the registered Dataset carries all requested dims. Returns
487+
``None`` otherwise so the caller falls back to per-dim discovery.
488+
Skipping discovery avoids one full plan execution per dim and
489+
preserves the source's coordinate order (xarray-sql#171).
490+
491+
Coord values come from the **scanned** registered Dataset, not from
492+
any user-supplied ``template=`` (which is for metadata recovery
493+
only). That keeps the fast path correct when a user with multiple
494+
registered Datasets passes a metadata template that differs from
495+
the query's source.
496+
"""
497+
if not templates:
498+
return None
499+
table = _unfiltered_scan_table(inner_df)
500+
if table is None or table not in templates:
501+
return None
502+
source = templates[table]
503+
if not all(d in source.coords for d in dimension_columns):
504+
return None
505+
return {d: np.asarray(source.coords[d].values) for d in dimension_columns}
506+
507+
435508
def _build_lazy_scan(
436509
inner_df: Any,
437510
dimension_columns: list[str],
438511
field_names: list[str],
439512
field_types: dict[str, Any],
513+
templates: dict[str, xr.Dataset] | None = None,
440514
) -> xr.Dataset:
441515
"""Build a lazy Dataset whose data vars are :class:`SQLBackendArray`.
442516
443517
Used when output chunking is requested: each data variable stays lazy and,
444518
once wrapped by ``Dataset.chunk``, every chunk reads its coordinate range via
445-
a pushdown filter on first access. Coordinates are discovered per dim via
519+
a pushdown filter on first access. Coordinates come either from the
520+
scanned table's registered Dataset (fast path, for unfiltered scans -- see
521+
:func:`_maybe_template_coords`) or from per-dim
446522
``inner_df.select(col(d)).distinct().sort(...).execute_stream()``; the table
447523
provider projects to that single coordinate column and skips data variables,
448524
so discovery reads coordinate values only (no data-variable I/O).
449525
"""
450-
coord_arrays: dict[str, np.ndarray] = {}
451-
for d in dimension_columns:
452-
dim_only = (
453-
inner_df.select(col(f'"{d}"')).distinct().sort(col(f'"{d}"').sort())
454-
)
455-
chunks = [b.to_pyarrow() for b in dim_only.execute_stream()]
456-
if not chunks:
457-
coord_arrays[d] = np.asarray([])
458-
continue
459-
coord_arrays[d] = np.concatenate(
460-
[c.column(0).to_numpy(zero_copy_only=False) for c in chunks]
461-
)
526+
coord_arrays = _maybe_template_coords(
527+
templates, dimension_columns, inner_df
528+
)
529+
if coord_arrays is None:
530+
coord_arrays = {}
531+
for d in dimension_columns:
532+
dim_only = (
533+
inner_df.select(col(f'"{d}"'))
534+
.distinct()
535+
.sort(col(f'"{d}"').sort())
536+
)
537+
chunks = [b.to_pyarrow() for b in dim_only.execute_stream()]
538+
if not chunks:
539+
coord_arrays[d] = np.asarray([])
540+
continue
541+
coord_arrays[d] = np.concatenate(
542+
[c.column(0).to_numpy(zero_copy_only=False) for c in chunks]
543+
)
462544
shape = tuple(len(coord_arrays[d]) for d in dimension_columns)
463545

464546
data_vars: dict[str, xr.Variable] = {}
@@ -550,6 +632,7 @@ def _result_to_xarray(
550632
sparsity: Sparsity,
551633
fill_value: Any,
552634
chunks: Mapping[str, int] | str | None,
635+
templates: dict[str, xr.Dataset] | None = None,
553636
) -> xr.Dataset:
554637
"""Reconstruct an ``xr.Dataset`` from a SQL result.
555638
@@ -583,7 +666,11 @@ def _result_to_xarray(
583666
ds = _materialize(inner_df, dimension_columns, field_names, field_types)
584667
else:
585668
ds = _build_lazy_scan(
586-
inner_df, dimension_columns, field_names, field_types
669+
inner_df,
670+
dimension_columns,
671+
field_names,
672+
field_types,
673+
templates=templates,
587674
)
588675

589676
if sparsity == "template":
@@ -730,6 +817,7 @@ def to_dataset(
730817
sparsity=sparsity,
731818
fill_value=fill_value,
732819
chunks=resolved_chunks,
820+
templates=self._templates,
733821
)
734822

735823
# ------------------------------------------------------------------

0 commit comments

Comments
 (0)