public inbox for [email protected]
help / color / mirror / Atom feedFrom: surya poondla <[email protected]>
To: [email protected]
To: [email protected]
Subject: Re: BUG #19382: Server crash at __nss_database_lookup
Date: Fri, 23 Jan 2026 17:55:25 -0800
Message-ID: <CAOVWO5rbwKgHWLYJMvKuvGxW9eFSk7LADk=ZxDEvwA1uTefvAg@mail.gmail.com> (raw)
In-Reply-To: <CAOVWO5rVBKsjG4YwO_PJQu2OBGp8qUdF1jineYY6Lm3zc6-KWQ@mail.gmail.com>
References: <[email protected]>
<CAOVWO5rVBKsjG4YwO_PJQu2OBGp8qUdF1jineYY6Lm3zc6-KWQ@mail.gmail.com>
Hi All,
I am working on a patch for this.
>
I built a patch on 17.6 postgres version.
The overall issue is:
When ALTER TYPE modifies column types, the type cache updates its
tupDesc_identifier.
However, existing ExpandedRecordHeader instances still reference the old
tupdesc.
When the record is returned and flattened, the output functions expect data
to match the new type definition but receive data in the old format,
causing type confusion (e.g., interpreting an integer as a text pointer).
This was causing a segfault and crashing the server.
In my solution (attached patch), I added convert_record_for_altered_type()
function which detects type changes by comparing er_tupdesc_id against the
current tupDesc_identifier.
When a mismatch is found, it tries to convert each field value to match the
new type definition, if this fails we error out.
convert_record_for_altered_type() function is called in exec_stmt_return()
and exec_stmt_return_next() before returning records.
I tested my patch and see the below output
postgres=# DROP FUNCTION IF EXISTS bar();
DROP FUNCTION
postgres=# DROP TYPE IF EXISTS foo CASCADE;
DROP TYPE
postgres=# DROP FUNCTION IF EXISTS bar1();
DROP FUNCTION
postgres=# DROP TYPE IF EXISTS foo1 CASCADE;
DROP TYPE
postgres=# CREATE TYPE foo AS (a INT, b INT);
CREATE TYPE
postgres=# CREATE FUNCTION bar() RETURNS RECORD AS $$
postgres$# DECLARE
postgres$# r foo := ROW(123, power(2, 30));
postgres$# BEGIN
postgres$# ALTER TYPE foo ALTER ATTRIBUTE b TYPE TEXT;
postgres$# RETURN r;
postgres$# END;
postgres$# $$ LANGUAGE plpgsql;
CREATE FUNCTION
postgres=#
postgres=# SELECT bar();
bar
------------------
(123,1073741824)
(1 row)
postgres=# CREATE TYPE foo1 AS (a INT, b INT);
CREATE TYPE
postgres=#
postgres=# CREATE FUNCTION bar1(OUT r1 foo1) AS $$
postgres$# BEGIN
postgres$# r1 := ROW(1, 2);
postgres$# ALTER TYPE foo1 ALTER ATTRIBUTE b TYPE TEXT;
postgres$# RETURN;
postgres$# END;
postgres$# $$ LANGUAGE plpgsql;
CREATE FUNCTION
postgres=#
postgres=# SELECT bar1();
bar1
-------
(1,2)
(1 row)
postgres=# CREATE TYPE foo2 AS (a INT, b TEXT);
CREATE TYPE
postgres=# CREATE FUNCTION bar2() RETURNS foo2 AS $$
postgres$# DECLARE
postgres$# r foo2 := ROW(1, 'hello');
postgres$# BEGIN
postgres$# ALTER TYPE foo2 ALTER ATTRIBUTE b TYPE INT; -- TEXT → INT
postgres$# RETURN r; -- Should get clean error (not crash)
postgres$# END;
postgres$# $$ LANGUAGE plpgsql;
CREATE FUNCTION
postgres=#
postgres=# SELECT bar2();
ERROR: invalid input syntax for type integer: "hello"
CONTEXT: PL/pgSQL function bar2() line 6 at RETURN
postgres=#
Attachments:
[application/octet-stream] 0001-Fix-bug-19382-server-crash-when-ALTER-TYPE-is-used-m.patch (6.9K, 3-0001-Fix-bug-19382-server-crash-when-ALTER-TYPE-is-used-m.patch)
download | inline diff:
From 2ed5b4181a026729169e92e8bcd175f62b1daab1 Mon Sep 17 00:00:00 2001
From: spoondla <[email protected]>
Date: Fri, 23 Jan 2026 17:28:54 -0800
Subject: [PATCH] Fix (bug #19382) server crash when ALTER TYPE is used
mid-transaction in PL/pgSQL
When ALTER TYPE changes a composite type's column types within a
transaction, PL/pgSQL record variables that were populated before
the ALTER still hold data in the old format. Returning such records
causes a crash because the output functions expect data matching the
new type definition, not the old one.
The crash manifested as a segmentation fault in record_out() when it
attempted to interpret integer data as a text pointer, due to the
mismatch between the stored data and the current type definition.
---
src/pl/plpgsql/src/pl_exec.c | 161 +++++++++++++++++++++++++++++++++++
1 file changed, 161 insertions(+)
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index e7b0f2544b4..53add41882a 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -458,6 +458,9 @@ static char *format_preparedparamsdata(PLpgSQL_execstate *estate,
static PLpgSQL_variable *make_callstmt_target(PLpgSQL_execstate *estate,
PLpgSQL_expr *expr);
+static ExpandedRecordHeader *convert_record_for_altered_type(PLpgSQL_execstate *estate,
+ ExpandedRecordHeader *erh,
+ Oid rectypeid);
/* ----------
* plpgsql_exec_function Called by the call handler for
@@ -3251,6 +3254,15 @@ exec_stmt_return(PLpgSQL_execstate *estate, PLpgSQL_stmt_return *stmt)
/* If record is empty, we return NULL not a row of nulls */
if (rec->erh && !ExpandedRecordIsEmpty(rec->erh))
{
+ /*
+ * Check if the record's composite type was altered since
+ * the record was populated. If so, convert the data to
+ * prevent crashes when outputting the record.
+ */
+ if (rec->rectypeid != RECORDOID)
+ rec->erh = convert_record_for_altered_type(estate,
+ rec->erh,
+ rec->rectypeid);
estate->retval = ExpandedRecordGetDatum(rec->erh);
estate->retisnull = false;
estate->rettype = rec->rectypeid;
@@ -3404,6 +3416,16 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
TupleDesc rec_tupdesc;
TupleConversionMap *tupmap;
+ /*
+ * Check if the record's composite type was altered since
+ * the record was populated. If so, convert the data to
+ * prevent crashes when storing to the tuplestore.
+ */
+ if (rec->rectypeid != RECORDOID && rec->erh != NULL)
+ rec->erh = convert_record_for_altered_type(estate,
+ rec->erh,
+ rec->rectypeid);
+
/* If rec is null, try to convert it to a row of nulls */
if (rec->erh == NULL)
instantiate_empty_record_variable(estate, rec);
@@ -8897,3 +8919,142 @@ format_preparedparamsdata(PLpgSQL_execstate *estate,
return paramstr.data;
}
+
+/*
+ * convert_record_for_altered_type
+ *
+ * Check if a record's composite type has been altered since the record
+ * was populated, and if so, convert the record data to match the new
+ * type definition. This prevents crashes that can occur when the stored
+ * data doesn't match the current type definition.
+ *
+ * Returns a (possibly new) ExpandedRecordHeader with data matching the
+ * current type definition.
+ */
+static ExpandedRecordHeader *
+convert_record_for_altered_type(PLpgSQL_execstate *estate,
+ ExpandedRecordHeader *erh,
+ Oid rectypeid)
+{
+ TupleDesc old_tupdesc;
+ TupleDesc new_tupdesc;
+ TypeCacheEntry *typentry;
+ uint64 current_tupdesc_id;
+ ExpandedRecordHeader *new_erh;
+ Datum *old_values;
+ bool *old_nulls;
+ Datum *new_values;
+ bool *new_nulls;
+ int natts;
+ int i;
+ MemoryContext oldcxt;
+ bool need_conversion = false;
+
+ /* Nothing to do for anonymous RECORD type */
+ if (rectypeid == RECORDOID)
+ return erh;
+
+ /* Get current type definition from typcache */
+ typentry = lookup_type_cache(rectypeid,
+ TYPECACHE_TUPDESC |
+ TYPECACHE_DOMAIN_BASE_INFO);
+ if (typentry->typtype == TYPTYPE_DOMAIN)
+ typentry = lookup_type_cache(typentry->domainBaseType,
+ TYPECACHE_TUPDESC);
+
+ current_tupdesc_id = typentry->tupDesc_identifier;
+
+ /* If type hasn't changed, nothing to do (fast path) */
+ if (erh->er_tupdesc_id == current_tupdesc_id)
+ return erh;
+
+ /*
+ * Type version has changed. Need to check if field types actually differ
+ * and convert if necessary.
+ */
+ old_tupdesc = erh->er_tupdesc;
+ new_tupdesc = typentry->tupDesc;
+
+ /* Sanity check: must have same number of attributes */
+ if (old_tupdesc->natts != new_tupdesc->natts)
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("record type \"%s\" structure has changed",
+ format_type_be(rectypeid)),
+ errdetail("Number of columns changed from %d to %d.",
+ old_tupdesc->natts, new_tupdesc->natts)));
+
+ natts = old_tupdesc->natts;
+
+ /* Deconstruct the old record to access field values */
+ deconstruct_expanded_record(erh);
+ old_values = erh->dvalues;
+ old_nulls = erh->dnulls;
+
+ /* Allocate arrays for new values */
+ oldcxt = MemoryContextSwitchTo(get_eval_mcontext(estate));
+ new_values = (Datum *) palloc(natts * sizeof(Datum));
+ new_nulls = (bool *) palloc(natts * sizeof(bool));
+ MemoryContextSwitchTo(oldcxt);
+
+ /* Convert each field */
+ for (i = 0; i < natts; i++)
+ {
+ Form_pg_attribute old_att = TupleDescAttr(old_tupdesc, i);
+ Form_pg_attribute new_att = TupleDescAttr(new_tupdesc, i);
+
+ /* Skip dropped columns */
+ if (old_att->attisdropped || new_att->attisdropped)
+ {
+ new_values[i] = (Datum) 0;
+ new_nulls[i] = true;
+ continue;
+ }
+
+ /* If null, stays null */
+ if (old_nulls[i])
+ {
+ new_values[i] = (Datum) 0;
+ new_nulls[i] = true;
+ continue;
+ }
+
+ /* If same type, no conversion needed */
+ if (old_att->atttypid == new_att->atttypid &&
+ (old_att->atttypmod == new_att->atttypmod ||
+ new_att->atttypmod == -1))
+ {
+ new_values[i] = old_values[i];
+ new_nulls[i] = false;
+ continue;
+ }
+
+ /* Different type: convert using exec_cast_value */
+ need_conversion = true;
+ new_nulls[i] = false;
+ new_values[i] = exec_cast_value(estate,
+ old_values[i],
+ &new_nulls[i],
+ old_att->atttypid,
+ old_att->atttypmod,
+ new_att->atttypid,
+ new_att->atttypmod);
+ }
+
+ /* If no actual conversion was needed, return original */
+ if (!need_conversion)
+ {
+ pfree(new_values);
+ pfree(new_nulls);
+ return erh;
+ }
+
+ /* Build new expanded record with converted values */
+ new_erh = make_expanded_record_from_typeid(rectypeid, -1,
+ estate->tuple_store_cxt ?
+ estate->tuple_store_cxt :
+ CurrentMemoryContext);
+ expanded_record_set_fields(new_erh, new_values, new_nulls, true);
+
+ return new_erh;
+}
--
2.39.5 (Apple Git-154)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: BUG #19382: Server crash at __nss_database_lookup
In-Reply-To: <CAOVWO5rbwKgHWLYJMvKuvGxW9eFSk7LADk=ZxDEvwA1uTefvAg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox