Skip to content

Commit d6016ae

Browse files
FrancescAltedclaude
andcommitted
Speed up dictionary-column Arrow export; clearer mode='a' error
iter_arrow_batches() recomputed the full live-row-position array from scratch on every batch, for every dictionary column, instead of once per export call — an O(n_rows) scan repeated O(n_rows / batch_size) times. Hoisted the computation out of the loop (6-14x faster export for dictionary columns on a 1M-row benchmark). Added a regression test covering the fix across a batch boundary with deleted rows. Also raise a clear FileNotFoundError when mode='a' is used to open a CTable that doesn't exist yet, instead of falling through to create one silently (both the regular constructor and the from_arrow/import storage path). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 4dcbf51 commit d6016ae

2 files changed

Lines changed: 57 additions & 4 deletions

File tree

src/blosc2/ctable.py

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3941,6 +3941,11 @@ def __init__(
39413941
# ---- Create new table ----
39423942
if storage.is_read_only():
39433943
raise FileNotFoundError(f"No CTable found at {urlpath!r}")
3944+
if urlpath is not None and mode == "a":
3945+
raise FileNotFoundError(
3946+
f"No CTable found at {urlpath!r}: mode='a' opens an existing table; "
3947+
"use mode='w' to create a new one."
3948+
)
39443949

39453950
# Build compiled schema from either a dataclass or a legacy Pydantic model
39463951
if dataclasses.is_dataclass(row_type) and isinstance(row_type, type):
@@ -6714,6 +6719,15 @@ def iter_arrow_batches( # noqa: C901
67146719
names = self._resolve_arrow_columns(columns, include_computed=include_computed)
67156720
arrow_names = self._export_arrow_names(names)
67166721

6722+
# Dictionary columns need the physical positions of their live rows.
6723+
# This depends only on self._valid_rows (fixed for the whole call), so
6724+
# it is computed once here instead of once per batch per dictionary
6725+
# column — the previous per-batch recompute was an O(n_rows) scan
6726+
# repeated O(n_rows / batch_size) times.
6727+
dict_real_pos = None
6728+
if any(name in self.col_names and self[name].is_dictionary for name in names):
6729+
dict_real_pos = blosc2.where(self._valid_rows, _arange(len(self._valid_rows))).compute()
6730+
67176731
for start in range(0, self._n_rows, batch_size):
67186732
stop = min(start + batch_size, self._n_rows)
67196733
arrays = []
@@ -6756,10 +6770,9 @@ def iter_arrow_batches( # noqa: C901
67566770
if col.is_dictionary:
67576771
dc = self._cols[name] # DictionaryColumn
67586772
spec = self._schema.columns_by_name[name].spec
6759-
# Get physical positions for live rows in [start, stop)
6760-
valid = self._valid_rows
6761-
real_pos = blosc2.where(valid, _arange(len(valid))).compute()
6762-
batch_real_pos = real_pos[start:stop]
6773+
# Physical positions for live rows in [start, stop),
6774+
# precomputed once above (not per batch/column).
6775+
batch_real_pos = dict_real_pos[start:stop]
67636776
if len(batch_real_pos) == 0:
67646777
pa_dict = pa.array(dc.dictionary, type=pa.string())
67656778
pa_indices = pa.array([], type=pa.int32())
@@ -7235,6 +7248,11 @@ def _storage_for_arrow_import(urlpath: str | None, mode: str) -> TableStorage:
72357248
shutil.rmtree(urlpath)
72367249
else:
72377250
os.remove(urlpath)
7251+
elif mode == "a" and not os.path.exists(urlpath):
7252+
raise FileNotFoundError(
7253+
f"No CTable found at {urlpath!r}: mode='a' opens an existing table; "
7254+
"use mode='w' to create a new one."
7255+
)
72387256
return FileTableStorage(urlpath, mode)
72397257

72407258
@classmethod

tests/ctable/test_arrow_interop.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,41 @@ def test_from_arrow_dictionary_codes_use_aligned_grid():
454454
assert list(t["c"][:5]) == c.to_pylist()[:5]
455455

456456

457+
def test_to_arrow_dictionary_multi_batch_with_deletions():
458+
"""Dictionary-column export across several batches, with holes in the
459+
live-row mask from a deletion, still maps each batch to the correct
460+
physical positions.
461+
462+
Regression test for a perf fix where the live-row-position array was
463+
recomputed from scratch on every batch (and every dictionary column);
464+
this exercises the same code path across a batch boundary to make sure
465+
the now-precomputed-once array still slices correctly per batch.
466+
"""
467+
468+
@dataclass
469+
class Row:
470+
category: str = blosc2.field(blosc2.dictionary())
471+
value: int = blosc2.field(blosc2.int64())
472+
473+
n = 5000 # several batches at the default Arrow batch size (2048)
474+
categories = ["alpha", "beta", "gamma"]
475+
data = {
476+
"category": [categories[i % 3] for i in range(n)],
477+
"value": list(range(n)),
478+
}
479+
t = CTable(Row, new_data=data)
480+
t.delete(slice(100, 250)) # punch a hole spanning a batch boundary
481+
482+
expected_category = [categories[i % 3] for i in range(n) if not (100 <= i < 250)]
483+
expected_value = [i for i in range(n) if not (100 <= i < 250)]
484+
485+
batches = list(t.iter_arrow_batches(batch_size=500))
486+
assert len(batches) > 1 # actually exercises multiple batches
487+
at = pa.Table.from_batches(batches)
488+
assert at["category"].to_pylist() == expected_category
489+
assert at["value"].to_pylist() == expected_value
490+
491+
457492
def test_from_arrow_variable_batches_roundtrip():
458493
"""Variable-sized Arrow batches that straddle the column chunk grid import
459494
losslessly (exercises the chunk-aligned write buffer)."""

0 commit comments

Comments
 (0)