Repository navigation
feat(stack-encrypt, eql-bindings, golang): order dates, timestamps and decimals - #1337
Conversation
🦋 Changeset detectedLatest commit: f8fc092 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
Mutation testing (cargo-mutants,
|
| caught | missed | unviable | timeout |
|---|---|---|---|
| 40 | 0 | 11 | 0 |
Every mutant in the changed lines was caught by a test.
12f9e83 to
5a3e878
Compare
d683a45 to
c753de7
Compare
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. |
cipherstash-agent
left a comment
There was a problem hiding this comment.
Review attestation: /code-review medium + cross-check-review-findings
Reviewed head: c753de7. No critical or blocking issues.
- 1 minor finding, posted inline: the changeset
eql-v4-date-timestamp-numeric.mdstill namesv4::plaintextas the owner of the moved types. - 6 other candidate findings were dropped:
- Cosmetic line wrapping.
- A possible double parse in
order_of. - Missing tests for the
Some(_)arms inordered()andorder_of(). The mutation job reports 0 missed mutants. - The
Reason::OrderTermPendingremoval. The PR says it was never released, and I could not show a downstream use. - Fixture ciphertext churn.
- A Postgres test gap for OPE.
- Existing feedback was bots only (changeset-bot, CodeRabbit, Codex, mutants). CodeRabbit and Codex were still running when I checked, so overlap with their later findings is not ruled out.
- I read the diff and checked the cited code at head. I did not build the crates or run the tests.
Generated by Claude Code
5a3e878 to
f3ade86
Compare
7741dde to
300a799
Compare
vitaminc-ore 0.8 encodes the Date, Timestamp and Decimal targets, and
clww-ore 0.6 and ore-rs 0.10 take them, so the engine can now derive their
order terms.
The typed path needs a Rust type for each that the engine can wipe and
clone. chrono's and rust_decimal's types keep private fields, and
`Orderable` is sealed, so eql-bindings' wipeable `v4::plaintext` types
could not join it. They move here as `value::{Date, Timestamp, Decimal}`
(with `RangeError`, and the chrono and rust_decimal conversions behind
features of those names), and become `Orderable` and `op::Ordered`. Their
one `unsafe`, in the test that proves `Drop` wipes in place, is allowed on
that test alone.
The data-plan path dropped a scalar's logical type when it picked an order
target, so a date would have ordered as its bare day count. `order_of` now
reads a date, timestamp or decimal into its `value` type first, and
`IndexSpec::takes` admits one exactly when it reads. With nothing left to
refuse, `Reason::OrderTermPending` is removed; it was never released.
A timestamp orders by the microsecond, floored, and a decimal by value
whatever its scale, as their equality terms already compare. Block ORE
puts a date through the 6-bit fixed-width engine and a timestamp or
decimal through the chained one.
Refs #1236
Claude-Session: https://claude.ai/code/session_01XQN1QpaWVDGjHEg39KEJF3
…umeric types With the engine ordering dates, timestamps and decimals, the nine EQL v4 types it refused become producible: DateOrd, DateOrdOpe, DateOrdOre and their timestamp and numeric twins. eql-codegen lists them in ENCRYPTION_DOMAINS and loses the pending-order reason; the generated bindings, the target table and Go's encrypt/eql table follow. eql-bindings' `v4::plaintext` now re-exports Stack Encrypt's `value` types, and its chrono and rust_decimal features turn on Stack Encrypt's. Its one remaining direct use of vitaminc-aead, the null tag in the by-name dispatch, goes through `stack_encrypt::dynamic::frame`, so the dependency is dropped. Tests: the OPE types join v4_order's ordering checks, the block ORE types join v4_ore_block with a PostgreSQL comparison over dates, timestamps (ties within a microsecond, flooring before the epoch) and decimals (1.5 ties 1.50), and the between smoke test now ranges over every ordered family. Tests that used DateOrdOre as the unproducible example use Json. Go: stashgen's static engine and notices admit order indexes on decimals, dates and timestamps; the ValueKinds model takes the order indexes the value-kinds fixture now declares, and its test checks Lt against the stored terms, folded queries included. Closes #1236 Claude-Session: https://claude.ai/code/session_01XQN1QpaWVDGjHEg39KEJF3
…ncrypt's The changeset for the unordered types still named eql-bindings' v4::plaintext as their owner. This branch moved them to stack_encrypt::value and re-exports them there, and the ordered types' changeset says so, so the two entries would have shipped contradicting each other. Claude-Session: https://claude.ai/code/session_01XQN1QpaWVDGjHEg39KEJF3
300a799 to
f8fc092
Compare
Summary
Stack Encrypt is the Rust engine behind the Go SDK and the EQL types. It can now store order terms for dates, timestamps and decimals. An order term is an index value that lets the database answer
<,>,BETWEENandORDER BYwithout decrypting. Until now the engine refused an order index on these three kinds. That was the last open part of #1236.What this means for users:
encrypt.Decimal,encrypt.Dateortime.Timefield can now takeindex=oreorindex=ope. The generated code then gives the fieldLt,Lte,Gt,GteandBetween.DateOrd,DateOrdOpe,DateOrdOre, and theTimestampandNumericversions of each. Before, they were refused by name.How values order:
1.5ties1.50.Stacked on #1328, which is stacked on #1327 (the vitaminc 0.8 and ORE crate upgrade this depends on). It needs nothing from #1328 itself. It is stacked there only to avoid conflicts, since both PRs edit the same files.
Changes
Stack Encrypt
stack_encrypt::valuewithDate,Timestamp,DecimalandRangeError, moved here from eql-bindings.Orderabletrait is sealed, so the engine has to own these types to order them.chronoandrust_decimalfeatures add conversions to and from those crates' types.sem::Orderableandop::Ordered, so the ORE, OPE and block ORE indexes all take them.order_ofused to drop a value's logical type when choosing how to order it, so a date would have been ordered as a bare day count. It now reads a date, timestamp or decimal into itsvaluetype first.IndexSpec::takesadmits one exactly when that read succeeds.Reason::OrderTermPendingis removed. Nothing can produce it any more, and it was never released.eql-bindings and eql-codegen
v4::plaintextre-exports thestack_encrypt::valuetypes, so callers' code is unchanged.chronoandrust_decimalfeatures now turn on Stack Encrypt's features of the same name.vitaminc-aeaddirectly. Its one remaining use now goes throughstack_encrypt::dynamic::frame.Go SDK
enginetest) and the notices stashgen prints now admit order indexes on decimals, dates and timestamps.encrypt/eqltable is regenerated.ValueKindstest model takes the order indexes the value-kinds fixture now declares, andprobe_stash.goandvaluekinds_stash.goare regenerated.encryptpackage docs, stashgen README.Docs and release notes
@cipherstash/eqlchangeset (eql-bindings ships at the npm package's version).Verification
cargo nextest run --workspace --all-featurespasses (1278 tests, 1 skipped). clippy with-D warningsis clean with all features and with none. rustdoc with-D warningsandmise run test:docpass.1.5with1.50, tie instants within one microsecond, and floor one nanosecond before 1970 to the microsecond before it;opbounds.value_kinds.jsonis regenerated: it only gains the three fields' order terms, and a folded query (-1234.56,…03.123456Z) now also lands on the stored order term.Ordereddiagnostic now names the new types;--all-featuresturns on chrono, which adds chrono's types to the compiler's list of implementors.cargo mutants --in-difffor stack-encrypt ran 51 mutants: 40 caught, 11 unviable, none missed.cargo test -p eql-codegen,-p eql-bindings --all-featuresand-p eql-encryption-testspass, as domise run types:checkandcheck:encryption:wasi.mise run test:encryption:postgres:eql_v4.betweensmoke test now covers every ordered family, 29 overloads (up from 20).mise run go:test,mise run go:lintandmise run wasm:guest:testpass, andgo generate ./...changes no committed file.Lton the new fields derives the bytes Rust stored, including for the folded queries.pnpm run test:scriptspasses (78 files).Related
Closes #1236.
Stacked on #1328 and #1327. Uses clww-ore 0.6.0 and ore-rs 0.10.0 (cipherstash/ore.rs#130, cipherstash/ore.rs#131, cipherstash/ore.rs#132, cipherstash/ore.rs#135).
Review notes
packages/stack-encrypt/src/dynamic/term.rs(orderedandorder_of), thenpackages/stack-encrypt/src/value.rs.value.rsis eql-bindings'v4/plaintext.rswith its docs rewritten for the engine. Its tests moved with it.unsafe: stack-encrypt deniesunsafecode. The moved test that provesDropwipes a value in place needs one, so it is allowed on that test only, with the reason written down. Without that test, aDropthat wiped nothing would pass.expects: the conversions to chrono and rust_decimal types useexpect, each allowed with its reason, followingdescriptor.rs. Avaluetype is only ever built inside the target crate's range, and chrono has no unchecked constructor.ANSWERS) still answers no ordered type of any family, as before. These nine join that existing gap intyped_queries.rs; this PR doesn't close it.stored_rows_v1.jsonis not touched. Its rows keep the plans they were written with, so its date, timestamp and decimal fields have no order term. Nothing regenerates the file.https://claude.ai/code/session_01XQN1QpaWVDGjHEg39KEJF3
Summary by CodeRabbit