fix(v3): inline the jsonb_entry domain CHECK; plpgsql jsonb_query validator (+19% field_eq regression)#358
Merged
coderdan merged 3 commits intoJul 4, 2026
Conversation
… plpgsql Domain constraints cannot inline SQL functions, so eql_v3.jsonb_entry's CHECK calling eql_v3_internal.is_valid_ste_vec_entry_payload paid the per-call SQL-function executor (~18us) on every cast — the needle cast in every field_eq query, and the ENTIRE measured +19% v2->v3 regression on that scenario (cipherstash/benches#23). The CHECK now mirrors the validator body inline; the leading `VALUE IS NULL OR` preserves the validator's STRICT NULL-passes semantics (a bare COALESCE(..., false) would reject the NULL that `->` returns for a missing selector). Validated A->B->A on a live 100k bench database: 0.0287 -> 0.0137 -> 0.0285 ms/query in-DB, with validator calls dropping to zero. eql_v3.jsonb_query's CHECK CANNOT be inlined — validating the sv elements needs a subquery, which CHECK constraints forbid — so its validator is converted to plpgsql instead (cached plan vs the per-call SQL-function executor; the #353 finding), since the needle cast sits on the per-query hot path of every containment scenario. The eql_v3.json document CHECK is unchanged: it is part of the documented privilege contract (docs/reference/permissions.md, gated by v3_privilege_tests). permissions.md now notes the jsonb_entry cast no longer requires the internal grant; jsonb_query's still does. New family/jsonb_check tests: entry inline-CHECK <-> validator equivalence over a payload-shape corpus (accept/reject + NULL), a hardcoded accept/reject characterization for jsonb_query, and a language guard so a revert of the query validator to LANGUAGE sql fails CI. Closes #354 Claude-Session: https://claude.ai/code/session_01StQnoycoFXMDdSpQ6zKDav
Contributor
|
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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The jsonb_entry / jsonb_query domain doc blocks are public API docs (Doxygen renders src/ with INTERNAL_DOCS at its default NO), and the issue-#354 performance rationale added there is an internal concern — wrap it in @internal/@endinternal so the published docs keep only the user-facing contract. The #357-style notes on the internal validator functions were already excluded via their blocks' @internal tags. Claude-Session: https://claude.ai/code/session_01StQnoycoFXMDdSpQ6zKDav
…nb_entry-domain-check-calls-a-non-inlinable-sql
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.
Closes #354 (Linear: CIP-3381).
What
eql_v3.jsonb_entry: the domain CHECK is now an inline expression mirroringis_valid_ste_vec_entry_payload's body, instead of calling it. A leadingVALUE IS NULL ORpreserves the validator's STRICT NULL-passes semantics (a bareCOALESCE(..., false)would reject the NULL that->returns for a missing selector).eql_v3.jsonb_query: its CHECK cannot be inlined — validating the sv elements needs a subquery, and CHECK constraints forbid subqueries (discovered the hard way; this is presumably why the validators existed in the first place). Its validator is converted to plpgsql instead — cached plan vs the per-call SQL-function executor, the same finding as v3: opclass-path helper functions are LANGUAGE sql — cannot inline, regresses ORE ordered scans 43% vs v2 (validated fix: plpgsql) #353 — since the needle cast sits on the per-query hot path of every containment scenario.eql_v3.json(document) is deliberately unchanged: its CHECK-crosses-into-eql_v3_internalbehaviour is part of the documented privilege contract (docs/reference/permissions.md, gated byv3_privilege_tests). The doc now notes thejsonb_entrycast no longer requires the internal grant;jsonb_query's still does.Why
Domain constraints can't inline SQL functions, so every
::eql_v3.jsonb_entrycast paid ~18 µs of SQL-function-executor overhead — the needle cast in everyfield_eqquery, and the entire measured +19% v2→v3 regression onJSON/json/field_eq/functional(in-DB 0.011 → 0.029 ms/query with identicaleq_termcosts between versions). Third confirmed instance of the LANGUAGE-sql-in-non-inlinable-context pattern; attribution data inv3-regressions-report.mdon cipherstash/benches#23.Validation
ALTER DOMAINswap, then restored): field_eq harness 0.0287 → 0.0137 → 0.0285 ms/query;pg_stat_user_functionsshows validator calls drop to zero under the inline CHECK. v2's equivalent measures 0.0109 ms on the same harness — the residual is the CHECK expression itself, which v2 also pays.svand extra merged fields), missings/c, dual terms, term-less, ciphertext-carrying needle elements, scalars, JSON null, and{}all rejected; SQL NULL casts still succeed for both.family/jsonb_checktests: entry inline-CHECK ↔ validator equivalence over the payload corpus (so an edit to one that forgets the other fails CI), a hardcoded accept/reject characterization forjsonb_query, and a language guard that fails if the query validator reverts to LANGUAGE sql.Follow-up
The document CHECK (
is_valid_ste_vec_document_payload, and its per-element calls into the entry validator) still pays function-call costs per INSERT/cast — left as-is here because changing it touches the privilege contract. If ingest-side cost warrants it, the same plpgsql conversion applies; folded into the systemic audit suggested in #354.https://claude.ai/code/session_01StQnoycoFXMDdSpQ6zKDav