Skip to content

Fix pgrx APIs that let safe code cause undefined behavior - #2400

Draft
eeeebbbbrrrr wants to merge 83 commits into
codex/generated-c-macro-migrationfrom
macro_rules-safety-fixes
Draft

eeeebbbbrrrr wants to merge 83 commits into
codex/generated-c-macro-migrationfrom
macro_rules-safety-fixes

Conversation

@eeeebbbbrrrr

Copy link
Copy Markdown
Contributor

Several pgrx APIs let safe code read freed, misaligned or mistyped PostgreSQL memory. Values that borrowed from arrays, tuples, relcache entries and varlenas are now copied into Rust or into CurrentMemoryContext, and PgBox reads check the type's length and pass-by-value flag. APIs whose soundness depends on their inputs, such as AnyArray::into, heap_getattr, composite_row_type_make_tuple and the generated <type>_recv functions, are now unsafe fn. rust_regtypein finds an extension's own types through pg_depend instead of search_path, and fn_call checks argument types against the called function before calling it. These change signatures and behavior, and v20.0-MIGRATION.md describes each change and how to update code. AnyArray and AnyElement still do not carry the lifetime of the memory they read, so an AnyArray made inside Spi::connect must not be used after the closure returns. Stacked on #2390.

@eeeebbbbrrrr

Copy link
Copy Markdown
Contributor Author

Notes for review.

PgVarlena cost. PgVarlena<T> now holds its value in a Box<T>. Holding a pointer into PostgreSQL memory let SPI and set-returning-function callers mutate freed memory, and an inline MaybeUninit<T> loses zeroed padding when it moves. Whole-query times on non-assert PG18.6, release build with fat LTO, 1M rows, best of four rounds:

Workload Before After
1M reads of a 16-byte type 21.13 ms 19.12 ms (-9.5%)
2M reads in a comparison-shaped call 27.79 ms 22.55 ms (-18.8%)
1M reads of a 168-byte type 29.08 ms 26.00 ms (-10.6%)
1M read, mutate and return 18.38 ms 22.15 ms (+20.5%, about 3.8 ns per call)

The read, mutate and return case is slower because into_pg now always pallocs a new varlena. These were measured before the branch was split from #2390, at 0cbafb56 against 10e6a4a0, and later commits changed how the payload is read, so they have not been re-measured on this tip.

Behavior changes that are easy to miss.

  • PgHeapTuple::get_by_index reads a dropped attribute as Ok(None), as SQL field selection does, and set_by_index on one returns Err(NoSuchAttributeNumber).
  • get_by_index::<PgHeapTuple> on an attribute that is not a row type returns Err(IncompatibleTypes) instead of crashing the backend.
  • DatumWithTypeInfo::into_value, htup::heap_getattr and heap_getattr_datum_ex are unsafe fn.
  • PgVarlena::new requires T: Default; new_zeroed (unsafe) and From<T> are added, and into_pg always returns a new varlena.

Known gap. An AnyArray made inside Spi::connect, for example by AnyElement::value::<AnyArray>(), points into SPI's memory context and must not be iterated after the closure returns. Before this PR the hazard ran the other way: elements from an array made outside the closure and iterated inside it dangled afterwards. Closing it needs a lifetime parameter on AnyElement and AnyArray, which is left for a separate change.

