Mask-based nullable columns for CTable - #694
Open
FrancescAlted wants to merge 24 commits into
Open
Conversation
…ls 0-1)
Phase 0 of plans/mask-based-nulls.md: new ctable_nulls.py holds NullChannel,
a uniform accessor for a column's validity channel, plus the shared sentinel
helpers (kind_of_spec, sentinel_mask, is_nan_sentinel, is_null_value). The
hand-rolled `getattr(spec, "null_value")` comparisons scattered across
ctable.py, groupby.py, ctable_indexing.py, schema_validation.py and
schema_vectorized.py now route through it, with no behavior change -- the
existing suite passes unmodified. This incidentally unifies the "is the
sentinel a NaN" test, which several sites spelled as `isinstance(nv, float)`
and so missed a float32 NaN sentinel.
Phase 1: string predicates over a nullable column were not null-aware at all.
Only the operator form forced nulls to False; t.where("a > 10") compared the
raw sentinel, so a sentinel that satisfies the predicate (null_value=999
against > 10) returned its nulls as matches. CTable._rewrite_null_predicates
now conjoins a validity guard onto each comparison leaf, per leaf so that OR
stays correct -- a global conjunct would drop a row that is null in one
column but matches the other branch.
Two refinements keep this from costing performance: the guard is emitted
inline, `(a > 90) & (a != 999)`, so it stays a predicate on the same column
rather than an operand opaque to the index planner; and it is emitted only
when the sentinel could actually satisfy the leaf, since -1 cannot match
`> 10` and NaN cannot match anything but `!=`. Under a negation the guard
attaches at the negation itself, because ~((a > 10) & valid) would yield True
for a null where SQL says False.
Contrary to the plan, _exclude_null_positions and the indexed-OR bail are
retained. An ordered index answers `a > 90` by taking a range of the sorted
column and never evaluates the predicate, so a NaN sentinel -- which sorts
last -- lands inside every `>` range regardless: measured on 1M rows, the
scan matches 83146 rows while the FULL index returns 160070 positions, 76924
of them NaN. A null-aware expression therefore cannot make the index result
correct. The plan document is corrected in place.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 2 of plans/mask-based-nulls.md: the declaration layer for validity-mask null storage. Nothing reads or writes a sidecar yet, and the default is still "sentinel", so every existing table and every schema this release writes is byte-identical to before. A _NullableSpecMixin carries the nullability plumbing that nine specs used to each re-declare, so `null_storage` reaches all of them through one kwarg and one call apiece. _null_metadata() never emits "sentinel": that is still the default storage, so sentinel tables serialize exactly as they did and keep opening in readers that predate masks. Only "mask" is recorded, and only a mask column raises the schema version to 3 -- schema_to_dict computes the version as an explicit feature max, so a table without one stays at 1 or 2. schema_from_dict accepts 3 and its rejection message now names the version and points at convert_nulls(to='sentinel'). Two consequences fall out for free. A nullable bool under mask storage stays a real np.bool_ column instead of uint8 with a reserved 255, and a nullable string keeps its declared width instead of being widened to fit the sentinel text. Complex columns become nullable for the first time, mask-only: there is no complex value safe to reserve, which is the argument for a side channel in miniature, so nullable=True resolves to mask there regardless of policy. CTable._resolved_null_storage is the single decision point, resolving in order: explicit spec.null_storage, explicit null_value, a per-column policy.column_null_values entry, a type-wide policy sentinel field covering this kind, then policy.null_storage. That fourth rule is what keeps existing NullPolicy(float_value=...) code working once the default flips: setting a sentinel field opts those types into sentinel storage rather than becoming silently inert. __post_init__ raises only for null_storage="mask" written alongside such a field, which is a contradiction the caller wrote down. Contrary to the plan, the bool_ -> uint8 dtype flip stays in bool.__init__ rather than moving wholly into _resolve_nullable_specs: opening a stored table rebuilds specs through spec_cls(**data) and never runs the resolver, so a persisted uint8 column has to come back as uint8 from its metadata alone. __init__ resolves what it can and the resolver corrects it in both directions once the policy has spoken. Same for NDArraySpec's bool columns. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fix the stale _rewrite_null_predicates docstring that still claimed the index path could drop its null post-filtering, contradicting the plan's correction and the retained _exclude_null_positions. Pin the one known string/operator divergence -- the three-valued corner ~((a > 10) & (b == 999)) with a null operand -- as intentional, and record a pre-existing operator-form bug found while pinning it: bare ~(t.a > 10) wrongly returns null rows, because _null_aware_compare collapses null to False at the leaf and ~ inverts it. Documented as a strict xfail and as a dated addendum in the plan's negation blockquote, filed under the deferred Kleene follow-up. Also brings the plan's Phase 0-2 as-built annotations into the tree. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A mask-storage nullable column now has a ``.notnull`` sidecar beside its values: a plain bool NDArray, one byte per physical row, True where the value is not null. Nothing reads or writes one through the public API yet -- that is Phase 4 -- but every path that moves, grows, shrinks or serializes a column now carries it along. Five TableStorage methods across all four backends, plus the companion-suffix loop that generalizes delete_column/rename_column instead of adding a third copy of the utf8 branch. The sidecar is materialized lazily, on the first null actually written, so an absent one is a valid -- and, for a nullable column that has never seen a null, the expected -- state meaning "every row is valid". Null-free nullable columns therefore cost nothing on disk and nothing on the read path. Two things the plan's site list had wrong, both found by testing: * copy()'s in-memory path builds its result through _empty_copy and a per-column gather, never through _save_to_storage, so a copied table would have silently dropped its sidecars. * resize() zero-fills, i.e. *invalid*, so _grow has to write True over the new tail explicitly or every appended row past the old capacity reads as null. Iteration consults storage rather than the open handles: a freshly reopened table has opened no sidecar, and skipping an unopened one would leave it out of step with its column. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
``None`` becomes the canonical way to write a null into a fixed-width scalar column, which could not accept one at all before -- users had to write the sentinel literally. The null API now reads validity from the sidecar instead of inferring it from the values, which makes fillna() correct even when the fill equals data the column really holds. Two behaviours worth stating plainly: * Nullable bool is a real np.bool_ again. No uint8, no reserved 255. * NaN is a value, not a null. A mask-backed float column follows Arrow: only mask=False is missing. Sentinel float columns keep NaN-as-null forever, and the divergence is deliberate and tested on both sides. The riskiest part was not __setitem__, which needed one condition and one call per branch. It was that Column cached its NullChannel while the channel held the Column: a two-object cycle that refcounting can never break, harmless until extend() started building a channel per column, at which point every write pinned a whole table until the next gc pass. The cache is gone; the channel is a one-slot object and is rebuilt per access. Also not in the plan's site list: sort_by (all three forms), take, and slice each rebuild a table from gathered rows and silently dropped the sidecar. Factored into _permute_null_masks and _gather_null_masks_into, the latter leaving a copy sidecar-free when its selection contains no null. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
argmin/argmax and the ndarray reduction path both keyed off ``null_value is not None``, so under mask storage they reduced over the fill. For an int column the fill is 0 -- a plausible-looking minimum for positive data, and a plausible-looking maximum for negative data. ``_reduction_null_mask`` is now the one place that answers "which live rows are null", from the sentinel for one storage and from the sidecar for the other. The expression layer needed nothing: Phases 0-4 had already left it consuming an opaque boolean predicate, and Phase 4 taught the channel to produce one from the sidecar. Two things the plan expected here are *not* done, because measuring them showed the premises were wrong. Both are now pinned by tests and explained at the site where someone would otherwise try them again: * Fixed-shape ndarray columns get no lazy null predicate. Row-level is not enough -- blosc2.where(pred_(n,1), nan, values_(n,3)) returns (n,1), silently dropping the item dimension from the values, and combining an (n,1) predicate with the (n,) row mask NullableExpr reductions use explodes to (n,n). They get their null handling from the NumPy reduction paths instead. * The summary-index min/max shortcut stays disabled for mask columns. Letting them through on the grounds that their fill is NaN is defeated by NaN being a *value* under a mask: on [1.0, nan, 5.0, null, 3.0] the scan gives nan while the summaries would answer 1.0/5.0, so the same query would depend on whether an index exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This is what the design is for. A sentinel steals a value from the dtype's range, so importing Arrow data has always been lossy for any column whose data happens to use that value -- and silently so, since nothing raises. Measured, and now pinned side by side with the mask result: pa.array([-128, None, 127], int8) -> [None, None, 127] pa.array(["", "__BLOSC2_NULL__", None]) -> ["", None, None] Under mask storage both come back exactly as they went in, as do nullable bool without the 255 reservation, full-range uint8, float64 carrying nan/inf/-0.0 as values, timestamps holding int64.min as a value, and fixed-width string/bytes fully occupying their declared width. Ships opt-in: null_storage="mask" on from_arrow/from_parquet, or a NullPolicy. The default stays "sentinel" until phase 9. The import error for a type with no available sentinel is not deleted -- it still fires for sentinel storage, which still cannot represent those types -- but it now names the way out. Two departures from the plan: * No np.packbits. arrow_slice already had the better answer for its sentinel path: hand the booleans to pyarrow and take the resulting array's data buffer. Arrow packs booleans and validity bitmaps identically, so this borrows pyarrow's own packing and makes the bit order unrepresentable-as- wrong rather than merely tested. The literal-bytes test was still worth writing and pins 0xDD, not the MSB-first 0xBB. * The round-trip contract cannot be spelled `.equals()`. pyarrow compares floats with IEEE semantics, so two identical arrays containing NaN compare unequal. The tests compare what is observable instead: the validity bitmap and the values under valid rows, NaN equal to NaN, signed zeros distinct. Values under valid=False are deliberately not compared -- the fill is not part of the format contract. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A mask column's null rows hold the column's fill, and a fill is an ordinary
value to anything that only reads the values. So sort_by ordered nulls by
their fill (first, ascending, inverting the nulls-last contract), group_by
merged them into the genuine 0 / "" group and reduced over the fill, and
where() matched them whenever the fill satisfied the leaf.
Sort: the lexsort null-indicator key is now built from the sidecar, where for a
sentinel it was only a refinement of the value key; the FULL-index partition
reads one byte per row instead of the whole column (3.0x for U16, 1.2x for
int64); utf8 rank indexes stamp nulls with null_rank, and one built before that
is marked stale via null_aware, since neither O(1) signal can see the
difference. sorted_slice declines for a mask column with nulls -- only the
sidecar separates a null from a real fill, and the window is indexed by sorted
position -- and falls back to the mask-aware sorted view.
Groupby: _null_mask takes validity for value columns; a key column gets a
reserved null code instead, via _CodedKeyChunk, so its nulls form a group of
their own keyed None rather than joining whatever its fill was. The Cython
paths defer to the generic one, which is what the dense single-key path staying
(20.4 ms vs 91.8 ms on a 2M-row sum) buys back.
Also the mask half of Phase 1, which had never been done: _rewrite_null_predicates
and nullable_indexed both tested for NULL_SENTINEL and skipped everything else,
so where("a < 500") returned every null on the scan and an ordered index over a
float column returned every null for "f > 0.5", the NaN fill sorting into the
range it hands back. Verified against a NumPy SQL oracle over 32 combinations.
Three pre-existing storage-independent bugs fixed on the way, each reachable
only because mask storage widens what a nullable column can be: descending sort
of a bool column raised, descending sort of a full-range signed int put its
minimum last, and groupby min/max over a bool column always answered False.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
convert_nulls() moves a nullable column's nulls between an in-band sentinel and a .notnull sidecar, in either direction, for every V1 kind. It is never called implicitly: opening, copying and saving all preserve what a column already uses. Going back to a sentinel refuses what it cannot represent -- a column whose data already holds the proposed value, an int8 using all 256 -- and names the value in the way, rather than silently relabelling a real row as null. Every such reason is decided before a byte is written, so a refusal leaves the table untouched. The in-place ordering the plan recorded turns out to be wrong at its middle step: rewriting the null slots to the fill *before* the schema flips leaves a sentinel column reading a fill 0 as the value 0. Moving it last makes every intermediate state correct rather than merely recoverable -- sidecar, then schema, then fill, and the reverse for the other direction. Both are asserted. A dtype change is the one real limit: bool (uint8 <-> np.bool_) and a string too narrow for its sentinel need the stored array replaced, and no ordering of that write and the schema update survives a crash in between. Those refuse for a persistent inplace=True and point at inplace=False, which has no such window. Docs gain a "Where nulls are stored" section and a rewritten null-policy resolution order; info() now tags each column with where its nulls live. Three storage-independent bugs fixed on the way. copy() recorded its write watermark one below the exclusive bound every other writer means, so add_column() on a copied table backfilled one row short -- and raised outright for a variable-length column. _unflip_mask_bool_dtype keyed off the dtype rather than off whether the flip had happened, so a nullable uint8 ndarray column came back as bool_, every byte truncated to a flag. And copy() shares its schema object with the source, which conversion -- the one operation that mutates a spec in place -- was quietly reaching back through. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A bare nullable=True now resolves to mask storage, as does every nullable column inferred from Arrow, Parquet or CSV. Lossless round-trip is the reason the sidecar exists, so it is what a newly created column should get: to_arrow o from_arrow is now the identity for nullable bool, full-range int8/uint8, floats holding nan/-0.0 as values, free-text utf8, and timestamps holding int64.min -- none of which a sentinel can represent, because a sentinel has to steal one. Nothing on disk changes. The default governs creation only; opening never re-resolves, so stored tables keep their storage, dtype and sentinel, and the rewrites for the reserved 255 stay permanently. A table with a mask column records schema version 3, which older readers refuse with a clear error. Not the one-line change the plan budgeted. NullPolicy.null_storage becomes tri-state -- None means unspecified -- because "the user asked for a mask" and "the user said nothing" have to stay distinguishable: setting a type-wide sentinel field still implies sentinel storage for the kinds it covers, and collapsing the two would have made NullPolicy(float_value=...) raise on the very release that flipped the default. bool_value is the one field that cannot imply anything, since 255 is both its default and the only value it may hold. Three gaps the flip exposed, all pre-existing and all fixed here: - CSV was sentinel-only. from_csv had nothing to put in an empty field and raised; to_csv compared against the sentinel and wrote a mask column's fill as if it were data. Both go through the sidecar now, in either direction. - `~` on a mask bool column selected its nulls: it inverted the stored False fill, where SQL says a null satisfies neither a predicate nor its negation. - The Arrow importer never consulted the type-wide sentinel inference, so NullPolicy(signed_int_strategy="max") meant one thing for a declared schema and another for an inferred one. The ~65 tests that wrote a sentinel literally to mean "null" now say null_storage="sentinel" and mean it; several gained a mask-side counterpart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An index summarises the column's physical array, so every nullable column looked to it like a column of data: a sentinel entered the per-segment extrema, and so did a mask column's fill. That is why min()/max() declined the summary shortcut for any nullable column except a NaN-sentinel float, whose nulls the builder already dropped as NaNs. The summary builder now takes a per-column validity provider -- one callable, answering from the sentinel for one storage and from the .notnull sidecar for the other -- and takes its extrema over the valid rows only. A segment with no valid row is flagged FLAG_ALL_NULL, set together with FLAG_ALL_NAN so that a reader knowing only the NaN flags still skips it. The descriptor records null_aware; an index built before this carries no such key, keeps the old bail, and is promoted by rebuild_index(). min()/max() then answer from the index for a nullable column of either storage -- 236x on a 20M-row int64. This is not a mask feature: the suite's canonical fallback case was an INT64_MIN-sentinel column, on the grounds that its sentinel *is* the block minimum, and it takes the shortcut now. Phase 5's whole-column bail for mask floats narrows to what it was really about: FLAG_HAS_NAN over the valid rows marks a genuine NaN, so such a column declines only when it holds one, which is where the scan poisons to NaN. Two things planned for this phase were deliberately not done, and the plan records why. null_order is not written: no index kind reorders nulls, so "last" would have been a promise nothing keeps -- the contract lives in _build_lex_keys. The build token is not bumped: an older index is conservative rather than wrong, and a bump would rebuild every stored index in the wild to buy a shortcut. Plus the indexed-OR lift, which the phase did not anticipate. The earlier bail generalised from "an ordered index never evaluates the predicate" to "OR over a nullable indexed column must scan", but the segment path is not ordered: it prunes blocks by their summaries and then runs the predicate through miniexpr, which has been null-aware per leaf since phase 1. Its global post-filter was not merely wrong for OR, it was unnecessary. That path now serves OR (1.61x on a 20M-row probe); the exact-position paths still bail. The OR test also parses the AST rather than searching for "|", which used to take the index away from `name == 'a|b'`. One cost, recorded rather than hidden: the per-block summaries folded during writes carry no validity, so a nullable column holding a null cannot use them and pays one decompression pass at close() -- 1.8 ms to 33.2 ms for a 20M-row int64. A nullable column with no nulls keeps that fast path untouched. Threading validity into _ColumnSummaryAccumulator is a named follow-up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous wording claimed a pre-4.10.2 reader refuses a mask table "with a clear error". It refuses it, but the message is the bare `Unsupported schema version 3`: the hint naming convert_nulls(to='sentinel') was added in the same release that can already read the file, so the reader that actually hits the error never prints it. Also records the two things a reader of this section needs and could not get from it: that version 1 is still what a table with no nullable column stamps, so the compat break is scoped to tables that use the feature; and the process-wide opt-out via NullPolicy, which covers the inferred-schema paths (Arrow, Parquet, CSV) where nobody types a nullability keyword at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The default null storage flips in this release, which is a minor bump rather than a patch one. Retargets every reference: the version string, the release notes heading and its compatibility paragraph, the "since" notes in doc/reference/ctable.rst and CTable's docstrings, the plan's deviation note, and three test docstrings. Also carries the ctable.rst half of the correction made to RELEASE_NOTES.md in the previous commit: an older reader refuses a version-3 table with a bare "Unsupported schema version 3", not with the hint naming convert_nulls, which ships in the same release that can already read the file. Adds the scoping sentence there too -- a table with no nullable column still records version 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
NumPy 2.5 deprecated the generic timedelta unit, and a bare
np.datetime64("NaT") goes through that path, so under `filterwarnings = error`
the construction fails before the library is reached. Ubuntu CI on Python
3.14 caught it; NumPy 2.4.6 does not warn, which is why it passed locally.
The warning is raised in the *caller*, not in blosc2: detection is np.isnat,
which never cared about the unit. Reproduced against numpy 2.5.1 and pinned
by a second test that writes a NaT of each unit, so the unit-independence is
asserted rather than assumed. The first test takes its unit from the spec so
it tracks the column instead of a literal.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test_group_by_agrees_with_sentinel_storage[False-keys2] was the slowest test in CI at 4.46 s. None of it was blosc2: the table is 400 rows, and the helper called as_list(g[name]) *inside* the row loop, so each of the seven result columns was materialized once per output row. as_list is itself O(rows), which made the comparison O(rows^2 * columns) -- and the two-key parametrization, having the most groups, paid the most. Hoisting the column reads out of the loop takes it from 1.08 s to 0.06 s locally (18x), and drops it out of the file's ten slowest tests. The assertions, the fixture and the data are untouched. Also records why or_table() in test_null_aware_indexes.py uses two million rows: the planner only prefers the index over the scan above a threshold measured between 200k and 400k rows, so trimming it would silently cost the "index was used" assertion its margin. It is cheap regardless -- the values are repetitive enough that the table and both indexes come to ~90 KB. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
blosc2.utf8() raises without numpy.dtypes.StringDType, so on NumPy 1.26 the seven null-storage suites this branch added took ubuntu CI down with 32 collection errors and 4 failures. The distinction that matters: a call inside a test body can be skipped, but a parametrize list is built at *collection* time, so the call raises before any skipif can act and the whole module dies. tests/ctable/utf8_compat.py carries the guard once -- HAVE_UTF8, a needs_utf8 mark, and utf8_spec() returning None where NumPy cannot build a spec. Sibling helper modules already have precedent (tests/b2view/test_basics.py imports tree_store_gen), and the alternative was the same block copied into seven files. Suites that are *entirely* about utf8 keep skipping at module level; this is for the ones where utf8 is one kind among many and the rest must still run. One case needed more than a mark: test_sentinel_storage_is_lossy_where_mask_ storage_is_not asserts that the utf8 sentinel eats a literal "__BLOSC2_NULL__", which is only true when there is a utf8 column. Without StringDType an Arrow string column imports as vlstring, whose nulls are native None, so nothing is lost and the premise does not hold. Verified by running the suite with numpy.dtypes.StringDType deleted, which reproduces the CI failure exactly: 8482 passed, 70 skipped there, and 8766 passed, 29 skipped with it present. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors CTable nullability from in-band sentinels to Arrow-style per-column validity sidecars (.notnull) and makes mask-backed nullability the default for newly created nullable columns, while preserving sentinel-based storage for existing persisted tables and for callers that explicitly request it.
Changes:
- Add
null_storage={"mask","sentinel"}across schema specs and propagate it through schema serialization/versioning (schema v3 only when mask storage is used). - Implement mask-backed null persistence (
.notnull) through storage backends, Arrow/Parquet round-trips, expression/reduction semantics, and groupby correctness. - Make column indexes null-aware by building segment summaries over valid rows, improving correctness and enabling summary-based min/max for nullable columns.
Reviewed changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ctable/utf8_compat.py | Adds a helper to safely include utf8-spec cases in parametrizations without failing test collection on NumPy without StringDType. |
| tests/ctable/test_utf8.py | Pins legacy utf8 nullable behavior to sentinel storage where needed and adds tests for default mask behavior on Arrow import. |
| tests/ctable/test_parquet_interop.py | Updates Parquet interop tests to validate mask-backed nulls by default and sentinel behavior when explicitly requested. |
| tests/ctable/test_nullable.py | Updates nullable policy tests to explicitly request sentinel storage when asserting sentinel resolution behavior. |
| tests/ctable/test_null_storage_schema.py | Adds tests for spec/schema plumbing of null_storage, schema version gating, and resolution order with NullPolicy. |
| tests/ctable/test_null_predicate_rewrite.py | Adds tests for SQL-like null semantics in string predicates via per-leaf predicate rewriting and index/scan agreement. |
| tests/ctable/test_null_persistence.py | Adds persistence tests ensuring .notnull sidecars are lazily created, correctly pinned to row grids, and preserved through storage operations. |
| tests/ctable/test_null_mask_expressions.py | Adds coverage for expressions/reductions over mask-backed nullable columns and intentional divergences vs sentinel storage. |
| tests/ctable/test_null_mask_arrow.py | Adds lossless Arrow/Parquet round-trip tests for mask-backed nulls (including tricky full-range/special-value cases). |
| tests/ctable/test_null_mask_api.py | Adds API-level tests for writing/reading nulls under mask storage (None writes, is_null, fillna, views, persistence). |
| tests/ctable/test_null_expressions.py | Pins tests that depend on sentinel-null float semantics to explicitly request sentinel storage. |
| tests/ctable/test_null_channel.py | Adds tests for the NullChannel abstraction and shared sentinel helpers, including compile-time resolution behavior. |
| tests/ctable/test_null_aware_indexes.py | Adds tests for null-aware segment summaries, descriptor null_aware behavior, and index-planned OR semantics over nullable columns. |
| tests/ctable/test_nested_metadata_root.py | Updates nested-metadata schema version tests to account for nullable-by-default Arrow fields now resolving to mask storage. |
| tests/ctable/test_groupby.py | Pins groupby fixtures that expect NaN-as-null (sentinel float) by explicitly requesting sentinel storage. |
| tests/ctable/test_ctable_ndarray_columns.py | Pins nullable ndarray tests to sentinel storage where legacy behavior is required (including bool ndarray sentinel widening). |
| tests/ctable/test_column.py | Updates indexed min/max expectations now that null-aware summaries allow fast-path min/max for INT64_MIN sentinel columns. |
| src/blosc2/version.py | Bumps development version to 4.11.0.dev0. |
| src/blosc2/schema.py | Introduces null_storage across relevant spec types, shared null metadata plumbing, complex nullability via mask, and fill_value_for for mask fills. |
| src/blosc2/schema_vectorized.py | Updates vectorized validation to correctly detect nulls for both sentinel and mask storage (including NA markers prior to batch splitting). |
| src/blosc2/schema_validation.py | Reuses shared sentinel/null helpers for schema validation, including ndarray null detection via sentinel_mask. |
| src/blosc2/schema_compiler.py | Computes schema version as a feature-max, bumping to v3 only when mask storage is present; improves unsupported-version error messaging. |
| src/blosc2/indexing.py | Adds validity-aware segment summary computation (including all-null segment flagging) and records null_aware in index descriptors. |
| src/blosc2/groupby.py | Updates groupby null handling to support mask-backed validity (including key recoding) and fixes bool min/max identity handling in generic paths. |
| src/blosc2/ctable_storage.py | Adds storage-backend APIs and key conventions for per-column .notnull sidecars; ensures rename/drop carry companion keys. |
| src/blosc2/ctable_indexing.py | Adds validity providers for null-aware index summary builds, improves OR detection via AST parsing, and integrates mask validity in null filtering. |
| src/blosc2/_utf8_array.py | Extends utf8 expression evaluation and Arrow export to respect mask-backed validity sidecars (in addition to sentinel nulls). |
| RELEASE_NOTES.md | Documents the new default mask-based null storage, interoperability guarantees, schema v3 gating, and null-aware indexes. |
| doc/reference/ctable.rst | Updates reference docs for Column.null_storage, mask vs sentinel semantics, conversion, and index behavior on nullable columns. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
tests/ctable/test_null_mask_arrow.py:116
- This case does not exercise the advertised
int64.mintimestamp:min + 1is an ordinary datetime, while exactlyint64.minis NumPy's NaT bit pattern. Construct the Arrow array asint64and cast it to timestamp so the minimum remains a valid Arrow value; this will also expose whether export preserves its validity instead of converting it back through NumPy NaT.
np.datetime64(np.iinfo(np.int64).min + 1, "us"),
doc/reference/ctable.rst:200
- This guarantee has an intentional exception in the implementation: converting a mask-backed float containing a valid NaN to the default NaN sentinel silently relabels that row as null (
test_a_nan_already_present_does_not_block_the_nan_sentinel). Document that exception here so callers do not rely on the stated refusal to prevent this semantic data change.
Going back to ``"sentinel"`` refuses what it cannot represent — a column whose
data already contains the proposed sentinel, or one with no value left to
reserve — and says which value is in the way, rather than silently relabelling
a real row as null.
An empty CSV field cannot mean both "" and missing, so a mask-backed text column wrote a valid "" exactly like a null, and from_csv read both back as null. Such a column now writes \N for a null and \E for a valid "", escaping any value already shaped like one. Reading text also stops calling strip() before testing for an empty field, so whitespace-only text stays a value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two silent-data-loss paths, both where nullity or row identity travels beside the values rather than in them. **CSV and utf8.** Both halves of the CSV path keyed off a fixed element dtype, which utf8 does not have: a utf8 Column reports StringDType() (kind "T", not the "U"/"S" a fixed-width text column reports), and its CompiledColumn reports None. So from_csv raised AttributeError on any utf8 column, nullable or not, and to_csv skipped the \N / \E escapes and wrote a valid "" exactly like a null. to_csv now gates on _csv_is_text_column; from_csv creates utf8 through create_varlen_scalar_column and fills it via _csv_text_col_to_cells, with the shared text decoding factored into _csv_decode_text. A kind CSV genuinely cannot represent now raises naming itself, instead of failing on None deep inside the conversion. A sentinel text column still reads an empty field back as its sentinel, so a genuine "" becomes null. That is long-standing string/bytes behaviour which utf8 now matches, and changing it would alter how blank fields in third-party CSVs are read; left alone deliberately. **extend() from a CTable.** The raw source._cols[name][:n_rows] slice got two things wrong. It dropped validity: under mask storage nullity lives in a sidecar, so every null arrived as its fill -- a plausible 0 or "" rather than an error -- where the sentinel and list-of-rows forms of the same rows both kept it. And it read physical slots by a logical count, so a source with deleted rows, or a sorted view, copied the wrong rows entirely (storage-independent, and wrong on main too). _batch_columns_from_table now hands validity over separately and gathers at live positions, keeping the plain slice for the dense case behind two O(1) checks. tests/ctable: 2189 passed, 1 xfailed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A mask column can record "missing" without reserving a value, but add_column had no path to it: values=[1, None, 3] and default=None both died in NumPy's astype with "int() argument must be a string, a bytes-like object or a real number, not 'NoneType'", naming neither the column nor a way out. So the one operation that adds a column to a populated table could not be honest about the rows that predate it -- under a sentinel you write the sentinel, under a mask there was nothing to write. _add_column_values now splits nullity out of values= before validating and casting (a null cell has no value to constrain, exactly as extend already argues), a default of None backfills the live rows as null, and the sidecar is written last, once the column and its schema entry are both in place for _ensure_null_mask to pin its row grid. A batch with no null still writes no sidecar. The null-detection rules moved to ctable_nulls.split_batch_validity so add_column and NullChannel.coerce_batch cannot drift: they have to agree on MaskedArray, the NA singletons, NaT, and NaN-is-a-value, but reach it from different directions -- the channel has a live column to ask, add_column only has a spec. fill_item_for is the matching spec-level fill. Two things found on the way: * add_column silently corrupted *every* timestamp column, nullable or not. It cast datetime64 straight to int64 without going through the spec's unit, so a datetime64[s] value landed in a microsecond column and read back as 1970-01-01T00:26:17. extend has always converted correctly; that conversion is now _timestamps_to_stored_int64 and both call it. * A None that genuinely cannot be stored now says which column and what to do instead, from the astype failure path so a well-formed batch pays nothing for the check. Sentinel utf8 is untouched: _coerce maps None onto the sentinel there, and that already worked. tests/ctable: 2203 passed, 1 xfailed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Mask storage is a second implementation of nullability, and a second implementation of anything is a standing invitation to drift. The mask-based-nulls plan named a differential oracle its single highest-value test and it was never written; writing it turns "mask is a second implementation" from a permanent liability into a checked invariant. Each case builds the same logical rows twice, once per storage, and asserts the same answer from is_null, notnull, null_count, to_numpy(masked=), fillna, dropna, unique, value_counts, sort_by (both directions, single and multi-key, and as a view), every reduction, argmin/argmax, group_by (as key with dropna both ways, and as value), where (eight expression shapes including OR and negation, indexed and not), and to_arrow -- across every V1 kind. Comparison is always logical: a column is read as its values with None wherever is_null() says so. What sits under a null is the fill for one storage and the sentinel for the other, and decision 5 says that is not part of the format contract, so comparing it would pin down something the design says may change. Three divergences are deliberate and asserted as divergences: NaN is a value under a mask and the null under a sentinel (decision 6), a nullable bool is uint8 only under a sentinel, and complex is mask-only. Two are bugs, pinned as strict xfails so that fixing either trips this suite -- isin() and to_pandas() both read raw values and so match, or emit, whatever stands in for a null. Those xfails are scoped to the kinds where the leak is *visible*: a float fills with NaN and reserves NaN, a timestamp fills with int64.min and reserves it, so for those the two storages leak indistinguishable values and the correct behaviour is asserted outright. Beyond cross-storage agreement, each storage is also checked against itself indexed versus unindexed -- agreement alone would not catch an index that leaks nulls into both storages the same way. Verified non-vacuous by mutation: sorting mask nulls first fails 30 cases, under-reporting null_count fails 10, and withdrawing the validity guard from queries (the plan's Addendum 2 bug) fails 42. tests/ctable: 2470 passed, 15 xfailed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
isin() read col[:] and tested membership on the raw values, so a null row
matched whatever occupies its slot: the fill under mask storage, the
sentinel under a sentinel one. Neither is the row's value -- the fill is
explicitly not part of the format contract, and a sentinel is a value the
column promises never to mean literally -- so col.isin([0]) returned True
for a null in an int column that holds no zero, and isin([""]) did the
same for text.
Membership is now asked of the row's value, and a null row has none, so it
matches nothing. Nullity comes from the null channel, which makes this
right for both storages and for a view's own rows at the same time.
None in the values is how you select the nulls, matching exactly what
is_null() reports; pandas.NA and NaT are accepted as spellings of the same
request. That is what dictionary columns have always done with their
reserved code, so the three kinds now answer alike rather than two of them
disagreeing. A float NaN is deliberately not a null marker: under mask
storage NaN is an ordinary value (decision 6).
The equivalence oracle's isin xfail comes off, and two cases join it there
-- None selecting exactly the nulls, and None beside a real value getting
both. One wrinkle worth recording: a timestamp's fill decodes to NaT,
which *is* a way of spelling missing, so probing with it legitimately
selects the nulls rather than nothing.
Left alone, as a separate concern: isin([float("nan")]) does not match a
genuine NaN, because Python set membership compares NaN unequal to
itself. That predates this change and has nothing to do with nulls.
tests/ctable: 2505 passed, 8 xfailed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
to_pandas() wrote the raw values, so a null arrived as whatever occupies its slot -- the fill under mask storage, the sentinel under a sentinel one. A nullable int column came out holding 0 or -9 as ordinary data, where to_arrow() on the same table was already exact. A null now becomes pandas NA, and a column holding one is given a dtype that can say so: Int64/UInt8/... for integers, boolean for bools (both storages, since a sentinel bool is only physically uint8), NaN/NaT where the NumPy dtype already has one, and object with None for text, bytes, complex and ndarray cells. A column with no null is returned untouched, so no dtype moves under anyone; only a column that could not previously be represented changes, which is the same data-dependent widening pyarrow.Table.to_pandas does and for the same reason. The dictionary and variable-length kinds already materialised their nulls as None and are left alone. from_pandas had to keep up, or a table would not have survived its own round trip: to_numpy(dtype=...) raises on every NA-carrying extension dtype to_pandas now emits. It stays the fast path, and a series holding a missing value falls back to handing cells over one at a time for the write path to split, with a sentinel column taking its sentinel. That fallback also fixes two silent corruptions that predate this work: a missing cell bound for a bytes or string column was coerced by to_numpy() into the *string* b'None' / 'nan' rather than rejected. Verified: 18 of the 20 kind/storage combinations now round-trip through pandas losslessly. The two that do not are mask-backed float, and that is a limit of the destination -- pandas has no float dtype separating NaN from missing, since even Float64 folds a NaN into NA -- so it is documented on to_pandas, pinned by a test, and pointed at to_arrow. The equivalence oracle's last xfail comes off; it now passes clean, with the pandas round trip added to it. Re-checked non-vacuous by mutation: reverting to_pandas to raw values fails 14 of its cases. tests/ctable: 2542 passed, 1 xfailed (the unrelated operator-form negation leak). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
cov() already dropped nulls listwise, and already meant to: the comment at the site says so. It just computed nullity with _null_mask_for, the in-band sentinel test, which a mask column has no sentinel to answer -- so every null read as an ordinary value and the fill was averaged into the result. On [(1,2),(null,null),(3,6)] that gave 2.33 where a table holding the same rows without the null gives 2.0. Nullity now comes from the null channel: the sidecar for a mask column, and the same in-band test as before for a sentinel one, answered from the values already read rather than by reading the column again. The dictionary and variable-length kinds cannot reach it, having been rejected on dtype further up. Listwise is kept, not changed to the pairwise dropping pandas does: it was the existing intent, and it keeps every entry of the matrix computed over one set of rows, so the result stays consistent with itself. That was documented only in a comment, and is now in the docstring where a caller will find it, along with the pandas difference. A mask float column's NaN is still data (decision 6), so it propagates into the result rather than dropping the row -- the same answer sum() already gives, and asserted rather than left to be discovered. Added to the equivalence oracle, which checks both storages against each other and against a table holding the same rows with the null one simply absent. Non-vacuous by mutation: restoring the old blind spot fails 7 of the 8 new cases. tests/ctable: 2550 passed, 1 xfailed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test_to_arrow_agrees called to_arrow() with no importorskip, so a CI job without pyarrow got ten ImportErrors instead of ten skips. The pandas cases in the same file were guarded; the Arrow one was simply missed. Also guards the to_arrow assertion at the end of test_a_mask_float_cannot_round_trip_through_pandas, which is a pandas test that reaches for Arrow to show what pandas loses. It only escaped the same failure because that job has no pandas either, so it skipped before getting there -- a job with pandas and without pyarrow would have failed it. Verified by making the two modules unimportable: the whole ctable suite is 2191 passed, 103 skipped, 0 failed without either of them, and unchanged at 2550 passed with both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
A nullable CTable column now keeps its nulls in a per-column .notnull validity sidecar — Arrow's own model — instead of reserving a value from its own range, and that is what a bare nullable=True resolves to.
A sentinel is lossy by construction: a nullable int8 could not hold -128, free-text utf8 had no safe sentinel at all, bool(nullable=True) silently became uint8 with a reserved 255, and Arrow columns whose type had no value to spare could not be imported. With a side channel, to_arrow(from_arrow(x)) returns x for nullable bool, full-range int8/uint8, float64 holding nan/±inf/-0.0 as values, utf8 holding "" and "BLOSC2_NULL", and timestamp holding int64.min — none of which survive a sentinel. complex becomes nullable at all.
Sentinel storage is supported indefinitely and is one keyword away (null_storage="sentinel", any explicit null_value=, or a NullPolicy field). Nothing on disk changes: opening a stored table never re-resolves anything, so every existing table keeps the storage, dtype and sentinel it was written with. A table containing a mask column records schema version 3, which older readers refuse with a clear error rather than misreading. convert_nulls() moves columns between the two in either direction, never implicitly.
Landed in ten phases, each independently reviewable, ending with null-aware column indexes: per-segment extrema are taken over the rows that carry a value, so min/max answer from the index for a nullable column (236x on 20M rows) and where() with an OR over one uses the index instead of scanning.
Ten bugs were found and fixed on the way that have nothing to do with mask storage and are reachable today — descending sort_by on a bool column raised and on full-range signed ints ordered wrongly; group_by min over a bool column always returned False; add_column() after copy() backfilled one row short; and t.where("a > 10") returned a nullable column's nulls as matches on both the scan and the index paths.
plans/mask-based-nulls.md is the reference document for the design and carries the as-built record, including nine premises the implementation disproved. ~350 new tests across seven files; full suite green (8765 passed).