Skip to content

feat(eql): give EQL v4 range query domains and eql_v4.between - #1243

Merged
coderdan merged 7 commits into
nextfrom
feat/eql-v4-range-domains
Oct 19, 2026
Merged

coderdan merged 7 commits into
nextfrom
feat/eql-v4-range-domains

Conversation

@coderdan

@coderdan coderdan commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

EQL v4 can now take a BETWEEN query as one operand. EQL is the SQL library we install into a customer's PostgreSQL database to store and search encrypted values. EQL v4 is the copy of it that Stack Encrypt (our Rust engine) writes into.

ADR-0009 decision 7 makes a range query one term carrying both bounds, so a caller can't pair a low bound from one column with a high bound from another. Until now there was nothing in SQL to bind that term to. This PR adds a range query domain for each ordered family and ordering scheme, and an inlinable eql_v4.between(column, range) that uses the column's order-term index.

This PR is based on next. It was stacked on #1241, which has since merged. It doesn't depend on #1244, and #1244 doesn't depend on it. The two PRs only edit the same changelogs and changeset.

Changes

SQL (EQL v4 only)

  • Range query domains: eql_v4.query_<family>_range_ore and eql_v4.query_<family>_range_ope, one per (family, ordering term), as decision 8 says. That's 18 in total, over 9 families.
    • Wire shape: {"v":4,"i":{…},"lo":<bound>,"hi":<bound>}. Each bound is the value the single-bound domain stores under ob (ORE, an array of hex blocks) or op (OPE, a hex string).
    • The CHECK requires v, i, lo and hi, forbids c, and checks each bound's shape. Its version check is NULL-safe (COALESCE(VALUE->>'v' = '4', VALUE IS NULL)), so "v": null is refused. The other domains have the same hole; that's EQL domain CHECKs accept a payload whose version is JSON null ("v": null) #1285.
  • eql_v4.between(column, range), inclusive at both ends. It is LANGUAGE sql, IMMUTABLE, not STRICT and has no SET clause, so the planner inlines it and uses the functional index on the order term.
    • There are 29 overloads, one per stored ordered domain: _ord_ore and _search_ore take the ORE range, while _ord, _ord_ope and _search take the OPE range.
    • eql_v4_internal.lo(r) and eql_v4_internal.hi(r) read a bound with the term's own constructor. They're internal because only between calls them.
  • An ORE column's index needs the ore_block_256 btree operator class. A non-superuser install skips that class and disables the ORE column domains. The docs and the generated notes on the ORE overloads say so, and point to the OPE columns.
  • The range parameter must be cast to its exact range domain ($1::eql_v4.query_integer_range_ore). An uncast $1, a jsonb parameter, or a range of another family or term fails with 42725 "function … is not unique". The single-bound operators work differently: they accept an untyped $1. The function reference, the SQL comments and both READMEs say this.
  • between has to be written schema-qualified, because BETWEEN is an SQL keyword.

Keeping v3 unchanged

  • One source tree renders both EQL versions. A new file directive, -- ONLY EQL VERSION: 4, confines the range files to v4:
    • eql-codegen select <3|4> filters the files for each version's build. It refuses a kept file that -- REQUIRE:s a dropped one.
    • The renderer refuses to render a confined file as the other version.
    • The installer-completeness and Doxygen scripts honour the directive.
  • The v3 installer and both uninstallers are byte-identical to the base branch. No line of the v4 installer is lost; adding the range files only shifts where a few existing blocks sit in topological order.
  • docs:validate:documented-sql now also validates the v4-only files. It runs the whole v4 install order in one transaction and then rolls it back, so every statement in those files actually runs.

