Skip to content

Commit d561f45

Browse files
committed
Different fixes for issues that came out of a review
1. None==None view corruption — fast-path guard now explicitly checks _tbl.base is None to exclude view-backed columns, preventing the guard from firing when _last_pos and _n_rows are both None on a view. 2. CTable.__setitem__ missing view guard — added self.base is not None and self._read_only checks, matching the pattern of every other mutating CTable method. 3. Fast path dead for disk-opened tables — replaced the raw _last_pos read with _tbl._resolve_last_pos(), which lazily computes and caches the value on first call. Also replaced len(self) (O(n) scan) with _tbl._n_rows (cached integer). 4. Validation decompression — added a comment documenting that validate=True (the default) transiently decompresses NDArray inputs during constraint checking; validate=False is needed for fully bounded peak memory.
1 parent 51103bf commit d561f45

2 files changed

Lines changed: 43 additions & 2 deletions

File tree

src/blosc2/ctable.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,14 +1050,20 @@ def __setitem__(self, key: int | slice | list | np.ndarray, value): # noqa: C90
10501050
# Fast path: slice of a blosc2.NDArray into a scalar or ndarray column when
10511051
# there are no deleted rows (physical positions == logical positions).
10521052
# Skips the O(n) validity-mask gather and decompresses one chunk at a time.
1053+
# Guards: must not be a view (views have _last_pos=_n_rows=None, so
1054+
# None==None would be True and bypass the physical-position remapping);
1055+
# _resolve_last_pos() handles disk-opened tables where _last_pos starts None.
1056+
_tbl = self._table
10531057
if (
10541058
isinstance(key, slice)
10551059
and isinstance(value, blosc2.NDArray)
10561060
and not self.is_list
10571061
and not self.is_varlen_scalar
1058-
and self._table._last_pos == self._table._n_rows
1062+
and _tbl.base is None
1063+
and _tbl._resolve_last_pos() == _tbl._n_rows
10591064
):
1060-
start, stop, step = key.indices(len(self))
1065+
n_live = _tbl._n_rows
1066+
start, stop, step = key.indices(n_live)
10611067
chunk_size = value.chunks[0] if value.chunks else 65536
10621068
if self.is_ndarray:
10631069
spec = self._table._schema.columns_by_name[self._col_name].spec
@@ -9396,6 +9402,10 @@ def __setitem__(self, key: str, value) -> None:
93969402
raise TypeError(
93979403
f"CTable.__setitem__ only accepts a column name string, got {type(key).__name__!r}"
93989404
)
9405+
if self.base is not None:
9406+
raise ValueError("Table is a view and cannot be modified.")
9407+
if self._read_only:
9408+
raise ValueError("Table is read-only (opened with mode='r').")
93999409
physical = self._logical_to_physical_name(key)
94009410
if physical not in self._cols:
94019411
raise KeyError(f"Column {key!r} does not exist; use add_column() to add new columns")
@@ -10631,6 +10641,9 @@ def extend(self, data: list | CTable | Any, *, validate: bool | None = None) ->
1063110641
raw = raw_columns[name]
1063210642
if isinstance(raw, blosc2.NDArray):
1063310643
# Keep as-is; written chunk-by-chunk in the write loop below.
10644+
# Note: if do_validate=True, validate_column_batch() above will
10645+
# have already decompressed this column transiently for constraint
10646+
# checking. Pass validate=False to avoid that peak-memory spike.
1063410647
scalar_processed_cols[name] = raw
1063510648
else:
1063610649
scalar_processed_cols[name] = np.ascontiguousarray(raw, dtype=target_dtype)

tests/ctable/test_column.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,34 @@ def test_ctable_setitem_unknown_column_raises():
961961
t["nonexistent"] = np.zeros(20)
962962

963963

964+
def test_ctable_setitem_view_raises():
965+
"""t['col'] = arr on a view raises ValueError instead of silently mutating the parent."""
966+
t = CTable(Row, new_data=DATA20)
967+
view = t.where("id >= 0")
968+
with pytest.raises(ValueError, match="view"):
969+
view["score"] = np.zeros(len(view))
970+
971+
972+
def test_column_setitem_ndarray_fast_path_on_disk_table(tmp_path):
973+
"""Fast path fires for a disk-opened table (not just freshly-built in-memory tables)."""
974+
n = 60
975+
urlpath = str(tmp_path / "tbl.b2")
976+
977+
@dataclass
978+
class R:
979+
id: int = blosc2.field(blosc2.int64(ge=0))
980+
val: float = blosc2.field(blosc2.float64(), default=0.0)
981+
982+
t = CTable(R, new_data=[(i, 0.0) for i in range(n)], urlpath=urlpath, mode="w")
983+
t.close()
984+
985+
t2 = CTable(R, urlpath=urlpath, mode="a")
986+
arr = blosc2.asarray(np.arange(n, dtype=np.float64), chunks=(16,))
987+
t2["val"][:] = arr
988+
np.testing.assert_array_equal(t2["val"][:], np.arange(n, dtype=np.float64))
989+
t2.close()
990+
991+
964992
def test_column_setitem_blosc2_ndarray_no_holes():
965993
"""col[:] = blosc2.NDArray takes the no-holes fast path and round-trips correctly."""
966994
n = 200

0 commit comments

Comments
 (0)