@eeeebbbbrrrr
eeeebbbbrrrr force-pushed the codex/generated-c-macro-migration branch from 29af139 to a8d276f Compare October 29, 2026 08:13
@eeeebbbbrrrr
eeeebbbbrrrr force-pushed the macro_rules-safety-fixes branch from f48834e to a894ddc Compare October 29, 2026 08:13
@eeeebbbbrrrr
eeeebbbbrrrr force-pushed the codex/generated-c-macro-migration branch from a8d276f to f557971 Compare October 29, 2026 17:05
@eeeebbbbrrrr
eeeebbbbrrrr force-pushed the macro_rules-safety-fixes branch from a894ddc to 2689bf5 Compare October 29, 2026 17:05
The function was safe but dereferenced any non-NULL Datum, so safe code could pass an arbitrary address. It also used pg_detoast_datum_packed, which leaves a stored composite's one-byte header in place, and then read the header as four bytes, producing a garbage t_len and a misaligned t_data. It is now an unsafe fn whose caller supplies a live composite Datum, and it uses pg_detoast_datum like PostgreSQL's DatumGetHeapTupleHeader. This is a breaking change for callers, who must add an unsafe block, and v20.0-MIGRATION.md records it. A new test reads a packed composite column through SPI, and the test of a row stored before ALTER TYPE ... ADD ATTRIBUTE now also reads it through htup::heap_getattr.
PgVarlena handed out &T and &mut T at the varlena payload, which PostgreSQL places one or four bytes past an aligned allocation, so safe Deref and DerefMut created misaligned references for most types. It now copies the payload into an aligned Rust value, lends references to that copy, and writes changes back when the value becomes a Datum. A mutable borrow of a value read from a Datum allocates its own varlena in CurrentMemoryContext, instead of calling GetMemoryChunkContext on memory that may belong to a shared buffer. from_datum also rejects a payload shorter than T. New tests cover one-byte headers, compressed values, alignment above palloc's, and copy-on-write; they replace the test that expected changing a misaligned borrowed value to raise an error. v20.0-MIGRATION.md describes the new behavior.
PostgreSQL 17 returns SPI_OK_MERGE_RETURNING for MERGE ... RETURNING, which SpiOkCodes did not list, so Spi::check_status panicked after the statement succeeded. SpiOkCodes gains a MergeReturning variant on PG17-19, and a test runs MERGE ... RETURNING through SPI. v20.0-MIGRATION.md lists the new variant.
The example's lock guard released its LWLock in Drop unconditionally, also while a converted PostgreSQL ERROR unwinds. errfinish has reset InterruptHoldoffCount to zero by then, so LWLockRelease fails its RESUME_INTERRUPTS assertion on assert-enabled builds. The guard now releases only while InterruptHoldoffCount is positive, as pgrx's own lock guards do, and leaves the lock to transaction abort otherwise. Its documentation and SAFETY comment now describe that check instead of the failing assertion.
A PgVarlena read from a Datum borrowed it, and its first mutable borrow allocated a new varlena in whatever memory context was current. Changing a value read through SPI inside Spi::connect therefore left it pointing into the procedure context that SPI_finish deletes, and an SRF that changed a captured argument in a row allocated in the per-row context and later pfree'd the reset chunk; both are use-after-free from safe code. PgVarlena now copies the payload, and any bytes after the T, into Rust storage and holds no pointer into PostgreSQL memory, so into_pg and IntoDatum allocate a new varlena in CurrentMemoryContext. A plain or packed value is copied from its Datum without detoasting, and a compressed or external value is detoasted into a temporary copy that is freed at once. Changing a value no longer drops the payload bytes after the T when the Datum had a four-byte header. New pg_tests cover the SPI and SRF cases and the trailing bytes; they crash, raise "pfree called with invalid pointer", or lose the tail on the previous code.
PgVarlena::new and Default were safe but started from all-zero bytes, which is undefined behavior for any T whose zero bytes are invalid, such as NonZeroU32, a reference, or an enum without a zero discriminant. new now requires T: Default and starts from T::default(), and Default calls it. The unsafe new_zeroed keeps the old zero-filled start for types where that is valid, and From<T> wraps a value the caller already built. In-tree input functions and examples now build their value and convert it, or derive Default. This is a breaking change for callers of new on a type without Default, which v20.0-MIGRATION.md records.
pgrx::htup::heap_getattr and heap_getattr_datum_ex were safe, but they read the tuple's header and data at offsets taken from the descriptor. Safe code could pass PgBox::null() or a tuple of another row type and read through a null or unrelated pointer. Both are now unsafe fn with a Safety section requiring a live tuple formed with the descriptor, matching composite_row_type_make_tuple, and their SAFETY comments rely on it instead of naming what they could not check. Their docs no longer claim system attribute support. This is a breaking change, recorded in v20.0-MIGRATION.md.
…_index

PgHeapTuple::get_by_index kept a &FormData_pg_attribute from the descriptor while heap_getattr_raw ran, then read its type. On PG15-17, nocachegetattr caches offsets by writing attcacheoff into those same attributes, so the shared reference was live across a write to its memory. get_by_index now copies the type OID first, as htup::heap_getattr does, and no longer uses the reference after the read.
The guard skips LWLockRelease when a PostgreSQL ERROR has reset InterruptHoldoffCount, but no test raised an ERROR while it held the lock. A new pg_test holds XactTruncationLock through the guard and calls format_type_extended for type 0. With an unconditional LWLockRelease, the assert-enabled PG18 test server aborts on the RESUME_INTERRUPTS assertion; with the check, the test sees the expected error.
A PgVarlena stores every byte of its value, padding included. new, From and Default copy a typed T, so their padding is unspecified, and even new_zeroed could lose its zero padding when the inline MaybeUninit<T> was moved; a debug build stored stack bytes there. The value is now a Box<T>, so moving a PgVarlena never copies it as a typed value: padding read from a Datum is kept, and new_zeroed followed by field writes stores zero padding. This adds one Rust heap allocation per value, as develop's borrowed PgVarlena had, and removes the unsafe assume_init calls from AsRef and AsMut. The PgVarlena docs and v20.0-MIGRATION.md now say which constructors leave padding unspecified and recommend types without padding, and the operators example gains an explicit padding field. A new unit test checks the zero padding in the bytes into_pg copies, and it fails on the previous representation.
PgHeapTuple::get_by_index called T::type_oid() before reading the attribute, so a NULL attribute no longer returned Ok(None) without it. For a PostgresType or PgVarlena<T>, type_oid() looks the type up by name and raises an ERROR when the type is not on search_path. get_by_index now copies only the attribute's type OID before heap_getattr_raw, so no descriptor reference lives across nocachegetattr, and calls T::type_oid() only for a non-NULL value. A new pg_test reads a NULL attribute whose type is in the tests schema; before this change it raised "type "offsearchpath" does not exist".
The Safety sections of htup::heap_getattr and heap_getattr_datum_ex required a tuple formed with the given descriptor, which excluded the row stored before ALTER TYPE ... ADD ATTRIBUTE that a pg_test reads with the current descriptor. PostgreSQL reads such rows: the attributes a tuple stores must match a prefix of the descriptor's, and the rest come from getmissingattr as the recorded missing value or NULL. Those Safety sections, the SAFETY comments in heap_getattr, heap_getattr_datum_ex and PgHeapTuple::get_by_index, and v20.0-MIGRATION.md now state that prefix contract, and heap_getattr's Safety section also requires that T can read the stored value. Because PostgreSQL keeps a dropped attribute's length and alignment in place, a new pg_test reads a row stored before ALTER TYPE ... DROP ATTRIBUTE. An intra-doc link to heap_getattr in heap_getattr_datum_ex's docs, which rustdoc found ambiguous with the generated macro, now names the function.
… lifetime

The Safety sections of htup::heap_getattr and heap_getattr_datum_ex required the same attribute types, yet said a row stored before DROP ATTRIBUTE qualifies, although PostgreSQL clears a dropped attribute's type. They now state the contract that heap_getattr_raw already states: each stored attribute has the length, alignment and pass-by-value flag of the descriptor's attribute in that position, no reference to the descriptor's attribute entries may be live across the call, and a by-reference result can point into the descriptor before PostgreSQL 15.5. heap_getattr_raw now refers to heap_getattr for the stored rows that qualify. The get_by_index and pg_test SAFETY comments and v20.0-MIGRATION.md use the same wording. The get_by_index comments also name two existing gaps in that safe API: attribute references a caller holds across the attcacheoff write, and the unchecked attribute type when T reads records.
The upgrade note in v20.0-MIGRATION.md said that new, From and Default now copy a typed T with unspecified padding, which implied that Default changed. The old Default already assigned T::default() over the palloc0 bytes, a typed copy, so its padding was never defined; only new changed, and From is new. The note now says so, so an upgrader does not expect new_zeroed to reproduce bytes that Default used to store.
PgHeapTuple::get_by_index passed RECORDOID to try_from_datum whenever the requested Rust type reads records, such as PgHeapTuple or composite_type!. PgHeapTuple accepts RECORDOID, so the attribute's own type was never checked, and reading an int4 attribute as a PgHeapTuple treated the integer as a tuple pointer and crashed the backend. get_by_index now uses RECORDOID only when the attribute is a row type: a record, a composite type, or a domain over a composite type. Any other attribute keeps its own type, so the read returns IncompatibleTypes. get_by_name reads through get_by_index and gets the same check. New pg_tests cover the int4 attribute, which crashed the backend before this change, and composite and domain attributes, which still read.
DatumWithTypeInfo has public fields, and its safe into_value read the Datum as any FromDatum type, so safe code could wrap an arbitrary Datum and read it as a String. into_value is now an unsafe fn whose Safety section requires a NULL, or a live value of typoid that T can read, with the same lifetime rules as heap_getattr_datum_ex. It still panics on a NULL, which is now documented. The one in-tree caller, a unit test, now states why its call is sound, and v20.0-MIGRATION.md records the change.
…ptor

heap_getattr_datum_ex looked up typlen and typbyval with get_typlenbyvalalign on the attribute's type OID. PostgreSQL sets a dropped attribute's type to InvalidOid, so reading one from a row stored before the drop raised "cache lookup failed for type 0", and every call cost a syscache lookup. The function now copies attlen and attbyval from the descriptor's attribute before it reads the tuple. Those equal the type's values for a live attribute, and PostgreSQL keeps them for a dropped one, which reports PgOid::Invalid as its typoid. The dropped-attribute pg_test now also reads that attribute through heap_getattr_datum_ex, which raised the ERROR before this change.
PostgreSQL sets a dropped attribute's type to InvalidOid, but a row stored before the drop still holds the old value. get_by_index checked that value's type against T, so reading the attribute raised "cache lookup failed for type 0" for most T, and "type with OID 0 does not exist" for a T that reads records, instead of returning a Result. get_by_index now returns Ok(None) for a dropped attribute before reading the tuple. That matches SQL field selection, which reads a dropped attribute as NULL, and matches what rows stored after the drop already returned. The dropped-attribute pg_test now reads the attribute as String and as PgHeapTuple, and both reads raised an ERROR before this change.
PostgreSQL sets a dropped attribute's type to InvalidOid, and set_by_index checked the value's type against it. For most Rust types that check raised "cache lookup failed for type 0" instead of returning an error. A Rust type whose IntoDatum::type_oid is also InvalidOid, such as a two- or three-element tuple, passed the check, and set_by_index stored its Datum in the dropped attribute using the dropped type's length and by-value flag. set_by_index now returns NoSuchAttributeNumber for a dropped attribute, and set_by_name, which goes through it, does the same. A new pg_test sets a dropped attribute with a &str, which raised the ERROR before this change, and with a tuple, which was stored before this change.
…upgrade note

v20.0-MIGRATION.md says it describes every fix that changes a signature or behavior, but it left out several. It now says that PgHeapTuple::get_by_index reads a dropped attribute as None, that set_by_index and set_by_name reject one, and that a record-reading T gets IncompatibleTypes for an attribute that is not a row type. It also says that PgVarlena::from_datum panics on a short payload, and that heap_getattr_raw's Safety section states the same requirements as heap_getattr's.
PgTupleDesc::get and iter, and PgHeapTuple::attributes, get_attribute_by_index, and get_attribute_by_name, returned references into the descriptor's attribute array. On PostgreSQL 15 through 17, reading a tuple with that descriptor can make nocachegetattr write attcacheoff into those attributes, so safe code that held one of the references across PgHeapTuple::get_by_index, or across any other read of a shared relcache or typcache descriptor, had undefined behavior. These methods now return pg_sys::FormData_pg_attribute by value, read through raw pointers without creating a reference into the descriptor. This is a breaking change: callers get a copy taken at the time of the call, and code that names the reference types, calls copied() on the iterator, or keeps a borrow such as get(0).unwrap().name() past the statement needs updating. PostgreSQL 18 and later cache offsets in CompactAttribute instead and use the same by-value API. A new pg_test holds an attribute across such a read, and on PostgreSQL 15 through 17 the held reference saw PostgreSQL's write before this change.
FromDatum::try_from_datum raised a PostgreSQL ERROR instead of returning IncompatibleTypes when the type OID named no type, because IsBinaryCoercible, PgHeapTuple::is_compatible_with, and the type name lookup for the error all fail on a missing pg_type row. It also accepted InvalidOid for a Rust type whose own type OID is InvalidOid, such as Array<Array<i32>>, and then read each int4 element as an array pointer. InvalidOid now matches no Rust type, including AnyElement, an unknown OID is checked with get_typtype before IsBinaryCoercible, and the error names these types "-" and "???". PgHeapTuple::is_compatible_with now uses get_typtype instead of lookup_type_cache. AnyArray and AnyElement hold InvalidOid when their function is called without an expression tree, as fn_call does.
…ment::value