Bindings

  • Rust: 18 …RangeOreQuery and …RangeOpeQuery structs in eql_bindings::v4, listed by v4::all_range_query(). They implement no Stack Encrypt traits. DecryptInto never applies, because a query operand doesn't decrypt. EncryptFrom is follow-up No SDK can build an EQL v4 range operand for eql_v4.between — range bindings implement no EncryptFrom #1286: the derive can't describe two bounds under one column context, ORE bounds wait on block ORE, and query_term has nothing to call it with yet.
  • Go: 18 []byte types in encrypt/eql/eql_gen.go, with Value, Scan and JSON methods. They are not in the Types table.

Docs and release notes

  • New "Range queries in EQL v4" section in docs/reference/eql-functions.md.
  • packages/eql/AGENTS.md, the eql-bindings and Go READMEs and CHANGELOGs, and .changeset/eql-v4-bindings.md. The changeset says the v4 bundle reaches users as the eql-v4-install.sql attached to each Go release, not through npm or the crate.

Verification

  • mise run build, with the v3 installer and uninstallers compared byte for byte against the base branch: identical.
  • mise run test:encryption:postgres against a disposable postgres:17: text_eq 1, v4_types 3 and v4_range 6 tests passed. The v4_range tests cover:
    • Integer ORE and both OPE column domains: rows inside, outside and exactly at both bounds; across zero; a one-value range; an empty range; the full i32 range; an inverted range; a NULL range.
    • Text ORE and OPE.
    • EXPLAIN: an index scan with both bounds in the Index Cond, including for text. This holds for a literal range and for a $1 under a generic plan.
    • The domain CHECKs.
    • A catalog-driven smoke test over all 9 ordered families and all 29 between overloads.
    • The exact-cast rule, pinned with prepared statements.
  • Inlining: when one between overload was made STRICT by hand, the EXPLAIN test failed, so the test does catch lost inlining. The reviewer also confirmed an Index Scan under a generic plan with a real $1.
  • mise run test:crates, test:encryption, test:codegen, codegen:parity, types:check and typescript:check all passed.
  • The docs validators passed: required tags, 100% coverage, documented-sql (286 files, including the 18 range files) and doc anchors.
  • The other EQL gates passed: installer-complete, symbol order, self-contained v3, v4 isolation, schemas parity, public identifiers, and the matrix inventory and coverage. eql_v4.lints() returns no rows.
  • pnpm test:scripts: 1362 passed.
  • Go: all guests rebuilt, go vet ./... and go test ./... -count=1 passed, and go generate ./... leaves no diff. mise run wasm:guest:test: 71 of 71.
  • After the rebase onto next, these passed again:
    • build, test:codegen, test:crates and test:encryption;
    • the DB-free gates;
    • docs:validate:source, docs:validate:documented-sql (286 files) and test:doc-anchors;
    • types:check and codegen:parity;
    • test:encryption:postgres against a local PostgreSQL 17.5: all 10 tests.
    • After merging next and the NULL-version fix: test:encryption:postgres against postgres:17, 17 tests across the five suites, plus codegen:parity, test:v4_isolation, test:installer_complete and docs:validate:documented-sql.

Not run: the SQLx suite (test:sqlx). It needs CipherStash credentials to generate fixtures through ZeroKMS. The database tests live in packages/eql/tests/encryption instead, which installs the v4 bundle, whereas SQLx installs v3.

Where the test order terms come from. The ordered EQL types aren't producible yet (#1236, #1132). So the tests take order terms from cipherstash-client's OreIndexer and OpeIndexer over the fake key source's index key, the same pattern as ste_vec_selectors.rs. Stack Encrypt is untouched, and nothing in this PR derives order terms.

Related

Closes #1237

Review notes

https://claude.ai/code/session_01BZhkvKLpNtKuzj3HCfMXoL

@changeset-bot

