Repository navigation
Render JSON path keys as inline literals - #134
Open
jonassvalin wants to merge 2 commits into
Open
jonassvalin wants to merge 2 commits into
jonassvalin wants to merge 2 commits into
Conversation
Renders a scalar value into the SQL text via psycopg's quoting instead of as a bind parameter. Percent signs are doubled because psycopg re-parses composed queries for placeholders whenever parameters are passed, which would otherwise reject or silently rewrite values containing '%'.
expression_for_path now renders path sub-levels as text literals rather
than bind parameters, so jsonb_extract_path("state", 'key') matches
expression indexes under generic plans, which psycopg's automatic
statement preparation can lead Postgres to use.
Integer sub-levels are rendered as text, which jsonb_extract_path treats
as array indices. Previously they were bound as smallint and failed with
"function jsonb_extract_path(jsonb, unknown, smallint) does not exist".
Boolean sub-levels are rejected rather than rendered as 'True'.
This branch has not been deployed
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.
Summary
The Postgres query converters now render JSON path keys inline, as
jsonb_extract_path("state", 'key'), instead of as bind parameters. Expression indexes can then be used under generic (prepared) plans. This also fixes filters and sorts on paths with integer segments, which currently fail in Postgres.This is the second of four PRs from the plan in #132. It doesn't depend on #132. The README paragraph for this change extends the section #132 adds, so I'll add it once #132 merges.
Motivation
psycopg prepares a statement after it has run five times on a connection. Postgres may then switch to a generic plan, where a bound key renders as
jsonb_extract_path(state, VARIADIC ARRAY[$2]). No expression index can match that expression, so the planner falls back to the wrong index or a sequential scan. A downstream service saw 4.3s per list query locally in this state. With the key inline, the expression folds to the same constant as an index defined onjsonb_extract_path(state, 'key'), and the index matches.Integer path segments, such as
Path("state", "values", 0), are allowed byPathbut fail today. psycopg binds theintassmallint, and Postgres has nojsonb_extract_path(jsonb, unknown, smallint). Rendered as the text'0', the segment indexes into the array as intended.Changes
Key Changes
InlineLiteralquery node: renders a scalar inline through psycopg's quoting, with no params. It isn't exported from the package.expression_for_pathrenders every path sub-level as anInlineLiteral. This covers filters (including thejsonb_extract_path_textforms), sorts, similarity and key-set paging.boolsub-levels raiseValueError, rather than silently rendering as'True'.Implementation Details
%handling. Every adapter callscursor.execute(query, params)with a params list, so psycopg scans the whole SQL text, inline literals included, for%placeholders. Without escaping, a key like'50%x'raisesProgrammingError, and'a%%b'silently becomes'a%b', so the query reads a different key.InlineLiteraltherefore doubles%, and psycopg collapses it back when it binds the query.sql.Literal.Breaking Changes
QueryConverter.convert_querychange for nested paths: path keys move from the params into the SQL. Tests or code that assert on, or post-process, rendered queries need updating.Pathwith aboolsub-level now raisesValueErrorinstead of failing in Postgres.Migration Guide
Update any expectations on rendered SQL. For example:
"jsonb_extract_path"("state", %s) = …with params["value", 5]"jsonb_extract_path"("state", 'value') = …with params[5].Existing expression indexes keep matching. Under custom plans the expressions are identical once parameters are substituted, and under generic plans they now match where they didn't before.
How to Verify
Automated Verification
mise run types:checkmise run lint:checkmise run format:checkNew tests:
InlineLiteralrenders plain values, quotes, backslashes,%,%sand%%inline with no params, including as function arguments.%;boolsub-level rejection;mainwith thesmallinterror.',%sand%%: a regression guard; it proves escaping round-trips through Postgres.tests/integration/.../persistence/postgres/test_query_plans.py:(name, jsonb_extract_path(state, 'kind'))index, runsANALYZE, and checks the plan withEXPLAIN (GENERIC_PLAN, FORMAT JSON).Index Condcontainsjsonb_extract_path, not just that the index is used, since it could be scanned onnamealone.Manual Verification
plan_cache_mode = force_generic_plan.Checklist
Related Issues
None. The plan and its review are in #132, under
meta/plans/andmeta/reviews/plans/.