public inbox for [email protected]  
help / color / mirror / Atom feed
From: vaibhave postgres <[email protected]>
To: Pierre Forstmann <[email protected]>
Cc: Rahila Syed <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: BUG #19384: Server crash at textout
Date: Sun, 25 Jan 2026 16:17:34 +0530
Message-ID: <CAM_eQjwwMAN0usjGVEWZjPpSjSyGeAyctnfnd_P8D=whwf6-5g@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAH2L28tTiH2wNq4tDLcHr2B2NG=kMunJmNfYj5L5tMhQukVnJA@mail.gmail.com>
	<[email protected]>

Hey,

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

On Tue, Jan 20, 2026 at 8:33 PM Pierre Forstmann <[email protected]>
wrote:

> I can also reproduce with 18.1.
>
> But if type foo is used in a table, type foo cannot be modified:
> DROP TYPE IF EXISTS foo CASCADE;
> psql:bug4.sql:1: NOTICE:  drop cascades to column c of table bar
> DROP TYPE
> DROP TABLE if EXISTS bar CASCADE;
> DROP TABLE
> CREATE TYPE foo AS (a INT, b INT);
> CREATE TYPE
> CREATE TABLE bar(c foo);
> CREATE TABLE
> BEGIN;
> BEGIN
> INSERT into bar SELECT (i, power(2, 30))::foo FROM generate_series(1,10) i;
> INSERT 0 10
> ALTER TYPE foo ALTER ATTRIBUTE b TYPE TEXT;
> psql:bug4.sql:8: ERROR:  cannot alter type "foo" because column "bar.c"
> uses it
>
> Should PG allow to modify a type if this type is used to cast a SELECT
> list column in the same transaction ?
>
>
> Le 20/01/2026 à 12:15, Rahila Syed a écrit :
>
> Hi,
>
> On Tue, Jan 20, 2026 at 2:29 PM PG Bug reporting form <
> [email protected]> wrote:
>
>> The following bug has been logged on the website:
>>
>> Bug reference:      19384
>> Logged by:          Yuxiao Guo
>> Email address:      [email protected]
>> PostgreSQL version: 17.7
>> Operating system:   Ubuntu 20.04 x86-64, docker image postgres:17.7
>> Description:
>>
>> Hi, I found a crash in PostgreSQL. Here are the details:
>>
>> PoC:
>> DROP TYPE IF EXISTS foo CASCADE;
>> CREATE TYPE foo AS (a INT, b INT);
>> BEGIN;
>> DECLARE c CURSOR FOR SELECT (i, power(2, 30))::foo FROM
>> generate_series(1,10) i;
>> FETCH c;
>> ALTER TYPE foo ALTER ATTRIBUTE b TYPE TEXT;
>> FETCH c;
>>
>>
>> Stacktrace:
>> #0 0x7ae1c818a00b (gsignal+0xcb)
>> #1 0x7ae1c8169859 (abort+0x12b)
>> #2 0x542fa7 (_ZN11__sanitizer5AbortEv+0x47)
>> #3 0x5414d1 (_ZN11__sanitizer3DieEv+0xc1)
>> #4 0x528a14 (_ZN6__asan19ScopedInErrorReportD2Ev+0x1c4)
>> #5 0x52a5da (_ZN6__asan18ReportGenericErrorEmmmmbmjb+0x5ba)
>> #6 0x523ef6 (__asan_memcpy+0x1d6)
>> #7 0x17772d5 (textout+0x1b5)
>> #8 0x1835834 (OutputFunctionCall+0x174)
>> #9 0x167a568 (record_out+0x828)
>> #10 0x1835834 (OutputFunctionCall+0x174)
>> #11 0x595848 (printtup+0x958)
>> #12 0x1336280 (RunFromStore+0x1d0)
>> #13 0x1333ec0 (PortalRunSelect+0x150)
>> #14 0x133321d (PortalRun+0x51d)
>> #15 0x132f1de (exec_simple_query+0x146e)
>> #16 0x1328627 (PostgresMain+0x2c57)
>> #17 0x13192e4 (BackendMain+0xe4)
>> #18 0x10a26c3 (postmaster_child_launch+0x193)
>> #19 0x10adb91 (ServerLoop+0x4821)
>> #20 0x10a76ec (PostmasterMain+0x241c)
>> #21 0xd5c2b8 (main+0x458)
>> #22 0x7ae1c816b083 (__libc_start_main+0xf3)
>> #23 0x4a9c6e (_start+0x2e)
>>
>>
>>
> This problem is reproducible, also the issue seems to be linked to cursors
> since the type cast with only SELECT statements runs fine.
>
> CREATE TYPE foo AS (a INT, b INT);
> ALTER TYPE foo ALTER ATTRIBUTE b TYPE TEXT;
>
> postgres=# SELECT (i, power(2, 30))::foo FROM
> generate_series(1,10) i;
>        row
> -----------------
>  (1,1073741824)
>  (2,1073741824)
>  (3,1073741824)
>  (4,1073741824)
>  (5,1073741824)
>  (6,1073741824)
>  (7,1073741824)
>  (8,1073741824)
>  (9,1073741824)
>  (10,1073741824)
> (10 rows)
>
> Also, it happens only  if ALTER TYPE to TEXT is run after DECLARING the
> cursor.
>
> Another observation is that when I lower the exponent to 10, the crash no
> longer occurs,
> but the output is different.
>
> CREATE TYPE foo AS (a INT, b INT);
> BEGIN;
> DECLARE c CURSOR FOR SELECT (i, power(2, 10))::foo FROM
> generate_series(1,10) i;
> DECLARE CURSOR
> FETCH c;
>    row
> ----------
>  (1,1024)
> (1 row)
>
> ALTER TYPE foo ALTER ATTRIBUTE b TYPE TEXT;
> FETCH c;
>    row
> ----------
>  (2,\x10)
> (1 row)
>
> Thank you,
> Rahila Syed
>
>


Attachments:

  [application/octet-stream] 0001-Stable-tupDesc_identifier.patch (6.5K, 3-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



  [application/octet-stream] 0002-Fix-tests.patch (2.9K, 4-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



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], [email protected], [email protected]
  Subject: Re: BUG #19384: Server crash at textout
  In-Reply-To: <CAM_eQjwwMAN0usjGVEWZjPpSjSyGeAyctnfnd_P8D=whwf6-5g@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