changeset-bot Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: ad19081

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes changesets to release 12 packages
Name Type
@cipherstash/eql Minor
stash Patch
@cipherstash/stack-prisma Patch
@cipherstash/basic-example Patch
@cipherstash/e2e Patch
@cipherstash/prisma-example Patch
@cipherstash/stack-drizzle Patch
@cipherstash/stack-supabase Patch
@cipherstash/stack Patch
@cipherstash/wizard Patch
@cipherstash/bench Patch
@cipherstash/test-kit Patch

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3310e29d-f67b-406f-9357-caf9dd5c2932

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Base automatically changed from feat/eql-v4-equality-types to next October 18, 2026 05:52
@coderdan
coderdan force-pushed the feat/eql-v4-range-domains branch from 308ee9a to e66295a Compare October 18, 2026 12:04
@coderdan

Copy link
Copy Markdown
Contributor Author

Rebased onto next. Two things needed fixing:

  • languages/golang/CHANGELOG.md conflicted with the integer-widths entry from next. Both entries are kept.
  • tests/v4_range.rs used eql_bindings::encryption::plaintext::Int32. next removed that type, so the test now encrypts the i32 directly. The fix is folded into the first commit.

There were no review comments to address. The branch no longer depends on #1241, and it is independent of #1244.

I re-ran the checks locally after the rebase, and these passed:

  • build, test:codegen, test:crates, test:encryption;
  • the installer, symbol-order, v4-isolation, schemas-parity, public-identifier and matrix gates;
  • docs:validate:source, docs:validate:documented-sql and test:doc-anchors;
  • types:check and codegen:parity;
  • test:encryption:postgres against a local PostgreSQL 17.5: all 10 tests, including the 6 v4_range ones.

I couldn't run SQLx, since it needs credentials, so that's left to CI.

Comment thread packages/eql/docs/reference/eql-functions.md Outdated
Comment thread packages/eql/crates/eql-codegen/src/main.rs
Comment thread packages/eql/tasks/docs/validate/documented-sql.sh Outdated
Comment thread packages/eql/crates/eql-codegen/src/bindings.rs Outdated
Comment thread packages/eql/AGENTS.md
Comment thread languages/golang/CHANGELOG.md Outdated

@coderdan coderdan left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second pass at high effort, on the rebased head 8cfaf37 against next. Six inline comments from the first pass are already on the PR (the ORE opclass caveat in the between docs and templates, select not checking -- REQUIRE: closure, documented-sql.sh counting a missing eql_v4 schema as validated, the duplicated term_label, the stale verify_no_v3_in_v4.sh header, and the garbled Go changelog sentence). I have not repeated those; the three below are what that pass did not cover.

Rebase check. Nothing is stale. cargo check --all-targets --all-features passes for packages/eql/tests/encryption on this head, no FfiValue or encryption::plaintext reference survives in the PR, and the test crate builds its stored rows through keyset.encrypt_as as next now expects. (The only workspace-wide cargo check failure is the SQLx crate's include_str! of credential-generated fixtures, which predates this PR.)

What I ran without credentials. cargo test -p eql-domains -p eql-codegen and cargo test -p eql-bindings --features stack-encrypt --lib: all green (166 + 99 + 108 and the smaller suites). The Postgres tests were not run locally; I confirmed test-eql.yml runs mise run test:encryption:postgres, so the 29-overload smoke test is on CI.

What I checked in the SQL and found sound. Both comparisons keep the column on the comparator's left, so the bound is the operand compare_ore_block_256_term reads its right half from, which is the only shape that works with full-ciphertext bounds (the left-ciphertext question is already on #1132 as the PR says). Bounds are inclusive at both ends and the tests pin rows exactly at both bounds. NULL range and inverted range both yield no rows, and TermError::InvertedRange in stack-encrypt/src/dynamic/query.rs backs the comment that the client refuses lo > hi. pin_search_path_v3.sql's structural skip (LANGUAGE sql, IMMUTABLE, a jsonb-backed domain argument) leaves between, lo and hi unpinned, so post-install inlining survives. The uninstaller is DROP SCHEMA ... CASCADE, so the new objects are covered. No dynamic SQL anywhere in the change; the CHECKs are parsed at creation time and the function bodies resolve pg_catalog names like every existing unpinned extractor. build.sh assigns selected on its own line, so a failing select aborts under set -e.

Findings, most severe first. None is blocking.

  1. Medium (test coverage): the index-engagement test can only see a folded constant, not a parameter, so it cannot detect between losing inlining under a generic plan. Inline comment on v4_range.rs.
  2. Low (surface): lo and hi are user-facing names in eql_v4 with no user purpose. Inline comment on the template.
  3. Low (scope): json_entry leaves carry the OPE cross-type comparisons but get no between, so a range over an encrypted JSON path is still two predicates. Inline comment on spec.rs; may be deliberate.

Comment thread packages/eql/tests/encryption/tests/v4_range.rs
Comment thread packages/eql/crates/eql-codegen/templates/range_functions.sql.j2 Outdated
Comment thread packages/eql/crates/eql-domains/src/spec.rs
ADR-0009 decision 7 makes a Between query one term holding both bounds,
and asks that it stay one operand in SQL. EQL v4 had no domain to carry
it and no function to apply it, so a Between term had nothing to bind to
but two comparisons, which loses the one-term guarantee (#1237).

Every ordered family now has one range query domain per order term,
beside its single-bound ones (decision 8): eql_v4.query_integer_range_ore
beside query_integer_ord_ore, query_integer_range_ope beside
query_integer_ord_ope, and the same for smallint, bigint, date,
timestamp, numeric, text, real and double. A range is
{"v": 4, "i": ..., "lo": <term>, "hi": <term>}: each bound is the value
the single-bound domain holds under ob or op, which is Stack Encrypt's
RangeQuery { lo, hi } with nothing added. The CHECK requires both bounds
in their term's shape and forbids c.

eql_v4.between(column, range) has an overload for every ordered column
domain against the range of the term it stores, and is
ord_term(a) >= lo(r) AND ord_term(a) <= hi(r): LANGUAGE sql, IMMUTABLE,
not STRICT, no SET clause, so PostgreSQL inlines it and a btree on the
column's order term serves it as one range scan. lo and hi read a bound
with the term's own constructor, as a single-bound term is read; they are
in eql_v4_internal, since only between calls them. An ORE column's index
needs the ore_block_256 btree opclass, which a non-superuser install
skips; the docs say to use an OPE column there.

All of it is generated from the catalog (Term::range_suffix,
DomainFamily::range_terms / range_columns, RANGE_BOUND_KEYS) by a new
eql-codegen range module, as are the bindings: a v4 struct per range
domain (IntegerRangeOreQuery, ..., v4::all_range_query()) and a Go type
of the same name in encrypt/eql, so a range binds as one parameter.

EQL v3 does not change. One source tree renders both versions, so the
range files carry a new directive, `-- ONLY EQL VERSION: 4`. The build
selects each version's files with `eql-codegen select` before rendering
(which refuses a kept file that requires a dropped one, so each order
stays closed under REQUIRE),
`render` refuses to render a confined file as the other version, the
installer completeness gate filters its disk side with its own grep, the
symbol-order checker now runs on the v4 order too, and the Doxygen
filter reads a confined file from its version's rendered tree. The v3
installer and both uninstallers are byte-identical to the base; the v4
installer only gains lines.

The ordered EQL types are not producible yet (#1132, #1236), so the
PostgreSQL tests (tests/encryption/tests/v4_range.rs, run by
test:encryption:postgres) take their order terms from cipherstash-client's
ORE and OPE indexers over FakeDataKeySource's index key, and the rest of
each value from a real Stack Encrypt Integer or TextEq. They cover rows
inside, outside and at both bounds for ORE and OPE over integers and
text, inverted and NULL ranges, a range of the wrong term, the CHECKs,
and an EXPLAIN showing the order-term index serving between with both
bounds in its index condition.
… envelope is

The docs said a range of the wrong term fails "when the query is planned".
It fails when the statement is parsed, with 42725 ("function
eql_v4.between(...) is not unique"), and so do an uncast $1, a jsonb
parameter and a range of the wrong family: PostgreSQL resolves a
function's domain arguments as their base type, jsonb, which every
between overload takes. The single-bound operators resolve an untyped
$1; between does not. The function reference, the generated SQL doc
comments, the eql-bindings README and changelog, the Go README and
changelog and the changeset now say the range parameter must be cast to
its exact range domain. The 42725 behaviour stays; there is no
catch-all overload.

They also said a range is Stack Encrypt's RangeQuery { lo, hi } "with
nothing added". It is not: RangeQuery has no serialized form,
Operand::Range carries raw bytes, the wire adds v and i, and query_term
refuses a range on an EQL target today. Every description now says each
bound is the single-bound domain's ob (ORE) or op (OPE) value, in a
{v, i, lo, hi} envelope, which the engine will need to build when the
ordered types become producible.

The changeset no longer implies the npm package or the crate ships the
v4 SQL: the v4 bundle reaches users as the eql-v4-install.sql attached
to each Go release.
…exact cast

postgres_between_smoke_test_over_every_ordered_family loops over the
catalog: for each of the nine ordered families it stores five ascending
values in every column domain that carries an ordering term, and checks
that a range from the second value to the fourth finds the rows at both
bounds and the one inside, and neither row outside, through every one of
the 29 between overloads (the _search and _search_ore ones included).
For text it also checks that a btree on the order term serves between,
for ORE on _ord_ore and for OPE on _search. Every family gets its terms
from cipherstash-client's indexers, as the other range tests do; c, hm
and bf are real Stack Encrypt values sealed from the value's text, since
between never reads them and most of these families have no producible
type.

postgres_between_needs_the_exact_range_domain prepares between with an
uncast $1, a jsonb parameter, a range of another family and one of
another term, and pins 42725 for each, beside the exact cast and the
single-bound operator, which both prepare.

The cipherstash-client comment in the test crate's manifest now names
v4_range.rs, and chrono and rust_decimal (already in the lockfile) name
the date, timestamp and numeric plaintexts.
…ted-sql

The task ran psql over build/render/eql_v3/src only, so the 18 files
confined to EQL v4 (the range domains and between) were never validated:
they are not rendered into the v3 tree. Run in isolation like the v3
files, each would stop at its first statement with "schema eql_v4 does
not exist", which the task counts as an expected dependency error. So it
now runs the whole v4 install order in one transaction that is rolled
back: every statement of the confined files runs after what it requires,
and any error fails the task. It also fails if it finds no confined file,
or one missing from the v4 order. The other v4 files are the v3 ones
under another schema name and are not run twice. The AGENTS paragraph on
confined files says so.

@coderdan coderdan left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incremental re-review at high effort of b7a793c against the previous head 8cfaf37 (same next base, git range-diff of the four commits). I only reviewed what changed.

Prior findings: all nine are addressed. I checked each one in the code, not in the commit messages.

  1. ORE opclass caveat in the between docs and template: fixed. The reference example now uses an OPE column. A bullet names the ore_block_256 opclass and what a non-superuser install does instead. The generated @note appears only on the ORE overloads, and the_ore_overloads_name_the_opclass_they_need pins that. There is a small follow-up below (the range ORE operand is not part of the fallback).
  2. select not checking -- REQUIRE: closure: fixed. render::select collects the dropped set and refuses any edge from a kept file into it, and two unit tests cover it. I also checked that the check works on the real input: build.sh feeds it src/deps-ordered-v3.txt, whose src/v3/... paths are the same form requires_of returns. surface_order has already validated every target by then, so no target can fall outside both sets.
  3. documented-sql.sh counting a missing eql_v4 schema as validated: fixed. I ran the task against a local PostgreSQL 15: 286 validated, 0 errors. Then I appended a broken CREATE FUNCTION to the rendered query_integer_range_functions.sql, and the task failed with 18 errors (one bad file counts as all 18, which is cosmetic).
  4. Duplicated term_label and key list in bindings.rs: fixed. It now uses range::term_label and range::range_required_keys().
  5. Stale verify_no_v3_in_v4.sh header: fixed.
  6. Garbled Go changelog sentence: fixed.
  7. Index test only covering the constant-folded path: fixed. The generic-plan PREPARE / EXPLAIN EXECUTE assertion is in place. I ran v4_range --ignored locally against the built v4 bundle on PostgreSQL 15: 6/6 pass.
  8. lo / hi in the public eql_v4 schema: fixed. After install, eql_v4_internal has 18 lo and 18 hi, and eql_v4 has none. I checked the four things you asked about:
    • proconfig is NULL on all of them and on all 29 between overloads, so pin_search_path_v3.sql's structural skip covers the internal schema too (owned_schemas() includes it).
    • The v4 uninstaller drops both schemas with CASCADE.
    • Grants are unchanged in practice. Ordering already needed USAGE on eql_v4_internal (ord_term inlines ope_cllw, see permissions.md). As a non-superuser role with only USAGE on eql_v4 and eql_v4_internal, a generic-plan between(c, $1::eql_v4.query_integer_range_ope) still runs as an Index Scan whose Index Cond is decode(...) >= eql_v4_internal.lo($1)::bytea AND ... <= eql_v4_internal.hi($1)::bytea, and it returns the right rows.
    • No reference to eql_v4.lo / eql_v4.hi is left anywhere in the tree.
  9. json_entry having no between: resolved as deliberate. The range_columns doc comment and the follow-up list say so.

New findings, most severe first. None is blocking.

  1. Low: on a non-superuser install, ore_fallback.sql disables query_<T>_ord_ore but not query_<T>_range_ore, so a range operand gets past the refusal that gives the single-bound operand its OPE hint. Inline on the reference.
  2. Low (conflicts with #1235): the reference and a test comment say the client refuses a range whose low bound is above its high bound. #1235 (open against next) removes TermError::InvertedRange and lets that range through. Inline on the reference.
  3. Nit: the generated ORE @note tells every family to use _search, which only text has. Inline on the template.

Checks run (no credentials): cargo test -p eql-codegen -p eql-domains and cargo clippy -p eql-codegen -p eql-domains --all-targets -- -D warnings (green). mise run build. docs:validate:documented-sql against a local PostgreSQL 15, both clean and with the injected failure. cargo test -p eql-encryption-tests --test v4_range -- --ignored against the built v4 bundle. A non-superuser install of release/cipherstash-encrypt-v4.sql. CI is green on this head.

Comment thread packages/eql/docs/reference/eql-functions.md Outdated
- **An ORE column needs the `eql_v4_internal.ore_block_256` btree operator class**, for its index and so for an ORE range scan. The installer creates that class only when it runs with superuser privileges. Otherwise (on Supabase and other managed roles) it skips it and disables the ORE column domains: a value cast to `_ord_ore` or `_search_ore` raises `feature_not_supported`, naming the OPE and equality alternatives. On such an install, use an OPE column and its `_range_ope` domain, which need nothing more.
- Always write it schema-qualified. `BETWEEN` is an SQL keyword, so a bare `between(...)` does not parse.
- **Always cast the range parameter to its exact range domain**, as in `$1::eql_v4.query_integer_range_ore`. PostgreSQL resolves a function's domain arguments as their base type, `jsonb`, and every `between` overload takes jsonb domains. So an uncast `$1`, a `jsonb` parameter, or a range of the wrong term (an OPE range on an `_ord_ore` column) or the wrong family matches several overloads, and the statement fails with SQLSTATE 42725 (`function eql_v4.between(...) is not unique`) when it is parsed, before it runs. This differs from the single-bound operators: `col < $1` resolves an untyped `$1`.
- A `NULL` range matches no row. So does a range whose low bound is above its high bound; the client refuses to build one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low (conflicts with #1235): "the client refuses to build one" will be wrong once #1235 lands.

#1235 (open, also against next) removes TermError::InvertedRange, so Between with the low bound above the high bound now derives its terms, as SQL BETWEEN 20 AND 10 does. The SQL behaviour this sentence describes is unchanged: the range matches no row, and the tests pin that. Only the client half becomes false. The same claim is in the test comment at packages/eql/tests/encryption/tests/v4_range.rs:366 ("the client refuses one, and the SQL does not reorder it").

Suggest wording that is true whichever PR merges first: "So does a range whose low bound is above its high bound, as with SQL BETWEEN; the SQL does not reorder the bounds."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in f3539d6 with your wording, in the reference, the range types template (so all 9 generated files) and the v4_range.rs comment. Each now says such a range matches no row, as with SQL BETWEEN, and that the SQL does not reorder the bounds. None of them says the client refuses it any more.

Comment thread packages/eql/crates/eql-codegen/templates/range_functions.sql.j2 Outdated
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-19T06:12:54.341828Z b7a793c Draft marked ready
🔒 Security Review ✅ Completed 2026-10-19T06:12:32.096321Z b7a793c Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Comment thread packages/eql/crates/eql-bindings/src/v4/date.rs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b7a793c494

ℹ️ 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".

Comment thread packages/eql/crates/eql-codegen/templates/range_types.sql.j2 Outdated
A non-superuser install skips the ore_block_256 btree operator class, and
ore_fallback.sql then poisons every single-bound ORE domain so a value cast
to one fails loudly with an OPE hint. It could not reach the nine
query_<T>_range_ore domains: it is shared with EQL v3 and they are v4-only.
So on such an install a range operand was accepted, and eql_v4.between ran
as an unindexed seq scan with no hint. Each v4-only range types file now
adds the same NOT VALID poison CHECK to its own ORE range, behind the same
opclass check, calling the function ore_fallback.sql creates, and names the
family's OPE range in the hint.

A new PostgreSQL test installs the built v4 bundle as a NOSUPERUSER role in
a database of its own and checks every ORE range is refused with that hint
while its OPE range is accepted. Stripping the new ALTER DOMAINs from the
bundle makes it fail.

Also: the generated ORE between note now lists the family's own OPE column
domains and OPE range instead of a fixed `_ord`, `_ord_ope`, `_search` (only
text has `_search`), and the inverted-range wording no longer says the
client refuses such a range, which #1235 makes untrue; it says what holds
either way: such a range matches no row and the SQL does not reorder it.
…ains

# Conflicts:
#	packages/eql/crates/eql-codegen/src/go_eql.rs
#	packages/eql/mise.toml
#	packages/eql/tests/encryption/Cargo.toml
The range domains' CHECK compared `VALUE->>'v' = '4'`. For `"v": null`
that comparison is SQL NULL, the whole CHECK is NULL, and PostgreSQL
treats a NULL CHECK as passing, so a range with no v4 version cast
cleanly and `eql_v4.between` ran on it.

The check is now `COALESCE(VALUE->>'v' = '4', VALUE IS NULL)`: false for
a JSON null version, still true for a NULL range (which matches no row).
The comparison keeps the one form the v4 isolation gate accepts.

`postgres_range_domains_check_their_shape` now casts a `"v": null` range
to both the ORE and the OPE domain and expects 23514. It fails without
the template change.

The same `->>'v' = '<n>'` predicate is in every other domain CHECK, most
of them shared with v3; that is left to a separate change so the v3
bundle stays byte-identical here.
@coderdan
coderdan merged commit 2b91de0 into next Oct 19, 2026
42 checks passed
@coderdan
coderdan deleted the feat/eql-v4-range-domains branch October 19, 2026 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant