public inbox for [email protected]  
help / color / mirror / Atom feed
From: surya poondla <[email protected]>
To: [email protected]
To: [email protected]
Subject: Re: BUG #19382: Server crash at __nss_database_lookup
Date: Thu, 26 Feb 2026 15:55:43 -0800
Message-ID: <CAOVWO5pbgCVx0zgTr1mxZug2hoGwxZOk+-Owvwg0jaQv9JE3Fw@mail.gmail.com> (raw)
In-Reply-To: <CAOVWO5py4zLPYsnGK1EEzHnC4feTSmbsEZn-BiDgY_=J1P6Wmw@mail.gmail.com>
References: <[email protected]>
	<CAOVWO5rVBKsjG4YwO_PJQu2OBGp8qUdF1jineYY6Lm3zc6-KWQ@mail.gmail.com>
	<CAOVWO5rbwKgHWLYJMvKuvGxW9eFSk7LADk=ZxDEvwA1uTefvAg@mail.gmail.com>
	<CAOVWO5qN8UXdwMnDa+a7aVq=irGSfm2aeYvEc-uV6j4QHZiyrA@mail.gmail.com>
	<CAOVWO5py4zLPYsnGK1EEzHnC4feTSmbsEZn-BiDgY_=J1P6Wmw@mail.gmail.com>

Hi All,

CFBot on the commitfest asked me to rebase my code, and during the rebase
(rebase to 17 Stable version) I realized my initial patch v1 might have
a memory management issue.
When conversion of type was needed, the old record object was never
properly freed, and the new record was being created in the wrong memory
context. v2 fixes this by passing the record variable directly to
convert_record_for_altered_type() and using assign_record_var() internally
to replace a record variable and this way correctly frees the old value,
transfers the new one into the right memory context.

Testing:
1) All 224 core regression tests pass
2) All 13 PL/pgSQL regression tests pass
3) All original test cases from the bug report produce correct results
(same results as my v1 patch) (adding them here again for quick reference)
and we no longer see the crash of the database.

psql (17.9)

Type "help" for help.


postgres=# DROP FUNCTION IF EXISTS bar();


DROP FUNCTION

postgres=#   DROP TYPE IF EXISTS foo 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=# SELECT bar();

       bar

------------------

 (123,1073741824)

(1 row)


postgres=# DROP FUNCTION IF EXISTS bar1();

DROP FUNCTION

postgres=#   DROP TYPE IF EXISTS foo1 CASCADE;

DROP TYPE

postgres=#   CREATE TYPE foo1 AS (a INT, b INT);

CREATE TYPE

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=# SELECT bar1();

 bar1

-------

 (1,2)

(1 row)


postgres=# DROP TYPE IF EXISTS foo2 CASCADE;

NOTICE:  drop cascades to function bar2()

DROP TYPE

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;

postgres$#       RETURN r;

postgres$#   END;

postgres$#   $$ LANGUAGE plpgsql;

CREATE FUNCTION

postgres=# SELECT bar2();

ERROR:  invalid input syntax for type integer: "hello"

CONTEXT:  PL/pgSQL function bar2() line 6 at RETURN

postgres=# quit


Regards,
Surya Poondla

>


Attachments:

  [application/octet-stream] 0002-Fix-bug-19382-server-crash-when-ALTER-TYPE-is-used-m.patch (6.8K, 3-0002-Fix-bug-19382-server-crash-when-ALTER-TYPE-is-used-m.patch)
  download | inline diff:
From 2cc161d3f8d1c081f0afd9a8438ab9358cbdbee3 Mon Sep 17 00:00:00 2001
From: spoondla <[email protected]>
Date: Fri, 23 Jan 2026 17:28:54 -0800
Subject: [PATCH v2] 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 | 165 ++++++++++++++++++++++++++++++++++-
 1 file changed, 164 insertions(+), 1 deletion(-)

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 6b077febdc8..f2ff1aa25ec 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -458,6 +458,8 @@ static char *format_preparedparamsdata(PLpgSQL_execstate *estate,
 static PLpgSQL_variable *make_callstmt_target(PLpgSQL_execstate *estate,
 											  PLpgSQL_expr *expr);
 
+static void convert_record_for_altered_type(PLpgSQL_execstate *estate,
+											PLpgSQL_rec *rec);
 
 /* ----------
  * plpgsql_exec_function	Called by the call handler for
@@ -3244,8 +3246,22 @@ exec_stmt_return(PLpgSQL_execstate *estate, PLpgSQL_stmt_return *stmt)
 				}
 				break;
 
-			case PLPGSQL_DTYPE_ROW:
 			case PLPGSQL_DTYPE_REC:
+				{
+					PLpgSQL_rec *rec = (PLpgSQL_rec *) retvar;
+
+					/*
+					 * 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 != NULL &&
+						!ExpandedRecordIsEmpty(rec->erh))
+						convert_record_for_altered_type(estate, rec);
+				}
+				/* FALL THROUGH */
+
+			case PLPGSQL_DTYPE_ROW:
 				{
 					/* exec_eval_datum can handle these cases */
 					int32		rettypmod;
@@ -3390,6 +3406,14 @@ 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)
+						convert_record_for_altered_type(estate, rec);
+
 					/* If rec is null, try to convert it to a row of nulls */
 					if (rec->erh == NULL)
 						instantiate_empty_record_variable(estate, rec);
@@ -8883,3 +8907,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.
+ *
+ * If conversion is needed, assigns the new record to rec via
+ * assign_record_var(), which transfers it to datum_context and frees
+ * the old record.
+ */
+static void
+convert_record_for_altered_type(PLpgSQL_execstate *estate,
+								PLpgSQL_rec *rec)
+{
+	ExpandedRecordHeader *erh = rec->erh;
+	Oid				rectypeid = rec->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;
+
+	/* 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;
+
+	/*
+	 * 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 without modifying rec */
+	if (!need_conversion)
+		return;
+
+	/* Build new expanded record with converted values */
+	new_erh = make_expanded_record_from_typeid(rectypeid, -1,
+											   get_eval_mcontext(estate));
+	expanded_record_set_fields(new_erh, new_values, new_nulls, true);
+
+	/*
+	 * Assign the new record to rec, transferring it to datum_context
+	 * and freeing the old record.
+	 */
+	assign_record_var(estate, rec, 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: <CAOVWO5pbgCVx0zgTr1mxZug2hoGwxZOk+-Owvwg0jaQv9JE3Fw@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