AnyArray::into was a safe function that read the array as any FromDatum type without checking the array's type. Reading an int4[] as Array<PgHeapTuple> crashed the backend with SIGSEGV, and reading it as String produced a String holding invalid UTF-8. AnyArray::into is now unsafe, which breaks callers that use it outside an unsafe block, and it documents the same contract as AnyElement::into. The new AnyArray::value and AnyElement::value return Result<Option<T>, TryFromDatumError> after the type check of FromDatum::try_from_datum, so they reject InvalidOid and accept domains. They are not named try_into because TryInto::try_into, in the prelude, wins method resolution on an owned handle. Both handles are still Copy and 'static and can outlive the memory they point into, as a handle from Spi::get_one does. The type documentation now describes that gap.
Iterating over &AnyArray built an Array that detoasted the array into a fresh copy when it had a 1-byte header or was compressed, external, or expanded, as a text[] read from a table usually is. The yielded AnyElement handles point into that copy, which the iterator freed when it dropped. Reading an element after collecting them returned freed memory, and returning one from a function crashed the backend. Iteration now detoasts the array itself, so the copy stays allocated in CurrentMemoryContext until that context is reset, as with PG_GETARG_ARRAYTYPE_P. Each iteration over such an array therefore keeps its copy until then instead of freeing it.
The upgrade note now says that FromDatum for pg_sys::TransactionId keeps the Datum's low 32 bits, so an xid of 2^31 or more read from a tuple is no longer None.
test_anyarray_from_spi_outlives_connection read a 73-byte array, which SPI's tuple stores with a 1-byte header. Since AnyArray's from_polymorphic_datum detoasts such a value, that detoast alone made a copy outside SPI's results, so the test passed without the copy that from_datum_in_memory_context makes. The test now reads an array of more than 126 bytes, which keeps its 4-byte header and is read in place unless the override copies it.
The upgrade note said only int2, int4 and "char" elements changed. Sign-extending applies to every 1-, 2- or 4-byte pass-by-value element type, so the raw Datum of an oid or xid of 2^31 or more, a negative date or a negative float4 now has its high bits set, as one read from a tuple already did. The note now names those types and says to keep a raw Datum's low 32 bits instead of rejecting it with u32::try_from. The xid test's comment also says that array elements are now sign-extended too.
An AnyArray now detoasts a compressed, out-of-line or short-header array once, into the CurrentMemoryContext in effect when the handle is made. Before, iteration copied the array again on each pass, in the context current then. A handle made inside Spi::connect, for example by AnyElement::value::<AnyArray>(), therefore points into SPI's memory context, which is freed when the connection closes. The AnyElement::value docs and the upgrade note now say to make the handle in the context where it is iterated.
AnyElement::value and AnyArray::value return IncompatibleTypes when the type does not match, but reading Array<PgBox<U>> or Vec<PgBox<U>> panics in Array when PgBox<U> cannot copy the element type. Their Panics sections now say so.
The PgBox layout test's OverAligned tuple struct made cargo check --tests warn that its field is never read. The field only gives the struct its size, so it now has an underscore-prefixed name.
heap_getattr's and DatumWithTypeInfo::into_value's Safety sections bounded a borrowed T only by the tuple and descriptor. A compressed or out-of-line value read as &str is detoasted into a copy in CurrentMemoryContext, so the caller must also keep that context alive. get_by_index's docs and SAFETY comment now say that 'tup does not keep such a copy alive, which is a known gap that this change does not fix.
The upgrade note says every PgTupleDesc constructor panics on a null pointer, but from_pg_copy passed the pointer to CreateTupleDescCopyConstr first, which dereferenced it and crashed the backend. It now checks the pointer before the copy. Its Safety section now states that the pointer must be NULL or a live descriptor, and the SAFETY comment states what the copy relies on.
…tattr's read

