public inbox for [email protected]  
help / color / mirror / Atom feed
From: vaibhave postgres <[email protected]>
To: [email protected]
Subject: [BUG#19384] Fix Server crash at textout
Date: Mon, 26 Jan 2026 10:46:38 +0530
Message-ID: <CAM_eQjyEFTN_bmWWJLo+R_Q10DP2yQ1-QO96YHXGPzB9B2QMXA@mail.gmail.com> (raw)

Hey hackers,

I replaced the `tupDesc_identifier` with a 64-bit signature from its
structure which was a todo. Stored Datums can detect stale definitions.

Thanks.

[1] BUG #19384: Server crash at textout - Mailing list pgsql-bugs :
Postgres Professional
<https://postgrespro.com/list/id/[email protected];


Attachments:

  [application/octet-stream] 0002-Fix-tests.patch (2.9K, ../CAM_eQjyEFTN_bmWWJLo+R_Q10DP2yQ1-QO96YHXGPzB9B2QMXA@mail.gmail.com/3-0002-Fix-tests.patch)
  download | inline diff:
From 4a6521055859a5ddb60250be0d341ce30d9d2424 Mon Sep 17 00:00:00 2001
From: Vaibhave Sekar <[email protected]>
Date: Sun, 25 Jan 2026 10:34:55 +0000
Subject: [PATCH 2/2] Fix tests.

---
 src/test/regress/expected/portals.out  | 19 +++++++++++++++++++
 src/test/regress/expected/rowtypes.out |  7 ++-----
 src/test/regress/sql/portals.sql       | 14 ++++++++++++++
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/src/test/regress/expected/portals.out b/src/test/regress/expected/portals.out
index 06726ed4ab7..c4408c8d0bf 100644
--- a/src/test/regress/expected/portals.out
+++ b/src/test/regress/expected/portals.out
@@ -1561,3 +1561,22 @@ fetch all in held_portal;
 (1 row)
 
 reset default_toast_compression;
+-- Changing a composite type after a cursor has emitted rows should error
+DROP TYPE IF EXISTS portal_composite CASCADE;
+NOTICE:  type "portal_composite" does not exist, skipping
+CREATE TYPE portal_composite AS (a int, b int);
+BEGIN;
+DECLARE stale_portal CURSOR FOR
+  SELECT (i, power(2, 10))::portal_composite FROM generate_series(1, 2) i;
+FETCH FROM stale_portal;
+   row    
+----------
+ (1,1024)
+(1 row)
+
+ALTER TYPE portal_composite ALTER ATTRIBUTE b TYPE text;
+FETCH FROM stale_portal; -- should fail instead of crashing
+ERROR:  type portal_composite has changed
+DETAIL:  The composite value was created using a previous definition of type portal_composite.
+ROLLBACK;
+DROP TYPE portal_composite;
diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out
index 9168979a620..4b9104219b7 100644
--- a/src/test/regress/expected/rowtypes.out
+++ b/src/test/regress/expected/rowtypes.out
@@ -135,11 +135,8 @@ ERROR:  cannot alter table "fullname" because column "people.fn" uses its row ty
 -- but this should work:
 alter table fullname add column suffix text default null;
 select * from people;
-     fn      |     bd     
--------------+------------
- (Joe,Blow,) | 01-10-1984
-(1 row)
-
+ERROR:  type fullname has changed
+DETAIL:  The composite value was created using a previous definition of type fullname.
 -- test insertion/updating of subfields
 update people set fn.suffix = 'Jr';
 select * from people;
diff --git a/src/test/regress/sql/portals.sql b/src/test/regress/sql/portals.sql
index fc4cccb96c0..17110ee7c2e 100644
--- a/src/test/regress/sql/portals.sql
+++ b/src/test/regress/sql/portals.sql
@@ -605,3 +605,17 @@ drop table toasted_data;
 fetch all in held_portal;
 
 reset default_toast_compression;
+
+-- Changing a composite type after a cursor has emitted rows should error
+DROP TYPE IF EXISTS portal_composite CASCADE;
+CREATE TYPE portal_composite AS (a int, b int);
+
+BEGIN;
+DECLARE stale_portal CURSOR FOR
+  SELECT (i, power(2, 10))::portal_composite FROM generate_series(1, 2) i;
+FETCH FROM stale_portal;
+ALTER TYPE portal_composite ALTER ATTRIBUTE b TYPE text;
+FETCH FROM stale_portal; -- should fail instead of crashing
+ROLLBACK;
+
+DROP TYPE portal_composite;
-- 
2.43.0



  [application/octet-stream] 0001-Stable-tupDesc_identifier.patch (6.5K, ../CAM_eQjyEFTN_bmWWJLo+R_Q10DP2yQ1-QO96YHXGPzB9B2QMXA@mail.gmail.com/4-0001-Stable-tupDesc_identifier.patch)
  download | inline diff:
From 8c4d491c2cb02cdb51805f52eaa8c0f5fc37ac4c Mon Sep 17 00:00:00 2001
From: Vaibhave Sekar <[email protected]>
Date: Sun, 25 Jan 2026 10:34:46 +0000
Subject: [PATCH 1/2] Stable tupDesc_identifier.

---
 src/backend/utils/cache/typcache.c | 111 +++++++++++++++++++++++++++--
 1 file changed, 104 insertions(+), 7 deletions(-)

diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index aa4720cb598..6795ac294c1 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -57,6 +57,7 @@
 #include "catalog/pg_range.h"
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
+#include "common/hashfn.h"
 #include "common/int.h"
 #include "executor/executor.h"
 #include "lib/dshash.h"
@@ -287,12 +288,15 @@ static int32 RecordCacheArrayLen = 0;	/* allocated length of above array */
 static int32 NextRecordTypmod = 0;	/* number of entries used */
 
 /*
- * Process-wide counter for generating unique tupledesc identifiers.
+ * Process-wide counter for generating unique tupledesc identifiers for
+ * record (RECORDOID) tupdescs. Named composite types now derive their
+ * identifiers from a deterministic signature of the tupledesc contents.
  * Zero and one (INVALID_TUPLEDESC_IDENTIFIER) aren't allowed to be chosen
  * as identifiers, so we start the counter at INVALID_TUPLEDESC_IDENTIFIER.
  */
 static uint64 tupledesc_id_counter = INVALID_TUPLEDESC_IDENTIFIER;
 
+static uint64 compute_tupdesc_identifier(TupleDesc tupdesc);
 static void load_typcache_tupdesc(TypeCacheEntry *typentry);
 static void load_rangetype_info(TypeCacheEntry *typentry);
 static void load_multirangetype_info(TypeCacheEntry *typentry);
@@ -331,6 +335,75 @@ static dsa_pointer share_tupledesc(dsa_area *area, TupleDesc tupdesc,
 								   uint32 typmod);
 
 
+/*
+ * compute_tupdesc_identifier
+ *
+ * Build a stable, deterministic 64-bit signature for a tuple descriptor so
+ * that unchanged composite type definitions keep the same identifier across
+ * cache reloads and backends.  We hash only structural fields that affect the
+ * layout or logical contract of the rowtype, avoiding volatile fields such as
+ * attcacheoff.
+ */
+static uint64
+compute_tupdesc_identifier(TupleDesc tupdesc)
+{
+	uint64		hash;
+	int		natts = tupdesc->natts;
+
+	hash = hash_bytes_extended((const unsigned char *) &tupdesc->tdtypeid,
+								 sizeof(Oid), 0);
+	hash = hash_bytes_extended((const unsigned char *) &tupdesc->tdtypmod,
+								 sizeof(int32), hash);
+	hash = hash_bytes_extended((const unsigned char *) &natts,
+								 sizeof(int), hash);
+
+	for (int i = 0; i < natts; i++)
+	{
+		Form_pg_attribute att = TupleDescAttr(tupdesc, i);
+
+		hash = hash_bytes_extended((const unsigned char *) att->attname.data,
+										 NAMEDATALEN, hash);
+		hash = hash_bytes_extended((const unsigned char *) &att->attisdropped,
+										 sizeof(bool), hash);
+		hash = hash_bytes_extended((const unsigned char *) &att->attnotnull,
+										 sizeof(bool), hash);
+		hash = hash_bytes_extended((const unsigned char *) &att->attlen,
+										 sizeof(int16), hash);
+		hash = hash_bytes_extended((const unsigned char *) &att->attbyval,
+										 sizeof(bool), hash);
+		hash = hash_bytes_extended((const unsigned char *) &att->attalign,
+										 sizeof(char), hash);
+		hash = hash_bytes_extended((const unsigned char *) &att->attstorage,
+										 sizeof(char), hash);
+		hash = hash_bytes_extended((const unsigned char *) &att->attcompression,
+										 sizeof(char), hash);
+		hash = hash_bytes_extended((const unsigned char *) &att->atttypid,
+										 sizeof(Oid), hash);
+		hash = hash_bytes_extended((const unsigned char *) &att->atttypmod,
+										 sizeof(int32), hash);
+		hash = hash_bytes_extended((const unsigned char *) &att->attcollation,
+										 sizeof(Oid), hash);
+		hash = hash_bytes_extended((const unsigned char *) &att->attndims,
+										 sizeof(int32), hash);
+		hash = hash_bytes_extended((const unsigned char *) &att->attidentity,
+										 sizeof(char), hash);
+		hash = hash_bytes_extended((const unsigned char *) &att->attgenerated,
+										 sizeof(char), hash);
+		hash = hash_bytes_extended((const unsigned char *) &att->atthasmissing,
+										 sizeof(bool), hash);
+	}
+
+	/* Avoid reserved identifiers (0 and 1). */
+	if (hash <= INVALID_TUPLEDESC_IDENTIFIER)
+		hash = hash_bytes_extended((const unsigned char *) &natts,
+										 sizeof(int), UINT64CONST(0xA5A5A5A5A5A5A5A5));
+	if (hash <= INVALID_TUPLEDESC_IDENTIFIER)
+		hash = INVALID_TUPLEDESC_IDENTIFIER + 1;
+
+	return hash;
+}
+
+
 /*
  * lookup_type_cache
  *
@@ -898,11 +971,14 @@ load_typcache_tupdesc(TypeCacheEntry *typentry)
 	Assert(typentry->tupDesc->tdrefcount > 0);
 	typentry->tupDesc->tdrefcount++;
 
+	typentry->tupDesc_identifier = compute_tupdesc_identifier(typentry->tupDesc);
+
 	/*
-	 * In future, we could take some pains to not change tupDesc_identifier if
-	 * the tupdesc didn't really change; but for now it's not worth it.
+	 * Stash a version marker in tdtypmod so composite Datums can detect stale
+	 * definitions later.  Keep it non-negative so callers can reliably check
+	 * for it.
 	 */
-	typentry->tupDesc_identifier = ++tupledesc_id_counter;
+	typentry->tupDesc->tdtypmod = (int32) (typentry->tupDesc_identifier & 0x7FFFFFFF);
 
 	relation_close(rel, AccessShareLock);
 }
@@ -1748,9 +1824,30 @@ lookup_rowtype_tupdesc_internal(Oid type_id, int32 typmod, bool noError)
 		typentry = lookup_type_cache(type_id, TYPECACHE_TUPDESC);
 		if (typentry->tupDesc == NULL && !noError)
 			ereport(ERROR,
-					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-					 errmsg("type %s is not composite",
-							format_type_be(type_id))));
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("type %s is not composite",
+					 format_type_be(type_id))));
+		else if (typentry->tupDesc == NULL)
+			return NULL;
+
+		/*
+		 * If the caller supplied a non-negative typmod, treat it as the version
+		 * of the composite type that was current when the Datum was formed.  If
+		 * it doesn't match the current definition, fail fast instead of
+		 * interpreting the tuple using the wrong layout.
+		 */
+		if (typmod >= 0)
+		{
+			int32		expected_typmod = typentry->tupDesc->tdtypmod;
+
+			if (expected_typmod >= 0 && typmod != expected_typmod)
+				ereport(ERROR,
+					(errcode(ERRCODE_DATATYPE_MISMATCH),
+					 errmsg("type %s has changed",
+						 format_type_be(type_id)),
+					 errdetail("The composite value was created using a previous definition of type %s.",
+						 format_type_be(type_id))));
+		}
 		return typentry->tupDesc;
 	}
 	else
-- 
2.43.0



view thread (2+ messages)

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]
  Subject: Re: [BUG#19384] Fix Server crash at textout
  In-Reply-To: <CAM_eQjyEFTN_bmWWJLo+R_Q10DP2yQ1-QO96YHXGPzB9B2QMXA@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