Repository navigation
fix(postgres): expand typed json and nullable casts on the client - #80
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
ℹ️ No critical issues. One minor doc-accuracy suggestion inline.
Reviewed changes
I reviewed how structured::present changed and how it decides which path a Postgres statement takes when the server's pgreylo extension has the same engine.
presentcovers every Postgres rewrite: it now also matches::json(…)/::jsonb(…)and::nullable(…), using the same helpers asfind. On Postgres, the casts it matches are now exactly the ones the pass rewrites.- Client-side analysis for rewritten statements: with
Server::Same, such a statement is now expanded and analyzed on the client, and no longer goes to the server throughANALYZE_QUERY. So.run()no longer receives$1::nullable(int)or$1::jsonb('…')in the text. - Routing test: the test checks the need ids for
ANALYZE_QUERY. Bundled needs go throughANSWER_QUERY, which has a different prefix, so the negative asserts actually test something. Plain::intand::jsonbstill go to the server. - Artifacts: all three
engine.wasmfiles are rebuilt, as a change underengine/core/requires.
claude-opus-5-5 ｜ 𝕏
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run covered the commits pushed since the last Pullfrog review (ac9fd4f), which apply the earlier doc-accuracy suggestion.
- Made
presentmatchfindon MySQL and SQLite: a plain::typeafter a placeholder now counts as a rewrite on dialects that have no::. This uses the samedialect != Dialect::Postgres && plain_cast_at(…)check asfind's own plain-cast branch, so the doc now holds on all three dialects. Postgres routing is unchanged. - Added
present_is_whether_the_pass_rewrites_the_statement: covers each cast the pass rewrites on all three dialects, and checks that a plain::intcounts on MySQL and SQLite but not on Postgres. - Rebuilt the three engine artifacts: required because the change is under
engine/core/.
claude-opus-5-5 ｜ 𝕏
colinhacks
force-pushed
the
structured-present
branch
from
October 23, 2026 20:16
c0a189e to
444e961
Compare
colinhacks
force-pushed
the
structured-present
branch
from
October 23, 2026 20:32
444e961 to
b1e84a4
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.
When the database server carries the same engine (the handshake in
engine/postgres/src/extension.rs), a Postgres statement is analyzed by the server whole, and the client rebuilds the statement's text with the lexer alone. Only a statement with a structured parameter was kept on the client, sincestructured::presentrecognised::rows(…)and::row(…)and nothing else. A typed JSON parameter (:doc::jsonb('…')) and a cast tonullable(…)(#79) went to the server too, and came back with the cast still in the text:.run()would send$1::nullable(int)or$1::jsonb('{…}')to Postgres, which refuses both.presentnow also recognises a typed JSON cast and anullable(…)cast, so such a statement is expanded and analyzed on the client, with the server answering its needs, as a structured parameter already is. A plain cast,::jsonbincluded, still goes to the server whole.engine/core/src/structured.rs:presentchecks the three casts the pass rewrites.engine/postgres/src/api.rs: a test that, with the same engine in the server,::rows(…),::jsonb('…')and::nullable(…)(in any spacing and case) never ask for the server's analysis, and::intand::jsonbdo.Checked:
cargo build --workspace --all-targetswith no warning, the engine tests, the full oracle with--strict-coverage,nub run typecheck,nub run test(none skipped), andnub run build. A parity run between the client engine and a server carrying this engine showed the two analyses differing on anullable(…)statement before this change.