Repository navigation
feat: give every null an equality term and let an order space place nulls first or last - #503
Merged
Merged
Conversation
…nger shows which values were absent A sealed None is now as long as a sealed Some, but the equality index still gave a null no term, so the term column held a SQL NULL exactly where the value was absent. Every null, typed or plain, now has a term under the new frozen domain vitaminc/prf/value/null/v1, whose bytes are the one byte 0x00. domain_of and normalize return it, the Terms sink pushes it at the null's path, and a typed null is the same term as a plain one. A boolean and an undefined still have no term. Also adds NormalizeError::Reserved, which the order index reports for the value it keeps for nulls. NormalizeError is #[non_exhaustive], so the new variant is not a breaking change. BREAKING CHANGE: a walk over a value with a null now returns one more equality term, and normalize(None, Physical::Null) now succeeds instead of returning NoDomain. A layer that must keep a SQL NULL for a column drops the term whose domain is domain::NULL. domain::ALL has nine labels.
…with nulls can be answered from the index Without a policy an order space gives a null no term, so the term column shows exactly which rows were absent. A space can now take a NullOrder, fixed when it is made like its target and context: scheme.target(I32).nulls(NullOrder::Last), or on a runtime target space.nulls(..), which returns NoDomain for text and bytes rather than make a space every encryption refuses. On a marker target, nulls compiles only for the thirteen fixed-width markers (the new sealed FixedWidth trait), and the space it returns is of the target Nulls<T>, so its terms do not compile against the plain space's. In a space with a policy every null, typed or plain, encodes to the sentinel: all 0x00 for nulls-first, all 0xFF for nulls-last, at the target's width. Integer and uuid codes move up by one under nulls-first, so zero, the minimum and the nil uuid stay encodable, and under both policies the target's maximum is refused with the new NormalizeError::Reserved. Float, date, timestamp and decimal codes never reach either extreme and move nothing. An undefined stays NoDomain under every policy. Each policy is a new frozen label, vitaminc/ore/value/<kind>/nulls-first/v1 and .../nulls-last/v1, twenty-six in all, and domain::ALL lists all forty-one. The salt binds the label, so a nulls-last term never compares with a plain-space term of the same target. OrderPlaintext, OrderTerm and QueryTerm learn nulls(); order_with_nulls encodes for a policy space; encrypt_null and null_query give the sentinel's term and query, so a layer can answer IS NULL and bound a range. test_support checks every fixed-width target in a nulls-first and a nulls-last space, so a scheme meets an all-zero and an all-ones plaintext. Nothing in OrderScheme changes, and no plain-space byte changes. BREAKING CHANGE: Order is no longer a tuple struct with a public field. Write Order::new(target) where Order(target) was written, and Order::new(target).nulls(policy) for a walk that places nulls.
…and keeps apart from the plain space Through the mock scheme: a null, typed or plain, is one term that sorts below every value under nulls-first and above under nulls-last; the target's maximum is refused with Reserved at any width, and the maximum uuid too, while a float target refuses nothing; a plain space still refuses a null; the three spaces of one target have three salts, and a nulls-last term keeps the plain bytes but not the plain salt; a runtime term of one policy compares as None with one of the other; a scheme takes a policy space exactly when it takes the target, and its refusal names the policy's label; the null's query finds the nulls and bounds age < 10 above the sentinel. The walk with Order::new(target).nulls(..) gives the null its sentinel at the path where equality has its term, and stops at the maximum. Compile-fail cases: nulls on a Text or Bytes marker, encrypt without a check on a space that places nulls, and a term of such a space in the plain space's comparison.
…veals, and how a range must skip the sentinel The One space section now shows a space that places nulls, the one value it refuses, why text and bytes take no policy, and that a policy is a space of its own. It states the leakage in the decision's words: in any order index whose stored terms compare with each other, the nulls are visible as the tied group at one end, and in a sparse domain as the group that differs from every value at the top of the encoding. The query material says that IS NULL is equality with the null's query and that a range in a space with a policy must exclude the sentinel. The Encodings section lists the nulls-first and nulls-last labels and bytes, and why the float, date, timestamp and decimal extremes are never reached.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…served means in plain words The ADR's walk example now shows that nulls on a runtime target returns a Result, and records that the policy is part of the space's type. The Reserved error says that the value's code is the space's null sentinel, rather than naming the index in a roundabout way.
Contributor
🧬 Mutation testing (cargo-mutants,
|
| caught | missed | unviable | timeout |
|---|---|---|---|
| 74 | 0 | 59 | 0 |
✅ Every mutant in the changed lines was caught by a test.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1849b9c042
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
✅ No CRAP threshold violations1376 function(s) analyzed · threshold 30 |
…me does not take `encrypt_null` and `null_query` went straight to the scheme without the runtime support check every other path makes. On a space whose target was chosen while the program runs, a fixed-width scheme was handed a plaintext of a width it does not take: a four-byte scheme opened on `I64` with nulls placed got an eight-byte sentinel. On a marker target the compiler has already checked the scheme takes the target, so those methods still return only the scheme's own failure. On an `OrderTarget` they now check first and return `OrderError`, with `Unsupported` naming the policy's label, as `encrypt_checked` of a null does.
…newer clippy Clippy 1.98 adds `chunks_exact_to_as_chunks`, which fails `cargo clippy --all-targets -- -D warnings` on this test for anyone on a current toolchain. `slice::as_chunks` is stable in Rust 1.88, the workspace's minimum, so the test uses it instead of allowing the lint. It counts the same words: the tail shorter than eight bytes is still skipped, as `chunks_exact` skipped it.
…does not depend on the value `make_room` checked whether a value's code was the sentinel with `iter().all`, which stops at the first byte that differs. How long it took then said how many leading bytes of the code were `0xFF` (or `0x00`): for an `i32` in a nulls-last space, whether the value is within 2^24 of the maximum. The fixed-width encoders it runs after take the same time for every value, and the decimal encoder says so in as many words. The check now ORs together each byte's difference from the sentinel byte and compares once at the end, so every byte is read whatever it holds. The answer is still reported as `Reserved`, which is the only thing it should say about the value.
… gaps mutation found The mutation gate on this PR left four mutants alive in the ore crate, and reading the tests turned up more a careless edit could slip past: - The twenty-six new labels were checked only against a rule built from the plain labels, so renaming a plain label would have renamed its policy labels with it. `domain::ALL` is now written out in full. - The sentinels were built from `width()`, and the nulls-first shift was pinned with a carry across one byte only, so an `increment` that stopped carrying after two bytes passed every test. Each sentinel is now written out, and carries across three, eight, and all sixteen bytes are pinned, for a uuid too. - The NaNs whose orderable bytes would be all `0x00` or all `0xFF` without the fold are now named, at both float widths, and a second quickcheck draws only NaNs and infinities, which a uniform bit pattern hits about once in two thousand draws. - `QueryTerm::nulls`, the `Supports` answer for a target with a null policy, and the conformance rows that place nulls had no test that failed without them. A query's policy is asserted, the `Supports` answer is tested directly, and a mock that mishandles only the all-ones timestamp, a width no plain row reaches, must fail the conformance checks by the nulls-last label. - Both policies of a marker target share one Rust type, so a term of one can reach the other's comparison. That it compares as `None` was tested for a runtime target only; it is now tested for a marker, and for a query against a term.
…tion gate The gate failed this PR on two mutants of `null`, which replace its body with `Protected::new(vec![0])` and `Protected::from(vec![0])`. Both return exactly what `null` returns, the one byte `0x00` in a protected vector, so no test can tell them apart. They are skipped by name, as the other equivalent mutants are, and the rest of `null`'s mutants stay gated.
…es it one The README still listed a null among the scalars `EqualityTerm` refuses with `PrfError::Normalize`. Since the normalizer gives every null a term under `vitaminc/prf/value/null/v1`, `EqualityTerm::new(None, Physical::Null)` derives one. The rustdoc on `EqualityTerm` already says so; the README is the crate's front page and now agrees.
…me cannot answer for every target Both policies of a marker target are one Rust type, so a nulls-first and a nulls-last term can reach one comparison. `OrderTerm` and `OrderSpace::compare` said a marker term could not meet another target's, which is true, but not that the policy is stored and two policies compare as `None`. They say so now. `Supports` now says the crate answers for a target in a space that places nulls, and that this is why a scheme cannot implement `Supports` for every `T: Target` at once: such an impl conflicts with the crate's. The ADR still described `order` taking a policy, `OrderTarget::domain` gaining one, and `encrypt_query` accepting a null. It now names `order_with_nulls`, `domain_with_nulls`, `encrypt_query_checked` and `null_query`, which are what shipped. A few rustdoc and README lines that ran past the wrap width are rewrapped.
…s, as on encrypt_checked `encrypt_null` and `null_query` on a runtime target return `Result<OrderTerm<.., Nulls<OrderTarget>>, OrderError<..>>`, which clippy's `type_complexity` refuses under `-D warnings`. The crate already allows it on `encrypt_checked` for the same shape, and a type alias would hide the target in the one signature where it matters.
… and normalizer already do `OrderSpace` and `Order` could not call their getter `nulls()`, because `nulls(..)` is the builder there, so they named it `null_order()`. The plaintext, the term, the query, the `Target` trait and `Nulls` still said `nulls()` or `order()`, so one question had three names depending on what a caller held. They all say `null_order()` now. Nothing here has been released, so there are no aliases.
… policies' terms cannot meet A nulls-first and a nulls-last space of one marker target were both `Nulls<I32>`, so a term of one compiled in the other's `compare` and the mistake showed only as `None` while the program ran. The policy of a marker space is now a marker too, `First` or `Last`, and the space's target is `Nulls<T, P>`: `scheme.target(I32).nulls(Last)`. A term of the other policy is a type error, as a term of another target already was. `NullPolicy` is the sealed trait the two markers implement; its `ORDER` gives the `NullOrder` each stands for. A layer that reads its column types from a schema chooses the policy while the program runs, so `nulls(NullOrder)` on an `OrderTarget` space keeps the enum and its `Result`, and still refuses text and bytes with `NoDomain`. It now makes an `OrderSpace<S, RuntimeNulls>`, its own sealed target type holding the `OrderTarget` and the `NullOrder`, whose terms carry their policy and compare as `None` across policies as before. A scheme that supports a target supports it under both marker policies and the runtime type, through the crate's blanket impls. A new compile-fail case shows a `Nulls<I32, First>` term refused by a `Nulls<I32, Last>` space's `compare` and `compare_query`, and the runtime tests keep the cross-policy `None` checks, now with the query side as well.
…untime space's a value Section 2 described the policy space as `OrderSpace<S, Nulls<T>>`, with both policies of a target one type. It now says that on a marker target the policy is `First` or `Last` and part of the type, `Nulls<T, P>`, so a nulls-first term does not compile against a nulls-last space, and that on an `OrderTarget` it stays a `NullOrder` value in a `RuntimeNulls` space, for a layer that chooses it while the program runs.
…et on Mutation found that `Supports<Nulls<T, P>>` could answer `true` without a test failing, because every scheme in the tests takes every marker it names. A scheme with a hand-written `Supports<U32>` that refuses now shows the answer carries through under both policies. `Nulls::target` returning `Default::default()` survives too, and cannot be caught: the target is a fixed-width marker, a zero-sized type with one value, so it is that value either way. It is skipped by name in `.cargo/mutants.toml` beside the other equivalent mutants.
cipherstash-bot
left a comment
cipherstash-bot
left a comment
There was a problem hiding this comment.
Recommendation: 🟢 merge as it is (1 of 4 review job(s) failed)
No change is needed before merge. One test gap can wait for a follow-up: the query side of the null-policy check in encrypt_query_term has no test. All sources found no correctness problem.
How this review was made
| Agent | Model | Review type | Result |
|---|---|---|---|
| claude | claude-opus-4-8 | rust | 0 found, 0 posted |
| claude | claude-opus-4-8 | test-gap | 1 found, 1 posted |
| codex | gpt-5.6-terra | rust | 0 found, 0 posted |
| codex | gpt-5.6-terra | test-gap | failed |
Synthesis: claude-sonnet-5-5 merged the findings, removed duplicates and dropped findings it could not confirm in the code. 0 posted finding(s) were raised by two or more models.
Plain language: claude-sonnet-5-5 read every comment as a new reader would. 1 comment(s) had a problem that stopped the reader acting; it rewrote 1.
Stack: position 3 of 3 (#497, #499, *️⃣ #503). *️⃣ marks this pull request.
Context loaded: the description, 1 linked issue(s) and 5 discussion entries.
…r null policy, as a term space does The query side's domain check had only negative tests across targets, which a check on the target alone would also pass. Feed a nulls-last I32 space a plain I32 plaintext and a nulls-first one, and expect Mismatch for both.
coderdan
force-pushed
the
feat/null-index-terms
branch
from
October 21, 2026 05:39
2fe824b to
97dbe4d
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
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 sealed
Noneis now as long as a sealedSome(#497), but the index terms beside it still gave a null no term, so the term column showed exactly where a value was absent. This PR implements the decision in ADR 0002 on this branch. It closes item 5 of #493, the one part the typed-null stack left open.What changes for a caller
vitaminc/prf/value/null/v1, whose bytes are the one byte0x00.normalize(None, Physical::Null)returns it, and a walk pushes it at the null's path. There is no setting for this. A layer that must keep a SQLNULLfor a column drops the term whose domain isdomain::NULL. A boolean and anundefinedstill have no term.scheme.target(I32).nulls(Last)on a marker target,scheme.target(OrderTarget::I32).nulls(NullOrder::Last)?on a target chosen while the program runs, orOrder::new(OrderTarget::I32).nulls(NullOrder::Last)for a walk. On a marker target the policy is part of the space's type:FirstandLastare markers in thetargetmodule, and the space is anOrderSpace<S, Nulls<I32, Last>>, so a term of a nulls-first space does not compile against a nulls-last space'scompareorcompare_query, as a term of another target does not. On anOrderTargetthe policy stays aNullOrdervalue, for a layer that reads it from a schema, in anOrderSpace<S, RuntimeNulls>; every term carries it, and a term of one policy compares with a term of the other asNone. Without a policy nothing changes. With one, every null encodes to the space's sentinel: all0x00for nulls-first, all0xFFfor nulls-last, at the target's width. Only the thirteen fixed-width targets take a policy. On a marker,nullsdoes not compile forTextorBytes. On anOrderTargetit returnsNoDomainfor those two. Each policy has its own labels (vitaminc/ore/value/<kind>/nulls-first/v1and.../nulls-last/v1, twenty-six new ones), and the salt binds the label, so a policy space's terms never compare with the plain space's.encrypt_nullandnull_querygive the sentinel's term and query, asencrypt_checkedandencrypt_query_checkedof a null do, so a layer can answerIS NULL, and a range in such a space must exclude the sentinel. On a target chosen while the program runs,encrypt_nullandnull_queryreturnOrderErrorand refuse a target the scheme does not take withUnsupported; on a marker they return only the scheme's own error.NormalizeError::Reservedunder both policies. Under nulls-first every integer and uuid code moves up by one, so zero, the minimum and the nil uuid stay encodable. Float, date, timestamp and decimal codes never reach either extreme, so they move nothing and refuse nothing.What a placed null reveals
In any order index whose stored terms compare with each other, the nulls are visible as the tied group at one end, and in a sparse domain as the group that differs from every value at the top of the encoding. That is never more than the SQL
NULLit replaces. The ore README says this where the policy is set.Breaking changes
vitaminc-normalize: a walk over a value with a null now returns one more equality term, andnormalize(None, Physical::Null)succeeds instead of returningNoDomain, anddomain_of(None, &Physical::Null)isSome(domain::NULL)instead ofNone.domain::ALLhas nine labels.NormalizeErroris#[non_exhaustive], so the newReservedvariant is not itself breaking.vitaminc-ore:Orderis no longer a tuple struct with a public field. WriteOrder::new(target)whereOrder(target)was written.vitaminc-ore:domain::ALLlists forty-one labels: the fifteen plain ones first, in the same order as before, then the thirteen nulls-first and the thirteen nulls-last.vitaminc-prf:EqualityTerm::new(None, Physical::Null)derives a term under the null domain instead of failing withPrfError::Normalize(NormalizeError::NoDomain).vitaminc-ore: the crate implementsSupports<Nulls<T, P>>for every scheme that implementsSupports<T>, andSupports<RuntimeNulls>for every scheme that implementsSupports<OrderTarget>, so a scheme can no longer write a single genericimpl<T: Target> Supports<T>: it conflicts with those impls. A scheme lists its targets withsupports_targets!or writes one impl per marker. No scheme in this workspace wrote a generic impl.vitaminc-ore:OrderSpace'sDebugoutput now shows its domain and a masked context.Nothing in
OrderSchemechanges. No byte of any plain-space term changes. The scheme crates, the Go and Node bindings, andtestdata/value-conformance.jsonare untouched.How it was tested
vitaminc_aead::plaintext::kinds), plus the NaNs at both float widths whose bytes would be all0x00or all0xFFwithout the fold, a quickcheck over everyf64bit pattern and one over NaNs and infinities only, and finds no code at either extreme.test_support::conformanceandquery_conformancenow check every fixed-width target in a nulls-first and a nulls-last space, so a scheme meets an all-zero and an all-ones plaintext. A mock that mishandles only the all-ones timestamp, a width no plain row reaches, fails them by the nulls-last label. The existing negative checks still fail where they did.tests/nulls.rsthrough the mock scheme, including a fixed-width scheme refusingencrypt_nullandnull_queryon a runtime target it does not take, a marker space and its terms and queries naming their own policy, and the two policies of one runtime target comparing asNonefor terms and for a query against a term. Four new compile-fail cases:nullsonTextorBytes,encrypton a policy space, a policy term in the plain space'scompare, and aNulls<I32, First>term in aNulls<I32, Last>space'scompareandcompare_query.hmac.Commands run from the repository root:
cargo fmt --all --check: passes.cargo clippy --workspace --all-targets --all-features -- -D warnings: passes,vitaminc-aeadincluded. Its wipe test now counts words withas_chunks, stable since Rust 1.88, the workspace's minimum, rather than allowing the newerchunks_exact_to_as_chunkslint.cargo test -p vitaminc-normalize -p vitaminc-ore -p vitaminc-hmac -p vitaminc-aead --all-features: passes.cargo test --workspace --no-fail-fast: everything passes exceptvitaminc-kmstests::test_finalize, which needs localstack on port 4566.cargo +1.96.0 test -p vitaminc-ore --all-features --test negative_space: the compile-fail output matches CI's pinned compiler.cargo +1.88.0 check --locked -p vitaminc-ore --all-targets: passes.cargo mutants --no-shuffle --in-diffover this PR's diff: 133 mutants, 74 caught, 59 unviable, none missed. Three equivalent mutants are skipped by name in.cargo/mutants.toml, as the other equivalent mutants are: the two ofnullthat return the same byte it does, andNulls::targetreturningDefault::default(), which is the same zero-sized marker.RUSTDOCFLAGS="-D warnings" cargo doc -p vitaminc-normalize -p vitaminc-ore --no-deps: no warnings.cargo check -p vitaminc-ore -p vitaminc-normalize --target wasm32-unknown-unknown: passes.