Take composite OIDs from the caller instead of guessing 16385 - #102
Merged
Conversation
`UserDefined.oid()` returned a hard-coded 16385. Postgres tolerated it only because it assumes an OID it does not recognise belongs to a sender-side type — and 16385 is the *first* OID a cluster hands out for user objects, so on any database that has ever created one, `record_recv` found a real and different type there and rejected the COPY. Fresh test clusters never had one, which is why nothing caught it. pgpq cannot know a composite's OID: the server allocates it when the type is created. So the caller supplies it. * `PostgresType::UserDefined` carries `oid: Option<u32>`. * `StructEncoderBuilder::with_oid`, and `ArrowToPostgresBinaryEncoder::with_composite_oids` for a whole map at once, keyed by the type name the generated DDL uses (`<field>_t`). Unknown names are reported rather than ignored, so a typo fails instead of silently leaving the wrong OID. Exposed to Python as `with_composite_oids` too. * Encoding a nested composite without its OID is now an error naming the type and the query that finds it, rather than putting a guess on the wire. Building the tree still works — the OID is supplied afterwards — so only shapes that can never carry an OID (arrays of composites or of arrays) still fail at construction. The roundtrip harness now creates the types, asks `pg_type` for the OIDs it just allocated and encodes with those, so the suite no longer depends on a composite landing on any particular OID. `nested_composite_uses_the_databases_own_oid` creates unrelated types first to force the composite elsewhere, and asserts the encoded bytes follow the server. The byte-exact snapshots still encode with 16385 — that is what produced them and what a fresh cluster allocates — but the value now lives in the test harness where it is visible, not in the library where it applied to everyone's database. BREAKING: nested composites require their OID; `PostgresType::UserDefined` gained a field.
adriangb
enabled auto-merge (squash)
August 15, 2026 04:41
CI caught `test_roundtrip_structs`: the Python roundtrip encoded before running
the DDL, so a nested struct had no OID to encode with. Same ordering problem the
Rust harness had, and the same fix — create the types, ask the server, encode.
Doing that from Python needed a way to know *which* type names to look up, and
the Python `UserDefined` exposes no field traversal. Rather than make every
caller write one, the encoder now answers the question directly:
for name in encoder.composite_type_names():
oids[name] = look_up(name)
encoder.with_composite_oids(oids)
`composite_type_names()` is on the Rust encoder too, and both test harnesses now
use it instead of the two hand-written schema walks this branch had grown.
Verified with a real server this time: `PATH=~/.theseus/postgresql/*/bin` (the
trick DEVELOPMENT.md already documents) runs the 9 Postgres-backed Python tests
that are otherwise skipped locally. All 25 pass.
# Conflicts: # core/tests/integration_tests.rs
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 #96 (item 1; item 2,
Json.oid(), is #100). Breaking, deliberately batched into thepending semver-major.
The bug
PostgresType::UserDefined::oid()returned a hard-coded16385. Postgres tolerated that onlybecause it assumes an OID it does not recognise belongs to a sender-side type — and 16385 is
the first OID a cluster hands out for user objects. On any database that has ever created a
user-defined type,
record_recvfinds a real (and different) type at 16385 and rejects the COPY.Fresh embedded test clusters never have one, which is why the suite never saw it.
pgpq cannot know a composite's OID: the server allocates it when the type is created. So the
caller supplies it.
API
PostgresType::UserDefinedcarriesoid: Option<u32>.StructEncoderBuilder::with_oidfor one,ArrowToPostgresBinaryEncoder::with_composite_oidsfor a whole map — keyed by the type name the generated DDL uses (
<field>_t), which is whatyou look up in
pg_typeafter running the DDL. Unknown names are reported, so a typo failsloudly instead of silently leaving the wrong OID in place.
ArrowToPostgresBinaryEncoder.with_composite_oids(mapping), with a stubentry.
Behaviour change
Encoding a nested composite without its OID is now an error that names the type and the query
that finds it:
Building the encoder tree still succeeds — the OID arrives afterwards — so only shapes that can
never carry an OID (arrays of composites, arrays of arrays) still fail at construction, as
before. This is the one judgement call worth flagging: it turns a silent wrong-OID-on-the-wire
into an explicit error, at the cost of requiring nested-struct callers to do a lookup they
previously got away without on a fresh database.
Tests
pg_typefor the OIDs it just allocated, andencodes with those — so the suite no longer depends on any composite landing on a particular
OID.
nested_composite_uses_the_databases_own_oidcreates unrelated types first to force thecomposite off 16385, asserts the real OID differs, asserts the encoded bytes carry it (and
differ from the placeholder encoding), and roundtrips successfully.
test_nested_struct_needs_composite_oidscovers the raise, the success with the OIDdeclared, and the unknown-name error.
The byte-exact snapshots still encode with 16385 — that is what produced them and what a fresh
cluster allocates — but that value now lives in the test harness where it is visible, rather than
in the library where it applied to every user's database.