Repository navigation
feat(go): bind a jsonpath predicate on a JsonSearch field as one parameter - #1452
Conversation
|
|
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
|
Mutation testing (cargo-mutants,
|
95a22e9 to
5902c80
Compare
bb9a5c9 to
cb6a190
Compare
5902c80 to
be8052a
Compare
|
Rebased onto #1451's new head (cb6a190); new head be8052a. Rebase only: no change to this PR's own commits. Why: the PRs below changed under review (#1444's feedback, the move of the JSON tree SQLx tests onto documents Stack Encrypt seals, and the clippy fix in #1445 that gates |
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
Reviewed at head be8052a by Claude Code: /code-review medium, then company-skills:cross-check-review-findings against the existing review threads.
Critical or blocking issues: none.
New findings (2, both minor, inline):
gensupport/json.go:345: a typed-nil*stringasvarsis refused, though the doc says a nil pointer sends no variables.internal/guest/diagnostic.go:197:wholeFloatis not bounded tomath.MaxInt, so a large float offset overflows on 32-bit builds.
Not repeated (already covered by earlier threads): the redacted Format change is listed under Breaking in the CHANGELOG, the LogValue consistency, the whitespace vars handling, and the TESTS array in mise.toml.
Dropped after verification: the stashgen version-skew concern, the json.RawMessage coupling concern, and the nil/empty print diagnosability concern are speculative or design preferences, not defects.
Process note: the changeset-bot reports no changeset. Go releases are recorded in languages/golang/CHANGELOG.md per the PR description, so this looks intentional.
Codex code and security reviews were still running when this review was posted and are not reflected here.
Generated by Claude Code
cb6a190 to
52802ef
Compare
be8052a to
6f533d4
Compare
52802ef to
2237691
Compare
6f533d4 to
7f60d98
Compare
7f60d98 to
2b401e8
Compare
2237691 to
cbab542
Compare
2b401e8 to
edc1137
Compare
cbab542 to
db20d2c
Compare
edc1137 to
0bb5e58
Compare
db20d2c to
e65fca9
Compare
0bb5e58 to
a7d14d2
Compare
A Go EQL value is the JSON its column stores: a stored value holds ciphertext and terms, and a query value holds terms. Until now each type was a bare []byte to fmt and slog, so %s or a log line wrote those bytes out. The compiled predicate Match returns (#1443) is the first operand a caller is likely to log whole, so every generated type now gets String, Format and LogValue, written by eql-codegen beside Value and Scan so no type can be missed. All three write one text, the type's name and [redacted] (or {} when empty); Format answers every verb, %#v included, so no GoString is generated. Value, MarshalJSON and a []byte or string conversion still give the JSON. The helpers (text, format, logValue) are hand-written in eql.go, so the generated methods stay one-line block bodies gofmt leaves alone. A codegen test checks every `type X []byte` the file declares has all three.
…meter An application that compiles a filter language into SQL targets jsonb_path_match(col, $1, $2): the predicate and its variables, two parameters. On an encrypted column the same filter was one Go call per leaf and a hand-assembled WHERE, one cast per parameter. The engine's match operator (#1441) compiles the predicate into one operand; this is the Go call on top (#1443). Fields.<Field>.Match(ctx, cipher, predicate, vars, opts) is generated on every JsonSearch field and returns eql.JSONSearchPredicateQuery, which binds as col @@ $1::eql_v4.query_json_predicate or eql_v4.json_tree_match(col, $1::...). The predicate crosses as text and vars as JSON text written by gensupport.JSONText, so Go adds no grammar: nil, or a value that writes null, sends no vars, and a Go string is refused as a Contains needle is. opts.WildcardBound bounds the filters that compile to the element match, as Elements.<Array>.Match takes it. The guest now names eql_v4.query_json_predicate, so the engine lists match (input predicate) on every JsonSearch field; the checker and stashgen read it, and stashgen refuses a document field the engine answers no match on. The guest maps Error::Predicate to STATUS_ENCODING: a refused predicate is the caller's input, not an internal failure. Diagnostic gains Construct and At, which read the construct and the byte offset the refusal carries. testusers.Entity grows the RC2 model's members (score, weight, address, country, preferred languages; $.weight ordered). The PostgreSQL test runs the seven filters of the customer's request and checks @@, json_tree_match, the per-operator query and jsonb_path_match over the plaintexts return the same rows; it joins go:test:postgres. Claude-Session: https://claude.ai/code/session_01MrxG73fnX1p7oiizh3GRG5
The stashgen README's "Encrypted JSON documents" table gains a Match row with both SQL forms, and a section on the grammar, the refusals and what Diagnostic's Reason, Construct and At give, the four places the answer parts from jsonb_path_match, the 22023 bound, and the index the per-operator form engages that json_tree_match does not. encrypt/README.md and doc.go say the same in short. The typed-document example finds Bea by one predicate that combines an ordering with the visit filter. The Go module's CHANGELOG records Match, the Diagnostic accessors and the redacted EQL types (#1443).
a7d14d2 to
8579e95
Compare
e65fca9 to
ae71ed2
Compare
Summary
A Go program can now send a whole JSON filter to an encrypted column as one SQL parameter, the way it would call
jsonb_path_match(col, $1, $2)on a plaintextjsonbcolumn. Before, each part of the filter was its own Go call, its own parameter and its own cast, and the application had to assemble theWHEREclause by hand. A filter compiler (such as a customer's AIP-160 engine, which turns Google's list-filter syntax into SQL) can now emit one template for both.Terms used below:
JsonSearchfield is a Go struct, map orjson.RawMessagefield stored as one encrypted JSON document in apublic.eql_v4_json_searchcolumn.$.country == $val && $.weight < $max. Its variables (vars) are the values named$valand$max.ordered=$.weight) whose numbers and strings get an ordering term, so the database can compare them without decrypting.This is the Go half of #1441 and the last issue of the "JSONB path match on encrypted JSON" milestone. The work it builds on (#1444, #1445, #1446 and #1451) is merged into
next, so this PR's diff is its own three commits. It does not change the engine or the EQL SQL.Changes
Public Go API
JsonSearchfield:Fields.<Field>.Match(ctx, cipher, predicate string, vars any, opts encrypt.JSONReadOptions) (eql.JSONSearchPredicateQuery, error).col @@ $1::eql_v4.query_json_predicate, oreql_v4.json_tree_match(col, $1::eql_v4.query_json_predicate). Both forms return the same rows.varsis any valueencoding/jsonwrites as a JSON object: a map, a struct, or ajson.RawMessageof one. It is written by the samegensupport.JSONTextthe other methods use.jsonb_path_match'svarsthrough pgx ordatabase/sqlanswers. A nil value (nil, or a nil map, slice, pointer orjson.RawMessage) is SQLNULLthere, so plaintext isNULLfor every row:Matchreturns an empty operand, which binds asNULL, and@@andjson_tree_match(both STRICT) match no row. The predicate is still compiled, so text the grammar does not read is still refused; a variable it names is not, as plaintext does not look for one. A value that writes the JSONnull(json.RawMessage("null"), a pointer to a nil map) is refused asjson_input, as plaintext refuses vars that are not an object. An empty map is the way to say "no variables".json.RawMessageis refused asjson_syntax, as{is.Containsneedle, because it writes one JSON string, not an object. One helper,isGoString, holds that rule for both.opts.WildcardBoundworks as it does forElements.<Array>.Match: 0 means the engine's default, 64.EqAt,Selector,At.<Path>andElements.<Array>.Matchalready derive.Errors
*encrypt.Diagnosticwrappingencrypt.ErrEncoding, with codestack_encrypt::dynamic_predicate. It is refused; it is never compiled to a tree that silently drops a leaf.Diagnostic.Construct()andDiagnostic.At()read the two fields the engine already sent.Construct()names the refused construct in the grammar's own fixed words (`like_regex`,a position ([2])).At()is the byte offset in the predicate where that construct starts; it reads the offset in any integer width the transport carries, or a float holding a whole number, and refuses a negative or fractional one.Reason()says why (json_predicate_unsupportedand the others). None of them carries text from the predicate or a variable's value.encrypt.ErrTermwith codestack_encrypt::json_path_not_ordered, as the per-path orderings report it.Guest and wiring
encrypt/guest/src/targets.rs) now names theeql_v4.query_json_predicatedomain. The engine therefore listsmatch(inputpredicate) on everyJsonSearchfield, with or without ordered paths.parseOperatorNameand the operator input parser inchecker.goacceptmatchandpredicate. So does stashgen'sjsonOperators.status.rsmapsError::PredicatetoSTATUS_ENCODING(bad input). Before this, it fell into the catch-all and a refused predicate was reported asErrInternal.Matchon everyJsonSearchfield. It stops with an error if the engine lists nomatchfor a document field: that would mean the generator and its engine disagree, the same rule feat(go): test several members of one array element of a JsonSearch field #1451 applies to a missingelement_match.Generated EQL types print redacted (outside the issue's strict scope; see Review notes)
encrypt/eqlnow hasString,FormatandLogValue, which all write one text:eql.TextEq{[redacted]}, oreql.TextEq{}when empty, and never the bytes.Formatanswers every verb,%#vincluded, so noGoStringis generated. Before this, every one of them was a bare[]bytetofmtandslog.eql.go.%sused to give a value's JSON. To get the JSON, convert the value (string(v),json.RawMessage(v)) or callMarshalJSON;Valueis unchanged. To print or log the JSON on purpose, callReveal()at that call (slog.Debug("stored", "email", v.Reveal())): every type has it, and it returns the JSON as a string, sofmtand everysloghandler print it as text.Tests
json_match_test.go, deterministic guest):and(contains, exists, not(contains), cmp.lt), and each leaf equals the operand its own method derives (EqAt,At.Weight.Selector,EqAtwithnil,At.Weight.Lt).Elements.ExtraEmergencyContacts.Matchat the same bound.varsgiven as a map, a struct, a pointer to a struct, ajson.RawMessageand amap[string]json.RawMessageall derive the same operand. With literals in place of variables,{}derives it too. Every kind of nilvars(untyped, map, pointers,json.RawMessage,[]byte, slice) gives the empty operand, which binds asNULL, for a predicate with variables or without; with nilvars, a syntax error and an unsupported construct are still refused.TestJSONPredicateMatchInPostgreschecks that the empty operand finds no row through@@andjson_tree_match. The comparison treats a containment needle'ssv/ellists as sets, because the engine shuffles them on every query.json_predicate_syntax,json_predicate_unsupported(5 constructs),json_predicate_unknown_variable,json_predicate_value_type(2),json_predicate_not_equal_at_wildcard,json_predicate_too_deep,json_path_position,json_path_no_wildcard,json_path_wildcardandjson_syntax.varsthat are not an object (json_input);varsthat are not JSON, an empty or whitespace-onlyjson.RawMessageincluded (json_syntax); the JSONnull, padded or not, and a pointer to a nil map (json_input);json_path_not_orderedfor a range and for an unordered filter member; a Go stringvars, before the engine is asked; andjson_wildcard_bound_zero, throughQueryTerm.TestJSONPredicateMatchInPostgres, added togo:test:postgres, whose tests are now named once in aTESTSarray that both the-runpattern and the pass-by-name check come from): the seven filters of the customer's request, with the predicates from the RC2 note written verbatim. Each runs four ways:@@,json_tree_match, the hand-written per-operator query, andjsonb_path_matchover the plaintext (stored beside each row). All four return the expected rows.a_document_names_the_leaf_element_and_predicate_domainsasserts the predicate domain is named andmatchis listed on fields with and without ordered paths.Error::Predicate→STATUS_ENCODING.matchthrough the query exports: one predicate deriveseql_v4.query_json_predicate, and alike_regexone isSTATUS_ENCODING.native_ops's catalog listing now includesmatch.Matchis generated on an ordered and an unordered document field, and an engine that answers nomatchis refused.testusers.Entitygains the RC2 note's members (score, weight, address, country, preferred languages), and$.weightis ordered.customer_stash.go,subscriber_stash.go,entity_stash.goand the example'scustomer_stash.goare regenerated.Docs
cmd/stashgen/README.md, "Encrypted JSON documents": aMatchrow with both SQL forms, and a section covering:jsonb_path_match(lax unwrapping not applied; lax wrapping of a non-array under[*]not applied; a string compared with a number is false under==and true under!=; orderings and multi-condition filters compare folded terms, so such a filter's==is of folded values; and an ordering of a JSONnullis NULL rather than false except under an odd number of!, which returns the same rows, since feat(eql): one-parameter jsonpath predicates on json_tree documents (json_tree_match, @@, match) #1446 sends thenullneedle only there);@>and->, not insidejson_tree_match, so a query that needs an index usesEqAt,ContainsandAt.<Path>.encrypt/README.mdandencrypt/doc.gosay the same in short. The typed-document example's README andmain.goadd aMatchcall.languages/golang/CHANGELOG.mdgainsAddedentries forMatchand the Diagnostic accessors, and aBreakingentry for the redacted types. The stashgen README lists everyReason()a refusal can give,json_syntaxandjson_inputincluded, one line each. The Go module records changes there, not in changesets.Verification
mise run wasm:guest:test(before the later restacks, which changed no guest source): fmt, clippy, rustdoc; nextest 73 passed (default features) and 78 passed (eql).wasm:guest:build,wasm:guest:build:eqland both deterministic builds):go vet ./...,go test ./...(every package ok),gofmt -l .(empty),mise run go:lint(0 issues), andgo generate ./...leaves the committed tree unchanged.mise run go:test:postgresagainst a local PostgreSQL 15 with the EQL v4 bundle built from this tree: all 7 tests passed, includingTestJSONPredicateMatchInPostgres, which also checks that nilvarsmatch no row through@@andjson_tree_match.Revealwas added; after it,cargo test -p eql-codegenandmise run types:checkpassed at every commit):cargo nextest run -p eql-codegen202 passed,cargo clippy -p eql-codegen --all-targets --all-features -D warningsandcargo fmt --checkclean.mise run types:checkandmise run codegen:parity(frompackages/eql) passed after commit.Related
Closes #1443
Review notes
%sto see its JSON now seeseql.X{[redacted]}; it converts the value (string(v)) or callsMarshalJSONinstead. Listed under Breaking.Reveal()is the named call for printing or logging the JSON, added after review: redacted by default, revealed only where a program asks for it by name.TermError::JsonPathNotOrdered. It carries no offset and no path text, as feat(eql): one-parameter jsonpath predicates on json_tree documents (json_tree_match, @@, match) #1446 left it, soAt()is false there; the docs name its sentinel and code. Wrapping it inError::Predicatewith the path's offset is a small engine follow-up, if wanted.varsmirror plaintext. Decided in review: checked against pgx v5.11 anddatabase/sql(pgx stdlib) over a plaintext column, where every Go nil binds as SQLNULL(sojsonb_path_matchis NULL and no row matches), the JSONnullis refused ("vars" argument is not an object), and an emptyjson.RawMessageis a syntax error.Matchgives the same three answers.encrypt/gensupport/json.go(JSONMatch),stashgen/emit.go(jsonMatch, and thematchcheck inplanJSON),encrypt/guest/src/status.rs, andjson_match_postgres_test.go.