Build a name bind as a NameData, not as text - #41
Open
sfc-gh-okalaci wants to merge 1 commit into
Open
Conversation
NAMEOID was handled in the same case group as TEXTOID, VARCHAROID and BPCHAROID,
all of which construct a varlena. `name` is not a varlena: it is a fixed-length
NameData of NAMEDATALEN bytes with no length word. Building one as text writes a
varlena header into the first bytes of the value, and anything that compares it
against a real name reads that header as characters.
Nothing raises, which is what makes it costly. The value simply never matches, so
a catalog lookup driven by a JavaScript string finds nothing:
pljs.execute('SELECT nspname FROM pg_namespace WHERE nspname = $1',
['pg_catalog']) -- 0 rows
pljs.execute('SELECT $1::name::text AS t', ['hello_name'])[0].t
-- '8'
Catalog introspection by name -- nspname, relname, typname, attname -- is how most
of it is written, so this affects a lot more than the round trip.
NAMEOID now gets its own case and goes through namein(), which lays the value out
correctly and applies the truncation rule at NAMEDATALEN - 1 rather than raising.
The QuickJS string is released on the error path too, so a rejected value does not
leak it.
Adds sql/pg_name_bind.sql, covering a catalog lookup by name, the round trip, the
empty name, truncation of an over-long value, and returning a name from a function.
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.
Stacked on #40.
NAMEOIDwas handled in the same case group asTEXTOID,VARCHAROIDandBPCHAROID, all of which construct a varlena.nameis not a varlena — it is afixed-length
NameDataofNAMEDATALENbytes with no length word. Building one as textwrites a varlena header into the first bytes of the value, and anything comparing it
against a real name reads that header as characters.
Nothing raises, which is what makes it expensive to find. The value simply never
matches:
Catalog introspection by name —
nspname,relname,typname,attname— is howmost of it is written, so the blast radius is wider than the round trip suggests: any
such query silently returns nothing.
NAMEOIDnow gets its own case and goes throughnamein(), which lays the value outcorrectly and applies the truncation rule at
NAMEDATALEN - 1instead of raising. TheQuickJS string is released on the error path as well, so a value
namein()rejects doesnot leak it.
Test plan
sql/pg_name_bind.sqlcovers a catalog lookup by name, the round trip, the empty name,truncation of an over-long value, and returning a
namefrom a function.It discriminates: with the conversion reverted, both catalog lookups return
0insteadof
1, and the round trip returns8and\x10instead of the names. Full suite greenon PostgreSQL 17; builds clean on 16, 18 and 19beta3.