PgRelation::with_lock, Drop for PgRelation and heap_getattr's final conversion were rewritten without SAFETY comments. They now say what each call relies on: the caller's lock choice for relation_open, the relcache reference and lock that Drop releases, and the caller's guarantee that T can read the attribute. The Drop comment also names the known to_owned bug, which can set need_close on a relation that the value does not own.
PgRelation::tuple_desc's example wrapped RelationIdGetRelation's reference in from_pg, which leaks the reference, takes no lock, and breaks from_pg's contract; it now uses with_lock. from_pg no longer says that PostgreSQL will close the relation later. FnCallArg::as_datum now says that fn_call only converts Arg::Value with it, while its Null and Default arms stay for direct callers. fn_call's Limitations now list VARIADIC parameters, which UnsupportedArgumentModes's message also names, and say that a set-returning function raises a PostgreSQL ERROR instead of returning an FnCallError. The wal_decoder SAFETY comments now reflect that tuple descriptor iteration returns copies, which attname may borrow.
…irst

test_get_by_index_reads_null_without_type_lookup relied on OffSearchPath's type_oid raising an ERROR because its schema was not on search_path. Since type lookups find any type of the extension, that lookup succeeds and the test passed even with the type looked up before the NULL check. The test now reads the NULL as PgBox<NotAnSqlType>, a Rust type with no SQL type whose type_oid always raises an ERROR, and the unused OffSearchPath type is gone.
A PostgresType's type_oid now searches the extension's types, which took about 24 us per call on an assert-enabled PG18 build. fn_call looked up each Arg::Value's type twice, once to find the function and once to check the argument, and get_by_index looked up T's type and then try_from_datum looked it up again. fn_call now computes each argument's type once and uses it for both. get_by_index converts a value of T's own type directly, because the try_ conversions would only look the type up again to accept it, and still uses them for any other type. On the same server, fn_call with one PostgresType argument went from 67 us to 45 us, and get_by_index of a PostgresType attribute from 48 us to 25 us.
The test only caught an unconditional LWLockRelease when the server was built with assertions, which CI's test servers are not. On other servers the release decremented InterruptHoldoffCount past 0, and rethrowing the ERROR reset it before the test could see it. The test now catches the ERROR, checks that InterruptHoldoffCount is still 0 and that this backend still holds the lock, and then rethrows it.
test_namespace_outlives_spi_context compared a name that the old namespace left in the deleted SPI context, so it passed whenever the server did not overwrite freed memory, as servers built without assertions do not. The test now fills the memory of the next SPI connection's context, which reuses the deleted context's first block, before it compares the name.
The earlier commit "Look up each fn_call argument type and get_by_index type once per call" made get_by_index read a value of T's own type with from_polymorphic_datum or from_datum_in_memory_context, skipping T's try_from_datum and try_from_datum_in_memory_context. A type can override those provided methods to reject a value it cannot read, so get_by_index accepted values that AnyElement::value still rejected. get_by_index now always reads through them. To keep one type lookup per read, it asks T for its type only for a row-type attribute, where it must know whether T reads records, and calls type_is_rowtype only for a variable-length attribute. On the same assert-enabled PG18 server, reading a PostgresType attribute stays at 25 us per read, and int4 and text reads take 0.40 us and 0.97 us, against 0.39 us and 0.92 us with the skip.
set_by_index called T::type_oid and then T::is_compatible_with, whose default calls type_oid again, so a PostgresType searched the extension's types twice per call. It now calls is_compatible_with first and calls type_oid only when that fails, to accept a composite value or to report the error. On an assert-enabled PG18 server, setting a PostgresType attribute went from 50 us to 26 us per call, and setting an int4 attribute stayed at 0.66 us.
…ntext

get_by_index's warning about values in CurrentMemoryContext named only a borrowed T, but a PgBox<T> it returns is a copy in that context too and has no lifetime to stop it outliving the context. The warning now covers both. The upgrade note said get_by_index and get_by_name remain safe because a PgHeapTuple owns a matching tuple and descriptor, which reads as a full soundness argument. It now says that argument does not cover such values and points to get_by_index's documentation.
The attname comment justified its 'static borrow by serde_json writing the key before serialize_field returns, but Tuple's Serialize impl accepts any Serializer, and one that keeps the key would read the name after it is freed. The comment now names that as a known, unfixed soundness gap. The heap_getattr comment said only PG15-17 write attcacheoff, but PG18's nocachegetattr writes it into the CompactAttribute array as well, so it now says PG15-18. PostgreSQL 19 beta 4's nocachegetattr no longer writes it.
fn_call's documentation says it returns UnsupportedArgumentModes, whose message names VARIADIC, for a VARIADIC function, and that a set-returning function raises PostgreSQL's "set-valued function called in context that cannot accept a set" ERROR. New tests call concat and generate_series to pin both.
The get_by_index documentation said a PgBox<T> holds the attribute's first size_of::<T>() bytes. Since PgBox's try_ conversions check the layout, get_by_index returns IncompatibleTypes unless the attribute's type is passed by reference with exactly that length and T needs no more than MAXALIGN alignment. The documentation now says so.
rust_regtypein read only the OID column when it looked up a pgrx_pg_sys type in pg_catalog, so after CREATE TYPE pg_catalog.list, PgBox<pg_sys::List>::type_oid() returned the shell's OID. Its documentation and the upgrade note say that a shell type raises an ERROR, as regtypein did before this branch. The pg_catalog lookup now reads the type's row and raises "is only a shell" for a shell, with the same single syscache search.
PostgreSQL 17 and later add and remove a range type's multirange type along with the range type in ALTER EXTENSION ADD and DROP, and earlier versions do not. A multirange type named like a Rust type therefore resolved on one set of versions and raised an ERROR on the other after such a change. The lookup now treats a multirange type as made for its range type, as PostgreSQL 17's isDependentType does, and skips it on every version. A multirange type that the extension's own script creates, which used to resolve on every version, now raises the "does not belong" ERROR as well.
The extension type lookup skips an extension table's row type both when it checks each same-named type and when it lists the extension's members. The tests with an extension table had only two same-named types, so they never reached the listing, and removing its skip passed every test. A new test adds an extension table among many same-named tables. On PostgreSQL 17 and later it fails with "more than one type named" when the listing does not skip the row type.
get_by_index now asks T for its type before converting only for a row-type attribute, so the test's NULL int4 attribute no longer reached that lookup. Moving the lookup above the NULL check left the test passing, although a NULL row-type attribute then raised T's type ERROR instead of returning None. The test now also reads a NULL attribute of a composite type, which fails in that case.
from_pg_copy's Safety section asked only for a live descriptor, but the copy is palloc'd in CurrentMemoryContext and freed on drop, so a value kept past that context, for example returned out of Spi::connect, reads and frees freed memory. The Safety section and SAFETY comment now state that requirement. for_composite_type_by_oid and Clone cannot meet it, because their lifetime does not bound the context. Their SAFETY comments now name that as a known gap, which for_composite_type_by_oid and for_composite_type now document.
The note told readers whose PgBox array elements now panic to read them as geo::Path or PgVarlena<T> instead. Neither implements UnboxDatum, which arrays, Vec arguments and get_by_index require, so that advice does not compile. The note now limits those types to single values and says to read such an array as AnyArray and each element with AnyElement::value, or to unnest it in SQL.
The upgrade note said that text array elements are checked for UTF-8, so that only an element that is not UTF-8 panics. That holds only in a SQL_ASCII database. In other encodings that are not UTF-8, such as LATIN1, the check accepts only ASCII, so any element with a non-ASCII byte panics even when its bytes are valid UTF-8. The note now says so, matching AnyElement::value's documentation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant