Skip to content

Commit 9c48b53

Browse files
committed
Major fix: gutting the introspection path. Using output chunks as a knob to define how to partition output; by default, it inherets chunks from the input (so by default, things are lazy).
Minor fix: if we ORDER BY in the query, the output dims should follow that order.
1 parent ac53d04 commit 9c48b53

2 files changed

Lines changed: 168 additions & 100 deletions

File tree

tests/test_ds.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,60 @@ def test_barrier_query_scans_source_once(air_dataset_small):
177177
assert reads_after_compute == reads_after_construct
178178

179179

180+
def test_order_by_direction_sets_dim_order(air_dataset_small):
181+
"""A barrier query's ORDER BY direction carries through to the Dataset
182+
dimension order, rather than being force-sorted ascending.
183+
184+
``ORDER BY lat DESC`` must yield a strictly descending ``lat`` dimension,
185+
with data still correctly aligned to those (descending) coordinates.
186+
"""
187+
ctx = XarrayContext()
188+
ctx.from_dataset("air", air_dataset_small)
189+
out = ctx.sql(
190+
"SELECT lat, AVG(air) AS air_avg FROM air GROUP BY lat ORDER BY lat DESC"
191+
).to_dataset(dimension_columns=["lat"])
192+
193+
lat = out["lat"].values
194+
assert (np.diff(lat) < 0).all(), f"expected descending lat, got {lat}"
195+
196+
# Values stay aligned to the descending coordinate (scatter handles order).
197+
expected = (
198+
air_dataset_small.compute().mean(dim=["time", "lon"])["air"].sortby(
199+
"lat", ascending=False
200+
)
201+
)
202+
np.testing.assert_allclose(out["air_avg"].values, expected.values)
203+
204+
205+
def test_chunks_argument_controls_partitioning(synthetic_dataset):
206+
"""``chunks`` controls eager-vs-chunked and inherits the source grid.
207+
208+
The default ``"inherit"`` reuses the source's genuinely multi-chunk
209+
dimensions, so the output chunk grid maps onto the source partitions;
210+
``chunks=None`` forces an eager, in-memory result. Both reproduce the source.
211+
"""
212+
import dask.array as da
213+
214+
ctx = XarrayContext()
215+
ctx.from_dataset("t", synthetic_dataset)
216+
var = next(iter(synthetic_dataset.data_vars))
217+
218+
inherited = ctx.sql("SELECT * FROM t").to_dataset()
219+
assert isinstance(inherited[var].data, da.Array)
220+
# Output time chunks align to the source's time partitions.
221+
assert (
222+
inherited.chunksizes["time"] == synthetic_dataset.chunksizes["time"]
223+
)
224+
225+
eager = ctx.sql("SELECT * FROM t").to_dataset(chunks=None)
226+
assert not isinstance(eager[var].data, da.Array)
227+
228+
xr.testing.assert_allclose(
229+
inherited.compute().sortby(["time", "lat", "lon"]),
230+
synthetic_dataset.compute().sortby(["time", "lat", "lon"]),
231+
)
232+
233+
180234
# ---------------------------------------------------------------------------
181235
# dimension_columns / template resolution rules
182236
# ---------------------------------------------------------------------------

xarray_sql/ds.py

Lines changed: 114 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -6,29 +6,32 @@
66
:meth:`XarrayContext.sql`, with a :meth:`XarrayDataFrame.to_dataset`
77
method that round-trips a query result back to ``xr.Dataset``.
88
9-
Reconstruction dispatches on the SQL result's physical-plan shape
10-
(:func:`_classify_plan`):
11-
12-
* **Partition-preserving scans** (``SELECT cols FROM t WHERE <dim
13-
predicate>``) stay lazy: data variables are backed by
14-
:class:`SQLBackendArray` wrapped in
15-
``xarray.core.indexing.LazilyIndexedArray``. xarray indexing
16-
operations (``isel``, ``sel``, slicing) translate to DataFusion
17-
``filter`` expressions and consume the filtered DataFrame via
18-
``execute_stream``, so only the requested data is materialized as
19-
Arrow ``RecordBatch`` es and scattered into numpy.
20-
* **Partition-collapsing barriers** (aggregations, sorts, joins) execute
21-
the plan exactly once and materialize a dense Dataset. Re-executing
22-
such a plan per coordinate and per variable -- as the lazy path would
23-
-- re-runs the whole upstream scan each time, so a single streamed pass
24-
is both faster and correct.
9+
Reconstruction is controlled by the ``chunks`` argument to
10+
:meth:`XarrayDataFrame.to_dataset` -- the xarray idiom for tuning how a
11+
result is partitioned -- rather than by reflecting on the query plan:
12+
13+
* **Eager** (``chunks=None``, or the default ``"inherit"`` when the
14+
result keeps no multi-chunk source dimension): the plan executes
15+
exactly once via ``execute_stream`` and the result is scattered into a
16+
dense in-memory Dataset. This is the right default for reductions
17+
(aggregations), whose results are small, and it never re-executes.
18+
* **Lazy / chunked** (``chunks`` is a mapping, ``"auto"``, or
19+
``"inherit"`` over a multi-chunk source dimension): data variables are
20+
backed by :class:`SQLBackendArray` wrapped in
21+
``xarray.core.indexing.LazilyIndexedArray`` and chunked via xarray's
22+
configured chunk manager (dask, cubed, ...). Each chunk maps onto the
23+
source partitions and reads its coordinate range on access by
24+
translating the indexer into a DataFusion ``filter`` expression, so only
25+
the requested partitions are materialized as Arrow ``RecordBatch`` es
26+
and scattered into numpy.
2527
2628
``.compute()`` materializes the whole Dataset in memory.
2729
"""
2830

2931
from __future__ import annotations
3032

3133
import warnings
34+
from collections.abc import Mapping
3235
from typing import Any, Literal, cast
3336

3437
import numpy as np
@@ -245,7 +248,7 @@ class SQLBackendArray(xr.backends.BackendArray):
245248
symptom of a filtered query whose coord discovery missed a
246249
value.
247250
248-
Constructed by :func:`_lazy_to_xarray`; users should not instantiate
251+
Constructed by :func:`_build_lazy_scan`; users should not instantiate
249252
this class directly.
250253
"""
251254

@@ -373,69 +376,20 @@ def _raw_getitem(self, key: tuple) -> np.ndarray:
373376
)
374377

375378

376-
# Physical-plan operator names that preserve the source partitioning, so each
377-
# output partition maps to exactly one source partition (xarray chunk) and
378-
# indexer filters push down to partition pruning. The Rust xarray table
379-
# provider surfaces as ``CooperativeExec`` / ``FFI_ExecutionPlan``; in-memory
380-
# tables surface as ``DataSourceExec``. Anything else (aggregate, sort, join,
381-
# hash-repartition, window) reshuffles or collapses partitions.
382-
_PARTITION_PRESERVING_OPS = frozenset(
383-
{
384-
"DataSourceExec",
385-
"FFI_ExecutionPlan",
386-
"CooperativeExec",
387-
"ProjectionExec",
388-
"FilterExec",
389-
"CoalesceBatchesExec",
390-
}
391-
)
392-
393-
394-
def _classify_plan(inner_df: Any) -> Literal["scan", "barrier"]:
395-
"""Classify a DataFusion plan as a ``"scan"`` or a ``"barrier"``.
396-
397-
A ``"scan"`` (e.g. ``SELECT cols FROM t WHERE <dim predicate>``) preserves
398-
the source partitioning, so lazy per-partition reads push filters down to
399-
partition pruning and stay cheap -- the lazy :class:`SQLBackendArray` path
400-
is the right tool. A ``"barrier"`` (aggregation, sort, join) must execute
401-
the whole plan to produce any row, so the lazy path's per-coordinate and
402-
per-variable re-execution would re-run the entire upstream scan each time;
403-
such plans are materialized once instead (see :func:`_materialize_barrier`).
404-
405-
Conservative by construction: returns ``"scan"`` only when *every* node is
406-
known to preserve partitioning; any unrecognized operator yields
407-
``"barrier"`` (execute-once), which is never incorrect, only less lazy.
408-
"""
409-
try:
410-
plan = inner_df.execution_plan()
411-
except Exception:
412-
return "barrier"
413-
stack = [plan]
414-
while stack:
415-
node = stack.pop()
416-
display = node.display() or ""
417-
op = display.split(":", 1)[0].strip()
418-
if op not in _PARTITION_PRESERVING_OPS:
419-
return "barrier"
420-
stack.extend(node.children())
421-
return "scan"
422-
423-
424-
def _materialize_barrier(
379+
def _materialize(
425380
inner_df: Any,
426381
dimension_columns: list[str],
427382
field_names: list[str],
428383
field_types: dict[str, Any],
429384
) -> xr.Dataset:
430-
"""Execute a barrier plan once and build a dense in-memory Dataset.
431-
432-
Barrier plans (aggregations, sorts, joins) collapse the source partitioning,
433-
so the whole plan must run to produce any output. We run it exactly once via
434-
``execute_stream()`` -- streaming the result as Arrow ``RecordBatch`` es
435-
(``datafusion.RecordBatch.to_pyarrow()``) -- then derive both the
436-
coordinates and every data variable from that single pass. No per-coordinate
437-
or per-variable re-execution, so an aggregation over a remote Zarr scan
438-
costs one scan instead of one-per-dim plus one-per-variable.
385+
"""Execute the query once and build a dense in-memory Dataset.
386+
387+
Runs the plan exactly once via ``execute_stream()`` -- streaming the result
388+
as Arrow ``RecordBatch`` es (``datafusion.RecordBatch.to_pyarrow()``) -- then
389+
derives both the coordinates and every data variable from that single pass.
390+
This is the eager path, used when no output chunking is requested. It never
391+
re-executes, so an aggregation over a remote Zarr scan costs exactly one
392+
scan, regardless of how many dimensions or variables the result has.
439393
"""
440394
batches = [b.to_pyarrow() for b in inner_df.execute_stream()]
441395

@@ -450,9 +404,12 @@ def _materialize_barrier(
450404
for b in batches
451405
]
452406
)
453-
# np.unique returns sorted, de-duplicated values -- the same ascending
454-
# coordinate order the lazy scan path produces via SQL ``.sort()``.
455-
coord_arrays[d] = np.unique(vals)
407+
# Preserve the order coordinate values first appear in the result so an
408+
# ORDER BY direction (e.g. ``ORDER BY level DESC``) carries through to
409+
# the Dataset dimension instead of being force-sorted ascending.
410+
# pd.unique keeps first-appearance order; the scatter below argsorts
411+
# internally, so arbitrarily-ordered coordinates are placed correctly.
412+
coord_arrays[d] = np.asarray(pd.unique(vals))
456413
shape = tuple(len(coord_arrays[d]) for d in dimension_columns)
457414

458415
data_vars: dict[str, xr.Variable] = {}
@@ -483,12 +440,12 @@ def _build_lazy_scan(
483440
) -> xr.Dataset:
484441
"""Build a lazy Dataset whose data vars are :class:`SQLBackendArray`.
485442
486-
For partition-preserving (``"scan"``) plans only. Coord arrays are
487-
discovered per-dim via ``inner_df.select(col(d)).distinct().sort(...)
488-
.execute_stream()``; the table provider projects to that single coordinate
489-
column and skips data variables, so each discovery reads coordinate values
490-
only (no remote data-variable I/O). Data variables stay lazy: indexer
491-
filters push down to partition pruning on first access.
443+
Used when output chunking is requested: each data variable stays lazy and,
444+
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
446+
``inner_df.select(col(d)).distinct().sort(...).execute_stream()``; the table
447+
provider projects to that single coordinate column and skips data variables,
448+
so discovery reads coordinate values only (no data-variable I/O).
492449
"""
493450
coord_arrays: dict[str, np.ndarray] = {}
494451
for d in dimension_columns:
@@ -524,21 +481,28 @@ def _build_lazy_scan(
524481
return xr.Dataset(data_vars=data_vars, coords=coords_arg)
525482

526483

527-
def _lazy_to_xarray(
484+
def _result_to_xarray(
528485
inner_df: Any,
529486
dimension_columns: list[str],
530487
template: xr.Dataset | None,
531488
sparsity: Sparsity,
532489
fill_value: Any,
490+
chunks: Mapping[str, int] | str | None,
533491
) -> xr.Dataset:
534-
"""Reconstruct an ``xr.Dataset`` from a SQL result, lazily where possible.
535-
536-
Dispatches on the plan shape (:func:`_classify_plan`): partition-preserving
537-
``"scan"`` plans keep the lazy :class:`SQLBackendArray` path
538-
(:func:`_build_lazy_scan`); partition-collapsing ``"barrier"`` plans
539-
(aggregations, sorts, joins) execute once and materialize
540-
(:func:`_materialize_barrier`), since re-executing them per coordinate and
541-
per variable would re-run the whole upstream scan repeatedly.
492+
"""Reconstruct an ``xr.Dataset`` from a SQL result.
493+
494+
``chunks`` (already resolved by :meth:`XarrayDataFrame._resolve_chunks`)
495+
selects the execution strategy:
496+
497+
* ``None`` -> eager: execute once and materialize a dense Dataset
498+
(:func:`_materialize`). Correct for any query and the right default for
499+
reductions, whose results are small.
500+
* a mapping (or ``"auto"``) -> lazy/chunked: build :class:`SQLBackendArray`
501+
data variables (:func:`_build_lazy_scan`) and wrap them with
502+
``Dataset.chunk`` so each chunk reads its coordinate range via filter
503+
pushdown. The chunk grid maps onto the source partitions. Chunking goes
504+
through xarray's configured chunk manager (dask, cubed, ...), so no
505+
chunked-array backend is imported directly here.
542506
"""
543507
if sparsity not in ("result", "template"):
544508
raise ValueError(
@@ -553,8 +517,8 @@ def _lazy_to_xarray(
553517
field_names = [f.name for f in schema]
554518
field_types = {f.name: f.type for f in schema}
555519

556-
if _classify_plan(inner_df) == "barrier":
557-
ds = _materialize_barrier(
520+
if chunks is None:
521+
ds = _materialize(
558522
inner_df, dimension_columns, field_names, field_types
559523
)
560524
else:
@@ -574,6 +538,11 @@ def _lazy_to_xarray(
574538

575539
if template is not None:
576540
ds = _apply_template(ds, template)
541+
542+
if chunks is not None:
543+
# Wrap the lazy data variables in the configured chunk manager (dask by
544+
# default). Each chunk reads its coordinate range via pushdown on access.
545+
ds = ds.chunk(chunks)
577546
return ds
578547

579548

@@ -629,6 +598,7 @@ def to_dataset(
629598
template_table: str | None = None,
630599
sparsity: Sparsity = "result",
631600
fill_value: Any = np.nan,
601+
chunks: Mapping[str, int] | str | None = "inherit",
632602
) -> xr.Dataset:
633603
"""Convert the result to an ``xr.Dataset``.
634604
@@ -648,6 +618,21 @@ def to_dataset(
648618
``fill_value``; requires a template.
649619
fill_value: Used when ``sparsity="template"`` reindexes
650620
to a wider extent. Defaults to ``np.nan``.
621+
chunks: Output chunking, controlling laziness (an xarray idiom).
622+
623+
* ``"inherit"`` (default): reuse the source Dataset's chunk
624+
sizes, but only for dimensions that were genuinely split into
625+
multiple chunks in the input -- so the output chunk grid maps
626+
onto the source partitions. A reduction that drops the chunked
627+
dimension (e.g. a global aggregation) inherits nothing and so
628+
is materialized eagerly. Falls back to eager when no source
629+
Dataset is resolvable.
630+
* ``None``: eager. Execute the query once and return a dense
631+
in-memory Dataset. Best for reductions (small results).
632+
* a mapping (e.g. ``{"time": 100}``) or ``"auto"``: chunk
633+
explicitly. Each chunk reads its coordinate range lazily via
634+
filter pushdown on access, through xarray's configured chunk
635+
manager (dask, cubed, ...).
651636
652637
Returns:
653638
An ``xr.Dataset`` with ``dimension_columns`` as dimensions and the
@@ -669,22 +654,51 @@ def to_dataset(
669654
dimension_columns = self._infer_dimension_columns(
670655
preferred_template=template
671656
)
672-
# Dispatch on plan shape: partition-preserving scans stay lazy with
673-
# filter pushdown; partition-collapsing barriers (aggregations, sorts,
674-
# joins) execute once and materialize, since re-executing them per
675-
# coordinate and per variable would re-run the whole upstream scan.
676-
return _lazy_to_xarray(
657+
resolved_chunks = self._resolve_chunks(
658+
chunks, template, dimension_columns
659+
)
660+
return _result_to_xarray(
677661
inner_df=self._inner,
678662
dimension_columns=dimension_columns,
679663
template=template,
680664
sparsity=sparsity,
681665
fill_value=fill_value,
666+
chunks=resolved_chunks,
682667
)
683668

684669
# ------------------------------------------------------------------
685670
# Internals
686671
# ------------------------------------------------------------------
687672

673+
@staticmethod
674+
def _resolve_chunks(
675+
chunks: Mapping[str, int] | str | None,
676+
template: xr.Dataset | None,
677+
dimension_columns: list[str],
678+
) -> Mapping[str, int] | str | None:
679+
"""Resolve the ``chunks`` argument to a concrete spec or ``None``.
680+
681+
``None`` selects the eager path; anything else selects the lazy/chunked
682+
path. ``"inherit"`` reuses the source Dataset's chunk sizes -- but only
683+
for dimensions actually split into more than one chunk in the input
684+
(a single full chunk is not "chunked"), so reductions that drop the
685+
chunked dimension resolve to ``None`` (eager) automatically. Mappings
686+
and ``"auto"`` pass through to ``Dataset.chunk`` unchanged.
687+
"""
688+
if chunks is None:
689+
return None
690+
if chunks == "inherit":
691+
if template is None:
692+
return None
693+
sizes = template.chunksizes # dim -> tuple of chunk lengths
694+
inherited = {
695+
d: sizes[d][0]
696+
for d in dimension_columns
697+
if d in sizes and len(sizes[d]) > 1
698+
}
699+
return inherited or None
700+
return chunks
701+
688702
def _resolve_template(
689703
self, template_table: str | None
690704
) -> xr.Dataset | None:

0 commit comments

Comments
 (0)