Subject: CheckAttributeType() forgot to recurse into multiranges

From: Heikki Linnakangas <[email-masked]>
Date: Wed, 22 Apr 2026 23:56:12 +0300

Happened to spot this little bug:

create type two_ints as (a int, b int);
create type two_ints_range as range (subtype = two_ints);

-- CheckAttributeType() forbids this:
alter type two_ints add attribute c two_ints_range;
ERROR:  composite type two_ints cannot be made a member of itself

-- But the same with a multirange is allowed:
alter type two_ints add attribute c two_ints_multirange;
ALTER TYPE

That looks like a straightforward oversight in CheckAttributeType(). 
When multiranges were introduced, it didn't get the memo.

Fix attached. Assuming no objections, I'll commit and backpatch that.

While working on the fix, I noticed that in case of dropped columns, 
CheckAttributeType() is called with InvalidOid. It tolerates that, but 
it seems accidental and it performs a bunch of futile syscache lookups 
with InvalidOid, so it would be better to not do that. The second patch 
fixes that.

- Heikki
From 50eecdd6477279785616bb4945b31b243265904e Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email-masked]>
Date: Wed, 22 Apr 2026 22:10:06 +0300
Subject: [PATCH 1/2] Don't allow composite type to be member of itself via
 multirange

CheckAttributeType() checks that a composite type is not made a member
of itself with ALTER TABLE ADD COLUMN or ALTER TYPE ADD ATTRIBUTE,
even indirectly via a domain, array, another composite type or a range
type. But it missed checking for multiranges. That was a simple
oversight when multiranges were added.

Discussion: xxx
Backpatch-through: 14
---
 src/backend/catalog/heap.c                | 10 ++++++++++
 src/test/regress/expected/alter_table.out |  9 +++++++++
 src/test/regress/expected/rangetypes.out  |  4 +++-
 src/test/regress/sql/alter_table.sql      |  3 +++
 src/test/regress/sql/rangetypes.sql       |  3 ++-
 5 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index ae6b7cda3dd..4a71f6a33b4 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -654,6 +654,16 @@ CheckAttributeType(const char *attname,
 						   containing_rowtypes,
 						   flags);
 	}
+	else if (att_typtype == TYPTYPE_MULTIRANGE)
+	{
+		/*
+		 * If it's a multirange, recurse to check its plain range type.
+		 */
+		CheckAttributeType(attname, get_multirange_range(atttypid),
+						   get_range_collation(atttypid),
+						   containing_rowtypes,
+						   flags);
+	}
 	else if (OidIsValid((att_typelem = get_element_type(atttypid))))
 	{
 		/*
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index dad9d36937e..fc27e22023b 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2335,6 +2335,15 @@ ERROR:  composite type recur1 cannot be made a member of itself
 create domain array_of_recur1 as recur1[];
 alter table recur1 add column f2 array_of_recur1; -- fails
 ERROR:  composite type recur1 cannot be made a member of itself
+create type range_of_recur1 as range (subtype=recur1, multirange_type_name='multirange_of_recur1');
+NOTICE:  function "range_of_recur1" will be effectively temporary
+DETAIL:  It depends on temporary type recur1.
+NOTICE:  function "range_of_recur1" will be effectively temporary
+DETAIL:  It depends on temporary type recur1.
+alter table recur1 add column f2 range_of_recur1; -- fails
+ERROR:  composite type recur1 cannot be made a member of itself
+alter table recur1 add column f2 multirange_of_recur1; -- fails
+ERROR:  composite type recur1 cannot be made a member of itself
 create temp table recur2 (f1 int, f2 recur1);
 alter table recur1 add column f2 recur2; -- fails
 ERROR:  composite type recur1 cannot be made a member of itself
diff --git a/src/test/regress/expected/rangetypes.out b/src/test/regress/expected/rangetypes.out
index e062a4e5c2c..b2a753fd179 100644
--- a/src/test/regress/expected/rangetypes.out
+++ b/src/test/regress/expected/rangetypes.out
@@ -1810,9 +1810,11 @@ select *, row_to_json(upper(t)) as u from
  ["(5,6)","(7,8)") | {"a":7,"b":8}
 (2 rows)
 
--- this must be rejected to avoid self-inclusion issues:
+-- these must be rejected to avoid self-inclusion issues:
 alter type two_ints add attribute c two_ints_range;
 ERROR:  composite type two_ints cannot be made a member of itself
+alter type two_ints add attribute c two_ints_multirange;
+ERROR:  composite type two_ints cannot be made a member of itself
 drop type two_ints cascade;
 NOTICE:  drop cascades to type two_ints_range
 --
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index f5f13bbd3e7..6aaa33b6e34 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1535,6 +1535,9 @@ alter table recur1 add column f2 recur1; -- fails
 alter table recur1 add column f2 recur1[]; -- fails
 create domain array_of_recur1 as recur1[];
 alter table recur1 add column f2 array_of_recur1; -- fails
+create type range_of_recur1 as range (subtype=recur1, multirange_type_name='multirange_of_recur1');
+alter table recur1 add column f2 range_of_recur1; -- fails
+alter table recur1 add column f2 multirange_of_recur1; -- fails
 create temp table recur2 (f1 int, f2 recur1);
 alter table recur1 add column f2 recur2; -- fails
 alter table recur1 add column f2 int;
diff --git a/src/test/regress/sql/rangetypes.sql b/src/test/regress/sql/rangetypes.sql
index 5c4b0337b7a..f0354736fe5 100644
--- a/src/test/regress/sql/rangetypes.sql
+++ b/src/test/regress/sql/rangetypes.sql
@@ -576,8 +576,9 @@ select *, row_to_json(upper(t)) as u from
   (values (two_ints_range(row(1,2), row(3,4))),
           (two_ints_range(row(5,6), row(7,8)))) v(t);
 
--- this must be rejected to avoid self-inclusion issues:
+-- these must be rejected to avoid self-inclusion issues:
 alter type two_ints add attribute c two_ints_range;
+alter type two_ints add attribute c two_ints_multirange;
 
 drop type two_ints cascade;
 
-- 
2.47.3

From 17dc5a3887965e0b00704d15d4325474db47ad8d Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email-masked]>
Date: Wed, 22 Apr 2026 23:53:24 +0300
Subject: [PATCH 2/2] Don't call CheckAttributeType() with InvalidOid on
 dropped cols

If CheckAttributeType() is called with InvalidOid, it performs a bunch
of pointless, futile syscache lookups with InvalidOid, but ultimately
tolerates it and has no effect. We were calling it with InvalidOid on
dropped columns, but it seems accidental that it works, so let's stop
doing it.

Discussion: xxx
Backpatch-through: 14
---
 src/backend/catalog/heap.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 4a71f6a33b4..b3bc454bd48 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -504,11 +504,15 @@ CheckAttributeNamesTypes(TupleDesc tupdesc, char relkind,
 	 */
 	for (i = 0; i < natts; i++)
 	{
-		CheckAttributeType(NameStr(TupleDescAttr(tupdesc, i)->attname),
-						   TupleDescAttr(tupdesc, i)->atttypid,
-						   TupleDescAttr(tupdesc, i)->attcollation,
+		Form_pg_attribute attr = TupleDescAttr(tupdesc, i);
+
+		if (attr->attisdropped)
+			continue;
+		CheckAttributeType(NameStr(attr->attname),
+						   attr->atttypid,
+						   attr->attcollation,
 						   NIL, /* assume we're creating a new rowtype */
-						   flags | (TupleDescAttr(tupdesc, i)->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL ? CHKATYPE_IS_VIRTUAL : 0));
+						   flags | (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL ? CHKATYPE_IS_VIRTUAL : 0));
 	}
 }
 
-- 
2.47.3

---

From: Chao Li <[email-masked]>
Date: Thu, 23 Apr 2026 11:24:26 +0800

> On Apr 23, 2026, at 04:56, Heikki Linnakangas <[email-masked]> wrote:
> 
> That looks like a straightforward oversight in CheckAttributeType(). When multiranges were introduced, it didn't get the memo.
> 
> While working on the fix, I noticed that in case of dropped columns, CheckAttributeType() is called with InvalidOid. It tolerates that, but it seems accidental and it performs a bunch of futile syscache lookups with InvalidOid, so it would be better to not do that. The second patch fixes that.
> 
> - Heikki
> <0001-Don-t-allow-composite-type-to-be-member-of-itself-vi.patch><0002-Don-t-call-CheckAttributeType-with-InvalidOid-on-dro.patch>

I traced this patch set, 0002 looks good, but I have a suspicion about 0001.

```
```

Looking at get_range_collation(), it only searches for RANGETYPE, so get_range_collation(atttypid) here will always return InvalidOid. This does not seem to cause a problem, because CheckAttributeType() will recurse into the TYPTYPE_RANGE path, and the collation will be evaluated there.

But to make the logic clearer, I think we could just pass InvalidOid as the collation OID in the TYPTYPE_MULTIRANGE case. If we really want to pass the actual collation OID here, I think it would need to be done more like this:

```
	else if (att_typtype == TYPTYPE_MULTIRANGE)
	{
		Oid multirange_range_typid = get_multirange_range(atttypid);
		Oid collation = get_range_collation(multirange_range_typid);
		/*
		 * If it's a multirange, recurse to check its plain range type.
		 */
		CheckAttributeType(attname, multirange_range_typid,
						   collation,
						   containing_rowtypes,
						   flags);
	}
```

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

---

From: Michael Paquier <[email-masked]>
Date: Thu, 23 Apr 2026 16:22:15 +0900

On Wed, Apr 22, 2026 at 11:56:12PM +0300, Heikki Linnakangas wrote:
> That looks like a straightforward oversight in CheckAttributeType(). When
> multiranges were introduced, it didn't get the memo.

Nice catch.

If you want to create a parallel with multirangetypes.sql, this
choking case may be better if placed there rather than rangetypes.sql,
as it is a multi case.  Not a big deal, still.

> CheckAttributeType() is called with InvalidOid. It tolerates that, but it
> seems accidental and it performs a bunch of futile syscache lookups with
> InvalidOid, so it would be better to not do that. The second patch fixes
> that.

This one seems harmless as far as I can see, but we should be careful
to bypass any attisdropped while scanning a set of attributes, so a
backpatch is in order, indeed.  LGTM.
--
Michael

---

From: Heikki Linnakangas <[email-masked]>
Date: Thu, 23 Apr 2026 21:49:04 +0300

> 		CheckAttributeType(attname, multirange_range_typid,
Oh, good catch, this is subtle. I'll change it to InvalidOid. As long as 
range types are not collatable, that'll do the right thing. If range 
types were collatable, I'm not sure what the right thing would be. It 
depends on what exactly the collation of a range type would mean.

So, pushed with InvalidOid. Thanks for the review!

- Heikki