fix: bulk shard decode must only engage for identity full reads - #256
Closed
d-v-b wants to merge 1 commit into
Closed
fix: bulk shard decode must only engage for identity full reads#256d-v-b wants to merge 1 commit into
d-v-b wants to merge 1 commit into
Conversation
d-v-b
force-pushed
the
fix/bulk-shard-decode-identity-gate
branch
from
July 29, 2026 11:46
9ea327a to
cef4415
Compare
d-v-b
force-pushed
the
fix/bulk-shard-decode-identity-gate
branch
from
July 29, 2026 13:29
cef4415 to
2287a0b
Compare
7 tasks
…x density check The FusedCodecPipeline's vectorized whole-shard decode accepted any indexer without `sel_shape` whose output shape matched the shard shape. An OrthogonalIndexer from `arr[perm, :]` / `arr.oindex[...]` satisfies that, so reordering or duplicating fancy-index reads on uncompressed, crc-free sharded arrays silently returned the shard in natural order. The bulk path now requires an identity full read: one whole-dimension, step-1 slice per dimension (structural, not a BasicIndexer type check, since `arr[:]` arrives as an OrthogonalIndexer). Also: - decline structured dtypes in the bulk path, which lacks the Struct byte-order branch of BytesCodec._decode_sync (latent until zarr-developers#3054 is fixed); - `_ShardIndex.is_dense` now requires offsets to exactly tile the data section instead of merely being unique, so corrupt indexes with overlapping or out-of-range offsets (e.g. pointing into an index_location='start' index region) cannot be served as array data. Assisted-by: ClaudeCode:claude-fable-5
d-v-b
force-pushed
the
fix/bulk-shard-decode-identity-gate
branch
from
July 29, 2026 14:32
2287a0b to
42241ba
Compare
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
1 participant
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.
🤖 AI text below 🤖
Fixes the second critical from the PR zarr-developers#3885 audit, plus two hardening items in the same code. Only the opt-in
FusedCodecPipelinereaches this path._decode_full_shard_bulk_if_uncompressedgated onsel_shape is None and indexer.shape == shard_spec.shape.OrthogonalIndexer— produced by plainarr[perm, :]andarr.oindex[...]— has nosel_shape, so on uncompressed, crc-free sharded arrays the bulk path fired for reordering and duplicate fancy-index reads and returned the shard in natural order: silent wrong data for multi-shard permutation reads and duplicate-index selections. (The earliersel_shapegate only stoppedvindex/CoordinateIndexer.)The gate is now a structural identity-full-read predicate over the indexer's
dim_indexers— every dimension must be a fullSliceDimIndexer— deliberately NOTisinstance(indexer, BasicIndexer), sincearr[:]produces anOrthogonalIndexerand a type gate would silently disable the fast path for the common full read. Spy-based engagement tests pin both directions: the bulk path still fires for full reads (single- and multi-shard) and never for permutation/duplicate/subset selections.Also in this PR:
_ShardIndex.is_densenow requires offsets to tile the data section exactly; corrupt shards with unique-but-overlapping or out-of-range offsets (which could previously return index bytes as array data withindex_location="start") are rejected. Separate error-case tests per failure mode.BytesCodec, so it's gated off before that latent divergence can become live when the pre-existing struct+sharding crash (sharding codec use of lru caching fails with numpy void scalars zarr-developers/zarr-python#3054) is fixed.oindex(previously 1-D pure permutations only — exactly why CI missed this).21 new/parametrized regression cases fail pre-fix; 533 passed post-fix on the verification suites; reviewer independently exercised multi-shard and edge-shard engagement scenarios live.
🤖 Generated with Claude Code