pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedDetaching a child table makes an expression using it unrestorable
7+ messages / 5 participants
[nested] [flat]
* Detaching a child table makes an expression using it unrestorable
@ 2026-09-10 14:12 Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 1 reply; 7+ messages in thread
From: Heikki Linnakangas @ 2026-09-10 14:12 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
I bumped into a sequence of commands that breaks pg_dump & restore:
------------
create table at_tab (a int, b int);
create table at_tab_child (a int, b int);
alter table at_tab_child inherit at_tab;
-- You can use the child's rowtype in the DEFAULT without a cast.
CREATE FUNCTION func_with_default(
arg at_tab DEFAULT ('(1, 2)'::at_tab_child)
) RETURNS integer LANGUAGE plpgsql as $$
begin
return arg.b;
end;
$$;
-- We allow detaching the child from the parent, despite the DEFAULT
-- expression. That is a not good, because if you try to recreate the
-- function, it's not accepted. I.e. pg_dump & restore is broken
alter table at_tab_child no inherit at_tab;
------------
The function still works after that. But if you run pg_dump (or do \ef
or something), the CREATE FUNCTION is deparsed as above, and when you
try to restore it you get an error:
ERROR: argument of DEFAULT must be type at_tab, not type at_tab_child
LINE 1: ...CTION public.func_with_default(arg at_tab DEFAULT '(1,2)'::a...
You get the same effect with ATTACH/DETACH PARTITION instead of
INHERIT/NO INHERIT.
- Heikki
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Detaching a child table makes an expression using it unrestorable
@ 2026-09-15 01:53 Jinqing Kuang <kuangjinqingcn@gmail.com>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 1 reply; 7+ messages in thread
From: Jinqing Kuang @ 2026-09-15 01:53 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Sep 10, 2026, at 22:12, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>
> I bumped into a sequence of commands that breaks pg_dump & restore:
>
> ------------
> create table at_tab (a int, b int);
> create table at_tab_child (a int, b int);
> alter table at_tab_child inherit at_tab;
>
> -- You can use the child's rowtype in the DEFAULT without a cast.
> CREATE FUNCTION func_with_default(
> arg at_tab DEFAULT ('(1, 2)'::at_tab_child)
> ) RETURNS integer LANGUAGE plpgsql as $$
> begin
> return arg.b;
> end;
> $$;
>
> -- We allow detaching the child from the parent, despite the DEFAULT
> -- expression. That is a not good, because if you try to recreate the
> -- function, it's not accepted. I.e. pg_dump & restore is broken
> alter table at_tab_child no inherit at_tab;
> ------------
>
> The function still works after that. But if you run pg_dump (or do \ef or something), the CREATE FUNCTION is deparsed as above, and when you try to restore it you get an error:
>
> ERROR: argument of DEFAULT must be type at_tab, not type at_tab_child
> LINE 1: ...CTION public.func_with_default(arg at_tab DEFAULT '(1,2)'::a...
>
> You get the same effect with ATTACH/DETACH PARTITION instead of INHERIT/NO INHERIT.
>
> - Heikki
>
Hi Heikki,
I reproduced your example on 20-devel at 92aaf50e230. The function
still works after NO INHERIT, but restoring the dump fails.
I’d first try to preserve the current coercion rules:
1. Reject NO INHERIT/DETACH when it removes the last inheritance path
needed by a stored ConvertRowtypeExpr, including indirect paths.
2. In pg_dump, make objects using these conversions depend on the
required TABLE ATTACH entries, including intermediate partitions.
This needs a way to find affected expressions in existing databases;
recording dependencies only when creating new objects would miss them.
It would also leave already-detached conversions needing repair.
Alternatively, we could allow explicit casts between named composite
types without inheritance if the source has matching names, types and
typmods for all target fields, and deparse ConvertRowtypeExpr with that
cast. Implicit coercions would still require inheritance. This would
keep DETACH working, but would broaden the explicit-cast rules. Do you
think that is preferable?
The partition variant also fails to restore before any DETACH:
CREATE TABLE p(a int, b int) PARTITION BY RANGE(a);
CREATE TABLE c(a int, b int);
ALTER TABLE p ATTACH PARTITION c FOR VALUES FROM (0) TO (10);
CREATE FUNCTION f(arg p DEFAULT '(1,2)'::c) RETURNS integer
LANGUAGE plpgsql AS $$BEGIN RETURN arg.b; END$$;
-- pg_dump -s puts CREATE FUNCTION before ATTACH; restore fails.
-- Moving ATTACH before CREATE FUNCTION makes restore succeed.
Regards,
Jinqing
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Detaching a child table makes an expression using it unrestorable
@ 2026-09-15 04:26 Andrey Rachitskiy <pl0h0yp1@gmail.com>
parent: Jinqing Kuang <kuangjinqingcn@gmail.com>
0 siblings, 1 reply; 7+ messages in thread
From: Andrey Rachitskiy @ 2026-09-15 04:26 UTC (permalink / raw)
To: Jinqing Kuang <kuangjinqingcn@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
вт, 15 сент. 2026 г. в 06:54, Jinqing Kuang <kuangjinqingcn@gmail.com>:
> On Sep 10, 2026, at 22:12, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> >
> > I bumped into a sequence of commands that breaks pg_dump & restore:
> >
> > ------------
> > create table at_tab (a int, b int);
> > create table at_tab_child (a int, b int);
> > alter table at_tab_child inherit at_tab;
> >
> > -- You can use the child's rowtype in the DEFAULT without a cast.
> > CREATE FUNCTION func_with_default(
> > arg at_tab DEFAULT ('(1, 2)'::at_tab_child)
> > ) RETURNS integer LANGUAGE plpgsql as $$
> > begin
> > return arg.b;
> > end;
> > $$;
> >
> > -- We allow detaching the child from the parent, despite the DEFAULT
> > -- expression. That is a not good, because if you try to recreate the
> > -- function, it's not accepted. I.e. pg_dump & restore is broken
> > alter table at_tab_child no inherit at_tab;
> > ------------
> >
> > The function still works after that. But if you run pg_dump (or do \ef
> or something), the CREATE FUNCTION is deparsed as above, and when you try
> to restore it you get an error:
> >
> > ERROR: argument of DEFAULT must be type at_tab, not type at_tab_child
> > LINE 1: ...CTION public.func_with_default(arg at_tab DEFAULT
> '(1,2)'::a...
> >
> > You get the same effect with ATTACH/DETACH PARTITION instead of
> INHERIT/NO INHERIT.
> >
> > - Heikki
> >
>
> Hi Heikki,
>
> I reproduced your example on 20-devel at 92aaf50e230. The function
> still works after NO INHERIT, but restoring the dump fails.
>
> I’d first try to preserve the current coercion rules:
>
> 1. Reject NO INHERIT/DETACH when it removes the last inheritance path
> needed by a stored ConvertRowtypeExpr, including indirect paths.
> 2. In pg_dump, make objects using these conversions depend on the
> required TABLE ATTACH entries, including intermediate partitions.
>
> This needs a way to find affected expressions in existing databases;
> recording dependencies only when creating new objects would miss them.
> It would also leave already-detached conversions needing repair.
>
> Alternatively, we could allow explicit casts between named composite
> types without inheritance if the source has matching names, types and
> typmods for all target fields, and deparse ConvertRowtypeExpr with that
> cast. Implicit coercions would still require inheritance. This would
> keep DETACH working, but would broaden the explicit-cast rules. Do you
> think that is preferable?
>
> The partition variant also fails to restore before any DETACH:
>
> CREATE TABLE p(a int, b int) PARTITION BY RANGE(a);
> CREATE TABLE c(a int, b int);
> ALTER TABLE p ATTACH PARTITION c FOR VALUES FROM (0) TO (10);
> CREATE FUNCTION f(arg p DEFAULT '(1,2)'::c) RETURNS integer
> LANGUAGE plpgsql AS $$BEGIN RETURN arg.b; END$$;
> -- pg_dump -s puts CREATE FUNCTION before ATTACH; restore fails.
> -- Moving ATTACH before CREATE FUNCTION makes restore succeed.
>
>
Hi Heikki,
In the process of research, I managed to discover the following:
Stored expression trees can contain a composite Const whose value is a
heap-tuple image. ALTER COLUMN TYPE updates the typcache descriptor for
that rowtype and does not rewrite the image. Later readers deform it with
lookup_rowtype_tupdesc() on the type OID from the tuple header, so they use
the new descriptor on the old bytes.
With a crafted stored value that is a deterministic SIGSEGV. With a less
lucky value the same mismatch is just a bad varlena read, for example
```
ERROR: compressed pglz data is corrupt
```
Reproduced on master and on REL_19/18/17/16.
```
create table rt (a int, b int);
create function f(arg rt default '(1073741826, 7)'::rt)
returns int language plpgsql as $$ begin return arg.b; end $$;
alter table rt alter column a type text;
select f();
-- LOG: client backend was terminated by signal 11
```
select pg_get_expr(proargdefaults, 0) from pg_proc where oid =
'f(rt)'::regprocedure crashes the same way. So do \ef (pg_get_functiondef)
and pg_dump. dumpFunc fetches defaults via pg_get_function_arguments(),
which is the same get_const_expr() path.
The planner splices the stored default in with fetch_function_defaults().
plpgsql copies that Datum into arg with expanded_record_set_tuple() and
does not deform it there. RETURN arg.b does: expanded_record_get_field() →
deconstruct_expanded_record() → heap_deform_tuple().
The image for rt(a int, b int) is 32 bytes (t_hoff is 24, plus two int4).
1073741826 is 0x40000002. On little-endian that is the data bytes 02 00 00
40. After a becomes text, those four bytes are read as a varlena header.
The first byte is 0x02, which on LE is a 4-byte compressed header
(VARATT_IS_4B_C), not an uncompressed one. align_fetch_then_add() still
advances with VARSIZE_ANY() → VARSIZE_4B(): va_header >> 2. That is exactly
0x10000000 (256MiB). The next attribute is then fetched as int4 at that
offset, well past the 32-byte buffer.
gdb on select f():
```
#0 heap_deform_tuple () at heaptuple.c:1331
#1 deconstruct_expanded_record () at expandedrecord.c:997
#2 expanded_record_fetch_field () at expandedrecord.c:1075
#3 expanded_record_get_field ()
#4 plpgsql_param_eval_recfield () at pl_exec.c:6893
```
gdb on the pg_get_expr deparse:
```
#0 heap_deform_tuple () at heaptuple.c:1331
#1 record_out () at rowtypes.c:390
#2 FunctionCall1Coll ()
#3 OutputFunctionCall ()
#4 OidOutputFunctionCall ()
#5 get_const_expr () at ruleutils.c:11557
#6 get_rule_expr ()
#7 deparse_expression_pretty ()
#8 pg_get_expr_worker ()
#9 pg_get_expr ()
```
With '(1, 2)' and ALTER COLUMN b TYPE text there is no 256MiB walk. a stays
int4. Deform hands the leftover bytes of b (02 00 00 00) to text output.
First byte 0x02 is again a compressed 4-byte header. Detoast then hits
pglz_decompress_datum(). I get ERROR: compressed pglz data is corrupt from
pg_get_expr on that variant.
find_composite_type_dependencies() refuses ALTER COLUMN TYPE only when some
table column uses the row type.
Functions and views are skipped on purpose, with a comment from bb3da43e3bd
(June 2004):
* We assume that functions and views depending on the type are not
* reasons to reject the ALTER. (How safe is this really?)
That commit predates function parameter defaults. Allowing the ALTER does
not make a wild read of the stale image acceptable. ConvertRowtypeExpr over
a changed rowtype already fails cleanly via attmap.c.
A function is not required. The mismatch is created by ALTER COLUMN TYPE.
Any stored composite Const of that rowtype is enough, as long as no table
column of that type blocks the ALTER.
This is shorter than the function DEFAULT:
create table rt (a int, b int);
create view v as select '(1073741826, 7)'::rt as x;
alter table rt alter column a type text;
select pg_get_viewdef('v'::regclass);
ALTER succeeds. pg_get_viewdef then dies with signal 11, on the same
heap_deform_tuple path as pg_get_expr / record_out.
A table column of type rt is the case the guard already catches:
create table t_col (x rt default '(1073741826, 7)'::rt);
alter table rt alter column a type text;
-- ERROR: cannot alter table "rt" because column "t_col.x" uses its row
type
If the Const sits in an expression that is not such a column, the ALTER is
allowed. For example an int default that only embeds the row value:
create table t_int (n int default (('(1073741826, 7)'::rt).b));
alter table rt alter column a type text; -- succeeds
select pg_get_expr(adbin, adrelid) from pg_attrdef
where adrelid = 't_int'::regclass; -- signal 11
The hole that creates the mismatch is still
find_composite_type_dependencies() looking only at stored table columns.
ALTER changes the rowtype descriptor and leaves the old images in place.
--
Regards,
Rachitskiy Andrey
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Detaching a child table makes an expression using it unrestorable
@ 2026-09-16 07:14 Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
parent: Andrey Rachitskiy <pl0h0yp1@gmail.com>
0 siblings, 1 reply; 7+ messages in thread
From: Manuel Reyes Bravo @ 2026-09-16 07:14 UTC (permalink / raw)
To: Andrey Rachitskiy <pl0h0yp1@gmail.com>; +Cc: Jinqing Kuang <kuangjinqingcn@gmail.com>; Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Hi Andrey, Jinqing, Heikki,
I reproduced Andrey's crash and wrote a patch for it. It is attached,
against master at 04c4c1c3a96. `make check` passes with it, and it adds
regression tests to alter_table.
First, what I measured, because it widens the range Andrey reported.
Affected releases
-----------------
The four-line reproducer
CREATE TABLE rt (a int, b int);
CREATE VIEW v AS SELECT '(1073741826, 7)'::rt AS x;
ALTER TABLE rt ALTER COLUMN a TYPE text;
SELECT pg_get_viewdef('v'::regclass);
kills the backend with SIGSEGV on every one of these:
20devel (master) 19beta2 18.6 17 16 15 14 13
Andrey reported master and REL_19/18/17/16; 15, 14 and 13 fail the same
way. Each of 13 through 18 was a container of the official image for
that release; 19beta2 and master were local builds. The server log is
the same everywhere:
LOG: client backend (PID N) was terminated by signal 11: Segmentation fault
LOG: terminating any other active server processes
That second line is the part that matters for severity: one SELECT from
any user who can create a table and a view takes down every session in
the cluster. `pg_dump -s` dies on the same path, in the pg_attrdef
query, so an affected database also cannot be dumped.
The variant without a crafted value behaves as Andrey described: with
'(1, 2)' and ALTER COLUMN b TYPE text, pg_get_viewdef() returns
ERROR: compressed pglz data is corrupt
The patch
---------
It answers the question the comment in find_composite_type_dependencies()
has been asking since bb3da43e3bd: functions and views are safe to ignore
*unless* the stored expression embeds a constant of the row type. That
constant is a frozen tuple image; the ALTER cannot rewrite it, and the
next reader deforms old bytes with the new descriptor.
So the patch walks the stored expressions and rejects the ALTER when it
finds such a constant:
- the catalogs that store an expression of their own: pg_attrdef,
pg_constraint, pg_policy, pg_proc (proargdefaults and prosqlbody),
pg_rewrite, pg_statistic_ext, pg_trigger;
- index expressions/predicates and partition key expressions, which
pg_depend records as whole-relation dependencies and which therefore
arrive at the relation branch.
Each of those was verified to crash before the patch and to be refused
after it. Example:
ERROR: cannot alter table "rt" because rule _RETURN on view v
stores a constant of its row type
DETAIL: The stored constant holds a row image that the altered type
would misinterpret.
Merely naming the row type is still allowed, which I think is the line
that matters for not breaking working systems: a function with a
parameter of the type, or a view that just reads the table, stores no
image and is untouched. The existing alter_table test at "Use of row
type in an expression is defended differently" also still passes: that
CHECK holds a RowExpr, not a Const.
What it does not do
-------------------
- It prevents new occurrences. A database where the ALTER already
succeeded still holds stale images, and this patch does not repair
it. That is the same gap Jinqing noted for the already-detached
inheritance cases.
- It does not address Heikki's original report. The dump/restore
problem after NO INHERIT / DETACH PARTITION is a separate issue, and
the choice between tightening DETACH and widening the explicit-cast
rules is a design call I did not want to pre-empt.
Questions for whoever picks this up
-----------------------------------
1. Is refusing the ALTER the direction you want, or would you rather
the reader defend itself? Refusing turns a crash into an error
message, but it also rejects ALTERs that work today.
2. Are there other places that store a parsed expression which I have
missed? I went through pg_depend for each reproducer rather than
from a list, so a list would be better than my enumeration.
3. Given that it is a crash reachable by any user who can create
objects, and that it reproduces back to 13, does this want a
backpatch?
Regards,
Manu
El mar, 15 sept 2026 a las 1:26, Andrey Rachitskiy
(<pl0h0yp1@gmail.com>) escribió:
>
>
>
> вт, 15 сент. 2026 г. в 06:54, Jinqing Kuang <kuangjinqingcn@gmail.com>:
>>
>> On Sep 10, 2026, at 22:12, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> >
>> > I bumped into a sequence of commands that breaks pg_dump & restore:
>> >
>> > ------------
>> > create table at_tab (a int, b int);
>> > create table at_tab_child (a int, b int);
>> > alter table at_tab_child inherit at_tab;
>> >
>> > -- You can use the child's rowtype in the DEFAULT without a cast.
>> > CREATE FUNCTION func_with_default(
>> > arg at_tab DEFAULT ('(1, 2)'::at_tab_child)
>> > ) RETURNS integer LANGUAGE plpgsql as $$
>> > begin
>> > return arg.b;
>> > end;
>> > $$;
>> >
>> > -- We allow detaching the child from the parent, despite the DEFAULT
>> > -- expression. That is a not good, because if you try to recreate the
>> > -- function, it's not accepted. I.e. pg_dump & restore is broken
>> > alter table at_tab_child no inherit at_tab;
>> > ------------
>> >
>> > The function still works after that. But if you run pg_dump (or do \ef or something), the CREATE FUNCTION is deparsed as above, and when you try to restore it you get an error:
>> >
>> > ERROR: argument of DEFAULT must be type at_tab, not type at_tab_child
>> > LINE 1: ...CTION public.func_with_default(arg at_tab DEFAULT '(1,2)'::a...
>> >
>> > You get the same effect with ATTACH/DETACH PARTITION instead of INHERIT/NO INHERIT.
>> >
>> > - Heikki
>> >
>>
>> Hi Heikki,
>>
>> I reproduced your example on 20-devel at 92aaf50e230. The function
>> still works after NO INHERIT, but restoring the dump fails.
>>
>> I’d first try to preserve the current coercion rules:
>>
>> 1. Reject NO INHERIT/DETACH when it removes the last inheritance path
>> needed by a stored ConvertRowtypeExpr, including indirect paths.
>> 2. In pg_dump, make objects using these conversions depend on the
>> required TABLE ATTACH entries, including intermediate partitions.
>>
>> This needs a way to find affected expressions in existing databases;
>> recording dependencies only when creating new objects would miss them.
>> It would also leave already-detached conversions needing repair.
>>
>> Alternatively, we could allow explicit casts between named composite
>> types without inheritance if the source has matching names, types and
>> typmods for all target fields, and deparse ConvertRowtypeExpr with that
>> cast. Implicit coercions would still require inheritance. This would
>> keep DETACH working, but would broaden the explicit-cast rules. Do you
>> think that is preferable?
>>
>> The partition variant also fails to restore before any DETACH:
>>
>> CREATE TABLE p(a int, b int) PARTITION BY RANGE(a);
>> CREATE TABLE c(a int, b int);
>> ALTER TABLE p ATTACH PARTITION c FOR VALUES FROM (0) TO (10);
>> CREATE FUNCTION f(arg p DEFAULT '(1,2)'::c) RETURNS integer
>> LANGUAGE plpgsql AS $$BEGIN RETURN arg.b; END$$;
>> -- pg_dump -s puts CREATE FUNCTION before ATTACH; restore fails.
>> -- Moving ATTACH before CREATE FUNCTION makes restore succeed.
>>
>
> Hi Heikki,
>
> In the process of research, I managed to discover the following:
>
> Stored expression trees can contain a composite Const whose value is a heap-tuple image. ALTER COLUMN TYPE updates the typcache descriptor for that rowtype and does not rewrite the image. Later readers deform it with lookup_rowtype_tupdesc() on the type OID from the tuple header, so they use the new descriptor on the old bytes.
>
> With a crafted stored value that is a deterministic SIGSEGV. With a less lucky value the same mismatch is just a bad varlena read, for example
> ```
> ERROR: compressed pglz data is corrupt
> ```
>
> Reproduced on master and on REL_19/18/17/16.
> ```
> create table rt (a int, b int);
> create function f(arg rt default '(1073741826, 7)'::rt)
> returns int language plpgsql as $$ begin return arg.b; end $$;
> alter table rt alter column a type text;
> select f();
> -- LOG: client backend was terminated by signal 11
> ```
>
> select pg_get_expr(proargdefaults, 0) from pg_proc where oid = 'f(rt)'::regprocedure crashes the same way. So do \ef (pg_get_functiondef) and pg_dump. dumpFunc fetches defaults via pg_get_function_arguments(), which is the same get_const_expr() path.
>
> The planner splices the stored default in with fetch_function_defaults(). plpgsql copies that Datum into arg with expanded_record_set_tuple() and does not deform it there. RETURN arg.b does: expanded_record_get_field() → deconstruct_expanded_record() → heap_deform_tuple().
>
> The image for rt(a int, b int) is 32 bytes (t_hoff is 24, plus two int4). 1073741826 is 0x40000002. On little-endian that is the data bytes 02 00 00 40. After a becomes text, those four bytes are read as a varlena header. The first byte is 0x02, which on LE is a 4-byte compressed header (VARATT_IS_4B_C), not an uncompressed one. align_fetch_then_add() still advances with VARSIZE_ANY() → VARSIZE_4B(): va_header >> 2. That is exactly 0x10000000 (256MiB). The next attribute is then fetched as int4 at that offset, well past the 32-byte buffer.
>
> gdb on select f():
> ```
> #0 heap_deform_tuple () at heaptuple.c:1331
> #1 deconstruct_expanded_record () at expandedrecord.c:997
> #2 expanded_record_fetch_field () at expandedrecord.c:1075
> #3 expanded_record_get_field ()
> #4 plpgsql_param_eval_recfield () at pl_exec.c:6893
> ```
> gdb on the pg_get_expr deparse:
> ```
> #0 heap_deform_tuple () at heaptuple.c:1331
> #1 record_out () at rowtypes.c:390
> #2 FunctionCall1Coll ()
> #3 OutputFunctionCall ()
> #4 OidOutputFunctionCall ()
> #5 get_const_expr () at ruleutils.c:11557
> #6 get_rule_expr ()
> #7 deparse_expression_pretty ()
> #8 pg_get_expr_worker ()
> #9 pg_get_expr ()
> ```
> With '(1, 2)' and ALTER COLUMN b TYPE text there is no 256MiB walk. a stays int4. Deform hands the leftover bytes of b (02 00 00 00) to text output. First byte 0x02 is again a compressed 4-byte header. Detoast then hits pglz_decompress_datum(). I get ERROR: compressed pglz data is corrupt from pg_get_expr on that variant.
>
> find_composite_type_dependencies() refuses ALTER COLUMN TYPE only when some table column uses the row type.
> Functions and views are skipped on purpose, with a comment from bb3da43e3bd (June 2004):
>
> * We assume that functions and views depending on the type are not
> * reasons to reject the ALTER. (How safe is this really?)
>
> That commit predates function parameter defaults. Allowing the ALTER does not make a wild read of the stale image acceptable. ConvertRowtypeExpr over a changed rowtype already fails cleanly via attmap.c.
>
> A function is not required. The mismatch is created by ALTER COLUMN TYPE. Any stored composite Const of that rowtype is enough, as long as no table column of that type blocks the ALTER.
>
> This is shorter than the function DEFAULT:
>
> create table rt (a int, b int);
> create view v as select '(1073741826, 7)'::rt as x;
> alter table rt alter column a type text;
> select pg_get_viewdef('v'::regclass);
> ALTER succeeds. pg_get_viewdef then dies with signal 11, on the same heap_deform_tuple path as pg_get_expr / record_out.
>
> A table column of type rt is the case the guard already catches:
>
> create table t_col (x rt default '(1073741826, 7)'::rt);
> alter table rt alter column a type text;
> -- ERROR: cannot alter table "rt" because column "t_col.x" uses its row type
> If the Const sits in an expression that is not such a column, the ALTER is allowed. For example an int default that only embeds the row value:
>
> create table t_int (n int default (('(1073741826, 7)'::rt).b));
> alter table rt alter column a type text; -- succeeds
> select pg_get_expr(adbin, adrelid) from pg_attrdef
> where adrelid = 't_int'::regclass; -- signal 11
>
> The hole that creates the mismatch is still find_composite_type_dependencies() looking only at stored table columns. ALTER changes the rowtype descriptor and leaves the old images in place.
>
> --
> Regards,
> Rachitskiy Andrey
--
Saludos cordiales,
Manuel Reyes
Attachments:
[text/x-patch] 0001-Refuse-ALTER-COLUMN-TYPE-when-a-stored-constant-hold.patch (18.2K, ../../CA+bCEdDvsVApd+AADX=5uyj6Q5C3r5aruWNzOFaYqaFqLwE_5g@mail.gmail.com/2-0001-Refuse-ALTER-COLUMN-TYPE-when-a-stored-constant-hold.patch)
download | inline diff:
From 3e1c0b983742b25e1e35d2ca5125ac768b92bdcf Mon Sep 17 00:00:00 2001
From: Manu <manuelreyesbravo@gmail.com>
Date: Wed, 16 Sep 2026 03:58:48 -0300
Subject: [PATCH] Refuse ALTER COLUMN TYPE when a stored constant holds that
row type
A composite Const carries a heap tuple image whose bytes are frozen when the
expression is parsed. ALTER TABLE ... ALTER COLUMN TYPE installs a new
descriptor for the row type but cannot rewrite those images, so whatever
deforms one afterwards reads the old bytes through the new descriptor. With
a value whose bytes happen to look like a compressed varlena header that is an
out-of-bounds read and the backend dies with SIGSEGV, taking every other
session down with it; with other values it surfaces as "compressed pglz data
is corrupt". pg_get_viewdef(), pg_get_expr() and pg_dump reach the same path,
so an affected database also cannot be dumped.
find_composite_type_dependencies() refused the ALTER only when a table column
used the row type. Functions and views were skipped deliberately, with a
comment from bb3da43e3bd (2004) asking "How safe is this really?". That
assumption predates function parameter defaults; this patch answers the
question: it is safe unless the expression stores a *constant* of the row
type.
So look inside the stored expressions. The catalogs that keep one of their
own are checked directly (pg_attrdef, pg_constraint, pg_policy, pg_proc --
both proargdefaults and prosqlbody -- pg_rewrite, pg_statistic_ext and
pg_trigger); index and partition key expressions arrive as whole-relation
dependencies and are checked there. Merely naming the row type is still
allowed: a function parameter of the type, or a view that reads the table,
carries no frozen image and is not affected.
This prevents new occurrences. A database where the ALTER already succeeded
still holds the stale images and is not repaired by this patch.
---
src/backend/commands/tablecmds.c | 296 +++++++++++++++++++++-
src/test/regress/expected/alter_table.out | 36 +++
src/test/regress/sql/alter_table.sql | 26 ++
3 files changed, 356 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 2f073ddb84a..6e092b0d1d0 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -46,6 +46,7 @@
#include "catalog/pg_largeobject_metadata.h"
#include "catalog/pg_namespace.h"
#include "catalog/pg_opclass.h"
+#include "catalog/pg_partitioned_table.h"
#include "catalog/pg_policy.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_publication_rel.h"
@@ -661,6 +662,8 @@ static void ATPrepAlterColumnType(List **wqueue,
AlterTableCmd *cmd, LOCKMODE lockmode,
AlterTableUtilityContext *context);
static bool ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno);
+static bool stored_expr_has_rowtype_const(Oid classId, Oid objectId,
+ Oid typeOid);
static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
AlterTableCmd *cmd, LOCKMODE lockmode);
static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype,
@@ -6990,6 +6993,232 @@ ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
}
+/*
+ * Context for has_rowtype_const_walker: the row type we are looking for.
+ */
+typedef struct
+{
+ Oid typeOid;
+} HasRowtypeConstContext;
+
+/*
+ * Does this expression tree embed a non-null Const of the given row type?
+ */
+static bool
+has_rowtype_const_walker(Node *node, HasRowtypeConstContext *context)
+{
+ if (node == NULL)
+ return false;
+ if (IsA(node, Const))
+ {
+ Const *con = (Const *) node;
+
+ if (con->consttype == context->typeOid && !con->constisnull)
+ return true;
+ }
+ if (IsA(node, Query))
+ return query_tree_walker((Query *) node, has_rowtype_const_walker,
+ context, 0);
+ return expression_tree_walker(node, has_rowtype_const_walker, context);
+}
+
+/*
+ * Catalogs that store a parsed expression which is deformed later, and the
+ * columns that hold it.
+ *
+ * pg_proc is not here because its defaults are reached through the syscache.
+ * Index and partition expressions are not here either: pg_depend records them
+ * as whole-relation dependencies, so they arrive at the relation branch of
+ * find_composite_type_dependencies() instead.
+ */
+typedef struct
+{
+ Oid classId;
+ Oid oidIndexId;
+ AttrNumber oidAttNum;
+ AttrNumber exprAttNum[2]; /* a zero ends the list */
+} StoredExprCatalog;
+
+static const StoredExprCatalog stored_expr_catalogs[] = {
+ {AttrDefaultRelationId, AttrDefaultOidIndexId, Anum_pg_attrdef_oid,
+ {Anum_pg_attrdef_adbin, 0}},
+ {ConstraintRelationId, ConstraintOidIndexId, Anum_pg_constraint_oid,
+ {Anum_pg_constraint_conbin, 0}},
+ {PolicyRelationId, PolicyOidIndexId, Anum_pg_policy_oid,
+ {Anum_pg_policy_polqual, Anum_pg_policy_polwithcheck}},
+ {RewriteRelationId, RewriteOidIndexId, Anum_pg_rewrite_oid,
+ {Anum_pg_rewrite_ev_action, Anum_pg_rewrite_ev_qual}},
+ {StatisticExtRelationId, StatisticExtOidIndexId, Anum_pg_statistic_ext_oid,
+ {Anum_pg_statistic_ext_stxexprs, 0}},
+ {TriggerRelationId, TriggerOidIndexId, Anum_pg_trigger_oid,
+ {Anum_pg_trigger_tgqual, 0}},
+};
+
+/*
+ * Is this catalog one that stores expressions we must inspect?
+ */
+static const StoredExprCatalog *
+stored_expr_catalog_lookup(Oid classId)
+{
+ for (int i = 0; i < lengthof(stored_expr_catalogs); i++)
+ {
+ if (stored_expr_catalogs[i].classId == classId)
+ return &stored_expr_catalogs[i];
+ }
+ return NULL;
+}
+
+/*
+ * stored_expr_has_rowtype_const
+ *
+ * Fetch the expression stored in the given catalog row, and report whether it
+ * contains a constant of the given row type.
+ *
+ * A composite Const carries a heap tuple image whose bytes were frozen when
+ * the expression was parsed. ALTER TABLE ... ALTER COLUMN TYPE installs a new
+ * descriptor for the row type but cannot rewrite those images, so whoever
+ * deforms one afterwards reads the old bytes through the new descriptor. That
+ * is not a display problem: if the new column is wider, or is a varlena where
+ * an integer used to be, the reader walks off the end of the image.
+ *
+ * Only pg_attrdef, pg_proc and pg_rewrite are examined; those are the places
+ * where a parsed expression is stored and later deformed.
+ */
+static bool
+stored_expr_has_rowtype_const(Oid classId, Oid objectId, Oid typeOid)
+{
+ HasRowtypeConstContext context;
+ Relation catalog;
+ SysScanDesc scan;
+ ScanKeyData key;
+ HeapTuple tup;
+ Datum datum;
+ bool isnull;
+ bool found = false;
+ const StoredExprCatalog *cat;
+
+ context.typeOid = typeOid;
+
+ if (classId == ProcedureRelationId)
+ {
+ tup = SearchSysCache1(PROCOID, ObjectIdGetDatum(objectId));
+ if (!HeapTupleIsValid(tup))
+ return false;
+ datum = SysCacheGetAttr(PROCOID, tup,
+ Anum_pg_proc_proargdefaults, &isnull);
+ if (!isnull)
+ found = has_rowtype_const_walker(stringToNode(TextDatumGetCString(datum)),
+ &context);
+ if (!found)
+ {
+ /* A standard-conforming SQL body is stored parsed, too */
+ datum = SysCacheGetAttr(PROCOID, tup,
+ Anum_pg_proc_prosqlbody, &isnull);
+ if (!isnull)
+ found = has_rowtype_const_walker(stringToNode(TextDatumGetCString(datum)),
+ &context);
+ }
+ ReleaseSysCache(tup);
+ return found;
+ }
+
+ cat = stored_expr_catalog_lookup(classId);
+ if (cat == NULL)
+ return false;
+
+ catalog = table_open(classId, AccessShareLock);
+
+ ScanKeyInit(&key,
+ cat->oidAttNum,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(objectId));
+
+ scan = systable_beginscan(catalog, cat->oidIndexId, true, NULL, 1, &key);
+
+ tup = systable_getnext(scan);
+ if (HeapTupleIsValid(tup))
+ {
+ for (int i = 0; i < lengthof(cat->exprAttNum); i++)
+ {
+ if (cat->exprAttNum[i] == 0)
+ break;
+ datum = heap_getattr(tup, cat->exprAttNum[i],
+ RelationGetDescr(catalog), &isnull);
+ if (isnull)
+ continue;
+ if (has_rowtype_const_walker(stringToNode(TextDatumGetCString(datum)),
+ &context))
+ {
+ found = true;
+ break;
+ }
+ }
+ }
+
+ systable_endscan(scan);
+ table_close(catalog, AccessShareLock);
+
+ return found;
+}
+
+/*
+ * relation_expr_has_rowtype_const
+ *
+ * Same question as stored_expr_has_rowtype_const(), for the expressions that
+ * hang off a relation rather than off a catalog row of their own: index
+ * expressions and predicates, and partition key expressions. pg_depend
+ * records those as whole-relation dependencies.
+ */
+static bool
+relation_expr_has_rowtype_const(Relation rel, Oid typeOid)
+{
+ HasRowtypeConstContext context;
+ HeapTuple tup;
+ Datum datum;
+ bool isnull;
+ bool found = false;
+
+ context.typeOid = typeOid;
+
+ if (rel->rd_rel->relkind == RELKIND_INDEX ||
+ rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
+ {
+ tup = SearchSysCache1(INDEXRELID,
+ ObjectIdGetDatum(RelationGetRelid(rel)));
+ if (!HeapTupleIsValid(tup))
+ return false;
+ datum = SysCacheGetAttr(INDEXRELID, tup, Anum_pg_index_indexprs,
+ &isnull);
+ if (!isnull)
+ found = has_rowtype_const_walker(stringToNode(TextDatumGetCString(datum)),
+ &context);
+ if (!found)
+ {
+ datum = SysCacheGetAttr(INDEXRELID, tup, Anum_pg_index_indpred,
+ &isnull);
+ if (!isnull)
+ found = has_rowtype_const_walker(stringToNode(TextDatumGetCString(datum)),
+ &context);
+ }
+ ReleaseSysCache(tup);
+ }
+ else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ tup = SearchSysCache1(PARTRELID,
+ ObjectIdGetDatum(RelationGetRelid(rel)));
+ if (!HeapTupleIsValid(tup))
+ return false;
+ datum = SysCacheGetAttr(PARTRELID, tup,
+ Anum_pg_partitioned_table_partexprs, &isnull);
+ if (!isnull)
+ found = has_rowtype_const_walker(stringToNode(TextDatumGetCString(datum)),
+ &context);
+ ReleaseSysCache(tup);
+ }
+
+ return found;
+}
+
/*
* find_composite_type_dependencies
*
@@ -7006,8 +7235,9 @@ ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
* these things. The name of this function is therefore somewhat historical,
* but it's not worth changing.
*
- * We assume that functions and views depending on the type are not reasons
- * to reject the ALTER. (How safe is this really?)
+ * Functions and views depending on the type are generally not reasons to
+ * reject the ALTER, with one exception: a stored expression that embeds a
+ * *constant* of the row type. See stored_expr_has_rowtype_const().
*/
void
find_composite_type_dependencies(Oid typeOid, Relation origRelation,
@@ -7062,6 +7292,42 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
continue;
}
+ /*
+ * A stored expression is not a reason to reject the ALTER by itself,
+ * but one that embeds a *constant* of this row type is: the constant
+ * holds a tuple image that we cannot rewrite here, and reading it
+ * afterwards through the new descriptor is an out-of-bounds read, not
+ * a wrong answer.
+ */
+ if (pg_depend->classid == ProcedureRelationId ||
+ stored_expr_catalog_lookup(pg_depend->classid) != NULL)
+ {
+ if (stored_expr_has_rowtype_const(pg_depend->classid,
+ pg_depend->objid, typeOid))
+ {
+ ObjectAddress obj;
+ char *objdesc;
+
+ ObjectAddressSet(obj, pg_depend->classid, pg_depend->objid);
+ objdesc = getObjectDescription(&obj, false);
+
+ if (origTypeName)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot alter type \"%s\" because %s stores a constant of that type",
+ origTypeName, objdesc),
+ errdetail("The stored constant holds a row image that the altered type would misinterpret.")));
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot alter table \"%s\" because %s stores a constant of its row type",
+ RelationGetRelationName(origRelation),
+ objdesc),
+ errdetail("The stored constant holds a row image that the altered type would misinterpret.")));
+ }
+ continue;
+ }
+
/* Else, ignore dependees that aren't relations */
if (pg_depend->classid != RelationRelationId)
continue;
@@ -7069,6 +7335,32 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
rel = relation_open(pg_depend->objid, AccessShareLock);
tupleDesc = RelationGetDescr(rel);
+ /*
+ * Index and partition key expressions reach us as whole-relation
+ * dependencies, so look inside them here. A constant of the row type
+ * in one of them is as dangerous as in any other stored expression.
+ */
+ if (relation_expr_has_rowtype_const(rel, typeOid))
+ {
+ char *relname = pstrdup(RelationGetRelationName(rel));
+
+ relation_close(rel, AccessShareLock);
+
+ if (origTypeName)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot alter type \"%s\" because an expression of \"%s\" stores a constant of that type",
+ origTypeName, relname),
+ errdetail("The stored constant holds a row image that the altered type would misinterpret.")));
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot alter table \"%s\" because an expression of \"%s\" stores a constant of its row type",
+ RelationGetRelationName(origRelation),
+ relname),
+ errdetail("The stored constant holds a row image that the altered type would misinterpret.")));
+ }
+
/*
* If objsubid identifies a specific column, refer to that in error
* messages. Otherwise, search to see if there's a user column of the
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index e167a41ce79..96aae5fabb3 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2172,6 +2172,42 @@ alter table at_tab1 alter column b type varchar; -- allowed, but ...
insert into at_tab2 values(1,'42'); -- ... this will fail
ERROR: ROW() column has type text instead of type character varying
drop table at_tab1, at_tab2;
+-- A stored *constant* of the row type is rejected: it holds a tuple image
+-- that ALTER COLUMN TYPE cannot rewrite, and reading it afterwards through
+-- the new descriptor is an out-of-bounds read, not a wrong answer.
+create table at_tab1 (a int, b text);
+create table at_tab2 (x int default (('(1,42)'::at_tab1).a));
+alter table at_tab1 alter column b type varchar; -- fails
+ERROR: cannot alter table "at_tab1" because default value for column x of table at_tab2 stores a constant of its row type
+DETAIL: The stored constant holds a row image that the altered type would misinterpret.
+drop table at_tab2;
+create view at_view1 as select '(1,42)'::at_tab1 as x;
+alter table at_tab1 alter column b type varchar; -- fails
+ERROR: cannot alter table "at_tab1" because rule _RETURN on view at_view1 stores a constant of its row type
+DETAIL: The stored constant holds a row image that the altered type would misinterpret.
+drop view at_view1;
+create table at_tab2 (x int check (x = ('(1,42)'::at_tab1).a));
+alter table at_tab1 alter column b type varchar; -- fails
+ERROR: cannot alter table "at_tab1" because constraint at_tab2_x_check on table at_tab2 stores a constant of its row type
+DETAIL: The stored constant holds a row image that the altered type would misinterpret.
+drop table at_tab2;
+create table at_tab2 (x int);
+create index at_tab2_idx on at_tab2 ((x + ('(1,42)'::at_tab1).a));
+alter table at_tab1 alter column b type varchar; -- fails
+ERROR: cannot alter table "at_tab1" because an expression of "at_tab2_idx" stores a constant of its row type
+DETAIL: The stored constant holds a row image that the altered type would misinterpret.
+drop table at_tab2;
+create function at_func1(arg at_tab1 default '(1,42)'::at_tab1)
+ returns int language sql as 'select 1';
+alter table at_tab1 alter column b type varchar; -- fails
+ERROR: cannot alter table "at_tab1" because function at_func1(at_tab1) stores a constant of its row type
+DETAIL: The stored constant holds a row image that the altered type would misinterpret.
+drop function at_func1;
+-- but merely mentioning the row type, with no constant stored, is fine
+create function at_func1(arg at_tab1) returns int language sql as 'select 1';
+alter table at_tab1 alter column b type varchar; -- allowed
+drop function at_func1;
+drop table at_tab1;
-- Check it for a partitioned table, too
create table at_tab1 (a int, b text) partition by list(a);
create table at_tab2 (x int, y at_tab1);
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 9f6c2a4bb08..494d8ae1c5c 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1448,6 +1448,32 @@ create table at_tab2 (x int, y text, check((x,y)::at_tab1 = (1,'42')::at_tab1));
alter table at_tab1 alter column b type varchar; -- allowed, but ...
insert into at_tab2 values(1,'42'); -- ... this will fail
drop table at_tab1, at_tab2;
+-- A stored *constant* of the row type is rejected: it holds a tuple image
+-- that ALTER COLUMN TYPE cannot rewrite, and reading it afterwards through
+-- the new descriptor is an out-of-bounds read, not a wrong answer.
+create table at_tab1 (a int, b text);
+create table at_tab2 (x int default (('(1,42)'::at_tab1).a));
+alter table at_tab1 alter column b type varchar; -- fails
+drop table at_tab2;
+create view at_view1 as select '(1,42)'::at_tab1 as x;
+alter table at_tab1 alter column b type varchar; -- fails
+drop view at_view1;
+create table at_tab2 (x int check (x = ('(1,42)'::at_tab1).a));
+alter table at_tab1 alter column b type varchar; -- fails
+drop table at_tab2;
+create table at_tab2 (x int);
+create index at_tab2_idx on at_tab2 ((x + ('(1,42)'::at_tab1).a));
+alter table at_tab1 alter column b type varchar; -- fails
+drop table at_tab2;
+create function at_func1(arg at_tab1 default '(1,42)'::at_tab1)
+ returns int language sql as 'select 1';
+alter table at_tab1 alter column b type varchar; -- fails
+drop function at_func1;
+-- but merely mentioning the row type, with no constant stored, is fine
+create function at_func1(arg at_tab1) returns int language sql as 'select 1';
+alter table at_tab1 alter column b type varchar; -- allowed
+drop function at_func1;
+drop table at_tab1;
-- Check it for a partitioned table, too
create table at_tab1 (a int, b text) partition by list(a);
create table at_tab2 (x int, y at_tab1);
--
2.55.0
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Detaching a child table makes an expression using it unrestorable
@ 2026-09-21 04:39 Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
parent: Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
0 siblings, 1 reply; 7+ messages in thread
From: Manuel Reyes Bravo @ 2026-09-21 04:39 UTC (permalink / raw)
To: Andrey Rachitskiy <pl0h0yp1@gmail.com>; +Cc: Jinqing Kuang <kuangjinqingcn@gmail.com>; Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Hi,
Attached is v2 of the patch that refuses ALTER COLUMN TYPE when a
stored expression holds a constant of the row type.
It fixes the wording for stand-alone composite types. v1 reported
ALTER TYPE t ALTER ATTRIBUTE b TYPE varchar;
ERROR: cannot alter table "t" because rule _RETURN on view v
stores a constant of its row type
where the existing column checks in find_composite_type_dependencies()
correctly say "cannot alter type". Both of the new reporting sites now
go through one helper that follows those checks (type, foreign table or
table), and the index case names the object the same way as the others
("index at_tab2_idx" instead of 'an expression of "at_tab2_idx"').
A regression test for the composite type case is added; it fails with
v1. make check passes.
I found it while reviewing Nikhil's patch in "Dropping a composite
attribute causes data integrity violations" [1], which makes ALTER TYPE
... DROP ATTRIBUTE and ALTER TABLE ... DROP COLUMN call
find_composite_type_dependencies() as well. The two patches compose:
with both, a drop is also refused when a stored constant of the row
type would change its meaning. The details are in that thread.
[1] https://postgr.es/m/CA+UBoq0F2ua2fQEAU3-6w0oxOgFc9BgEN4gRcchiUUZ+WouVrQ@mail.gmail.com
Regards,
Manu
Attachments:
[text/x-patch] v2-0001-Refuse-ALTER-COLUMN-TYPE-when-a-stored-constant-h.patch (19.5K, ../../CA+bCEdC167OdU3tHnRhLjGSibDMiPHODwSncCcOCxUZdWkUORA@mail.gmail.com/2-v2-0001-Refuse-ALTER-COLUMN-TYPE-when-a-stored-constant-h.patch)
download | inline diff:
From afe2f83a558df101240fd70b54318d2390a648b1 Mon Sep 17 00:00:00 2001
From: Manu <manuelreyesbravo@gmail.com>
Date: Wed, 16 Sep 2026 03:58:48 -0300
Subject: [PATCH v2] Refuse ALTER COLUMN TYPE when a stored constant holds that
row type
A composite Const carries a heap tuple image whose bytes are frozen when the
expression is parsed. ALTER TABLE ... ALTER COLUMN TYPE installs a new
descriptor for the row type but cannot rewrite those images, so whatever
deforms one afterwards reads the old bytes through the new descriptor. With
a value whose bytes happen to look like a compressed varlena header that is an
out-of-bounds read and the backend dies with SIGSEGV, taking every other
session down with it; with other values it surfaces as "compressed pglz data
is corrupt". pg_get_viewdef(), pg_get_expr() and pg_dump reach the same path,
so an affected database also cannot be dumped.
find_composite_type_dependencies() refused the ALTER only when a table column
used the row type. Functions and views were skipped deliberately, with a
comment from bb3da43e3bd (2004) asking "How safe is this really?". That
assumption predates function parameter defaults; this patch answers the
question: it is safe unless the expression stores a *constant* of the row
type.
So look inside the stored expressions. The catalogs that keep one of their
own are checked directly (pg_attrdef, pg_constraint, pg_policy, pg_proc --
both proargdefaults and prosqlbody -- pg_rewrite, pg_statistic_ext and
pg_trigger); index and partition key expressions arrive as whole-relation
dependencies and are checked there. Merely naming the row type is still
allowed: a function parameter of the type, or a view that reads the table,
carries no frozen image and is not affected.
This prevents new occurrences. A database where the ALTER already succeeded
still holds the stale images and is not repaired by this patch.
---
src/backend/commands/tablecmds.c | 311 +++++++++++++++++++++-
src/test/regress/expected/alter_table.out | 44 +++
src/test/regress/sql/alter_table.sql | 32 +++
3 files changed, 385 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 2f073ddb84a..180109e555a 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -46,6 +46,7 @@
#include "catalog/pg_largeobject_metadata.h"
#include "catalog/pg_namespace.h"
#include "catalog/pg_opclass.h"
+#include "catalog/pg_partitioned_table.h"
#include "catalog/pg_policy.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_publication_rel.h"
@@ -661,6 +662,8 @@ static void ATPrepAlterColumnType(List **wqueue,
AlterTableCmd *cmd, LOCKMODE lockmode,
AlterTableUtilityContext *context);
static bool ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno);
+static bool stored_expr_has_rowtype_const(Oid classId, Oid objectId,
+ Oid typeOid);
static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
AlterTableCmd *cmd, LOCKMODE lockmode);
static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype,
@@ -6990,6 +6993,268 @@ ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
}
+/*
+ * Context for has_rowtype_const_walker: the row type we are looking for.
+ */
+typedef struct
+{
+ Oid typeOid;
+} HasRowtypeConstContext;
+
+/*
+ * Does this expression tree embed a non-null Const of the given row type?
+ */
+static bool
+has_rowtype_const_walker(Node *node, HasRowtypeConstContext *context)
+{
+ if (node == NULL)
+ return false;
+ if (IsA(node, Const))
+ {
+ Const *con = (Const *) node;
+
+ if (con->consttype == context->typeOid && !con->constisnull)
+ return true;
+ }
+ if (IsA(node, Query))
+ return query_tree_walker((Query *) node, has_rowtype_const_walker,
+ context, 0);
+ return expression_tree_walker(node, has_rowtype_const_walker, context);
+}
+
+/*
+ * Catalogs that store a parsed expression which is deformed later, and the
+ * columns that hold it.
+ *
+ * pg_proc is not here because its defaults are reached through the syscache.
+ * Index and partition expressions are not here either: pg_depend records them
+ * as whole-relation dependencies, so they arrive at the relation branch of
+ * find_composite_type_dependencies() instead.
+ */
+typedef struct
+{
+ Oid classId;
+ Oid oidIndexId;
+ AttrNumber oidAttNum;
+ AttrNumber exprAttNum[2]; /* a zero ends the list */
+} StoredExprCatalog;
+
+static const StoredExprCatalog stored_expr_catalogs[] = {
+ {AttrDefaultRelationId, AttrDefaultOidIndexId, Anum_pg_attrdef_oid,
+ {Anum_pg_attrdef_adbin, 0}},
+ {ConstraintRelationId, ConstraintOidIndexId, Anum_pg_constraint_oid,
+ {Anum_pg_constraint_conbin, 0}},
+ {PolicyRelationId, PolicyOidIndexId, Anum_pg_policy_oid,
+ {Anum_pg_policy_polqual, Anum_pg_policy_polwithcheck}},
+ {RewriteRelationId, RewriteOidIndexId, Anum_pg_rewrite_oid,
+ {Anum_pg_rewrite_ev_action, Anum_pg_rewrite_ev_qual}},
+ {StatisticExtRelationId, StatisticExtOidIndexId, Anum_pg_statistic_ext_oid,
+ {Anum_pg_statistic_ext_stxexprs, 0}},
+ {TriggerRelationId, TriggerOidIndexId, Anum_pg_trigger_oid,
+ {Anum_pg_trigger_tgqual, 0}},
+};
+
+/*
+ * Is this catalog one that stores expressions we must inspect?
+ */
+static const StoredExprCatalog *
+stored_expr_catalog_lookup(Oid classId)
+{
+ for (int i = 0; i < lengthof(stored_expr_catalogs); i++)
+ {
+ if (stored_expr_catalogs[i].classId == classId)
+ return &stored_expr_catalogs[i];
+ }
+ return NULL;
+}
+
+/*
+ * stored_expr_has_rowtype_const
+ *
+ * Fetch the expression stored in the given catalog row, and report whether it
+ * contains a constant of the given row type.
+ *
+ * A composite Const carries a heap tuple image whose bytes were frozen when
+ * the expression was parsed. ALTER TABLE ... ALTER COLUMN TYPE installs a new
+ * descriptor for the row type but cannot rewrite those images, so whoever
+ * deforms one afterwards reads the old bytes through the new descriptor. That
+ * is not a display problem: if the new column is wider, or is a varlena where
+ * an integer used to be, the reader walks off the end of the image.
+ *
+ * Only pg_attrdef, pg_proc and pg_rewrite are examined; those are the places
+ * where a parsed expression is stored and later deformed.
+ */
+static bool
+stored_expr_has_rowtype_const(Oid classId, Oid objectId, Oid typeOid)
+{
+ HasRowtypeConstContext context;
+ Relation catalog;
+ SysScanDesc scan;
+ ScanKeyData key;
+ HeapTuple tup;
+ Datum datum;
+ bool isnull;
+ bool found = false;
+ const StoredExprCatalog *cat;
+
+ context.typeOid = typeOid;
+
+ if (classId == ProcedureRelationId)
+ {
+ tup = SearchSysCache1(PROCOID, ObjectIdGetDatum(objectId));
+ if (!HeapTupleIsValid(tup))
+ return false;
+ datum = SysCacheGetAttr(PROCOID, tup,
+ Anum_pg_proc_proargdefaults, &isnull);
+ if (!isnull)
+ found = has_rowtype_const_walker(stringToNode(TextDatumGetCString(datum)),
+ &context);
+ if (!found)
+ {
+ /* A standard-conforming SQL body is stored parsed, too */
+ datum = SysCacheGetAttr(PROCOID, tup,
+ Anum_pg_proc_prosqlbody, &isnull);
+ if (!isnull)
+ found = has_rowtype_const_walker(stringToNode(TextDatumGetCString(datum)),
+ &context);
+ }
+ ReleaseSysCache(tup);
+ return found;
+ }
+
+ cat = stored_expr_catalog_lookup(classId);
+ if (cat == NULL)
+ return false;
+
+ catalog = table_open(classId, AccessShareLock);
+
+ ScanKeyInit(&key,
+ cat->oidAttNum,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(objectId));
+
+ scan = systable_beginscan(catalog, cat->oidIndexId, true, NULL, 1, &key);
+
+ tup = systable_getnext(scan);
+ if (HeapTupleIsValid(tup))
+ {
+ for (int i = 0; i < lengthof(cat->exprAttNum); i++)
+ {
+ if (cat->exprAttNum[i] == 0)
+ break;
+ datum = heap_getattr(tup, cat->exprAttNum[i],
+ RelationGetDescr(catalog), &isnull);
+ if (isnull)
+ continue;
+ if (has_rowtype_const_walker(stringToNode(TextDatumGetCString(datum)),
+ &context))
+ {
+ found = true;
+ break;
+ }
+ }
+ }
+
+ systable_endscan(scan);
+ table_close(catalog, AccessShareLock);
+
+ return found;
+}
+
+/*
+ * report_stored_rowtype_const
+ *
+ * Refuse the ALTER because 'objdesc' stores a constant of the row type being
+ * altered. The wording follows the column checks in
+ * find_composite_type_dependencies(): altering a stand-alone composite type
+ * is "altering a type", not a table, whether it arrives as origTypeName or as
+ * the composite type's own relation (ALTER TYPE ... ALTER/DROP ATTRIBUTE).
+ */
+static void
+report_stored_rowtype_const(Relation origRelation, const char *origTypeName,
+ const char *objdesc)
+{
+ if (origTypeName ||
+ origRelation->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot alter type \"%s\" because %s stores a constant of that type",
+ origTypeName ? origTypeName :
+ RelationGetRelationName(origRelation),
+ objdesc),
+ errdetail("The stored constant holds a row image that the altered type would misinterpret.")));
+ else if (origRelation->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot alter foreign table \"%s\" because %s stores a constant of its row type",
+ RelationGetRelationName(origRelation), objdesc),
+ errdetail("The stored constant holds a row image that the altered type would misinterpret.")));
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot alter table \"%s\" because %s stores a constant of its row type",
+ RelationGetRelationName(origRelation), objdesc),
+ errdetail("The stored constant holds a row image that the altered type would misinterpret.")));
+}
+
+/*
+ * relation_expr_has_rowtype_const
+ *
+ * Same question as stored_expr_has_rowtype_const(), for the expressions that
+ * hang off a relation rather than off a catalog row of their own: index
+ * expressions and predicates, and partition key expressions. pg_depend
+ * records those as whole-relation dependencies.
+ */
+static bool
+relation_expr_has_rowtype_const(Relation rel, Oid typeOid)
+{
+ HasRowtypeConstContext context;
+ HeapTuple tup;
+ Datum datum;
+ bool isnull;
+ bool found = false;
+
+ context.typeOid = typeOid;
+
+ if (rel->rd_rel->relkind == RELKIND_INDEX ||
+ rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
+ {
+ tup = SearchSysCache1(INDEXRELID,
+ ObjectIdGetDatum(RelationGetRelid(rel)));
+ if (!HeapTupleIsValid(tup))
+ return false;
+ datum = SysCacheGetAttr(INDEXRELID, tup, Anum_pg_index_indexprs,
+ &isnull);
+ if (!isnull)
+ found = has_rowtype_const_walker(stringToNode(TextDatumGetCString(datum)),
+ &context);
+ if (!found)
+ {
+ datum = SysCacheGetAttr(INDEXRELID, tup, Anum_pg_index_indpred,
+ &isnull);
+ if (!isnull)
+ found = has_rowtype_const_walker(stringToNode(TextDatumGetCString(datum)),
+ &context);
+ }
+ ReleaseSysCache(tup);
+ }
+ else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ tup = SearchSysCache1(PARTRELID,
+ ObjectIdGetDatum(RelationGetRelid(rel)));
+ if (!HeapTupleIsValid(tup))
+ return false;
+ datum = SysCacheGetAttr(PARTRELID, tup,
+ Anum_pg_partitioned_table_partexprs, &isnull);
+ if (!isnull)
+ found = has_rowtype_const_walker(stringToNode(TextDatumGetCString(datum)),
+ &context);
+ ReleaseSysCache(tup);
+ }
+
+ return found;
+}
+
/*
* find_composite_type_dependencies
*
@@ -7006,8 +7271,9 @@ ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
* these things. The name of this function is therefore somewhat historical,
* but it's not worth changing.
*
- * We assume that functions and views depending on the type are not reasons
- * to reject the ALTER. (How safe is this really?)
+ * Functions and views depending on the type are generally not reasons to
+ * reject the ALTER, with one exception: a stored expression that embeds a
+ * *constant* of the row type. See stored_expr_has_rowtype_const().
*/
void
find_composite_type_dependencies(Oid typeOid, Relation origRelation,
@@ -7062,6 +7328,30 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
continue;
}
+ /*
+ * A stored expression is not a reason to reject the ALTER by itself,
+ * but one that embeds a *constant* of this row type is: the constant
+ * holds a tuple image that we cannot rewrite here, and reading it
+ * afterwards through the new descriptor is an out-of-bounds read, not
+ * a wrong answer.
+ */
+ if (pg_depend->classid == ProcedureRelationId ||
+ stored_expr_catalog_lookup(pg_depend->classid) != NULL)
+ {
+ if (stored_expr_has_rowtype_const(pg_depend->classid,
+ pg_depend->objid, typeOid))
+ {
+ ObjectAddress obj;
+ char *objdesc;
+
+ ObjectAddressSet(obj, pg_depend->classid, pg_depend->objid);
+ objdesc = getObjectDescription(&obj, false);
+
+ report_stored_rowtype_const(origRelation, origTypeName, objdesc);
+ }
+ continue;
+ }
+
/* Else, ignore dependees that aren't relations */
if (pg_depend->classid != RelationRelationId)
continue;
@@ -7069,6 +7359,23 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
rel = relation_open(pg_depend->objid, AccessShareLock);
tupleDesc = RelationGetDescr(rel);
+ /*
+ * Index and partition key expressions reach us as whole-relation
+ * dependencies, so look inside them here. A constant of the row type
+ * in one of them is as dangerous as in any other stored expression.
+ */
+ if (relation_expr_has_rowtype_const(rel, typeOid))
+ {
+ ObjectAddress obj;
+ char *objdesc;
+
+ ObjectAddressSet(obj, RelationRelationId, RelationGetRelid(rel));
+ objdesc = getObjectDescription(&obj, false);
+ relation_close(rel, AccessShareLock);
+
+ report_stored_rowtype_const(origRelation, origTypeName, objdesc);
+ }
+
/*
* If objsubid identifies a specific column, refer to that in error
* messages. Otherwise, search to see if there's a user column of the
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index e167a41ce79..d054c95ca62 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2172,6 +2172,50 @@ alter table at_tab1 alter column b type varchar; -- allowed, but ...
insert into at_tab2 values(1,'42'); -- ... this will fail
ERROR: ROW() column has type text instead of type character varying
drop table at_tab1, at_tab2;
+-- A stored *constant* of the row type is rejected: it holds a tuple image
+-- that ALTER COLUMN TYPE cannot rewrite, and reading it afterwards through
+-- the new descriptor is an out-of-bounds read, not a wrong answer.
+create table at_tab1 (a int, b text);
+create table at_tab2 (x int default (('(1,42)'::at_tab1).a));
+alter table at_tab1 alter column b type varchar; -- fails
+ERROR: cannot alter table "at_tab1" because default value for column x of table at_tab2 stores a constant of its row type
+DETAIL: The stored constant holds a row image that the altered type would misinterpret.
+drop table at_tab2;
+create view at_view1 as select '(1,42)'::at_tab1 as x;
+alter table at_tab1 alter column b type varchar; -- fails
+ERROR: cannot alter table "at_tab1" because rule _RETURN on view at_view1 stores a constant of its row type
+DETAIL: The stored constant holds a row image that the altered type would misinterpret.
+drop view at_view1;
+create table at_tab2 (x int check (x = ('(1,42)'::at_tab1).a));
+alter table at_tab1 alter column b type varchar; -- fails
+ERROR: cannot alter table "at_tab1" because constraint at_tab2_x_check on table at_tab2 stores a constant of its row type
+DETAIL: The stored constant holds a row image that the altered type would misinterpret.
+drop table at_tab2;
+create table at_tab2 (x int);
+create index at_tab2_idx on at_tab2 ((x + ('(1,42)'::at_tab1).a));
+alter table at_tab1 alter column b type varchar; -- fails
+ERROR: cannot alter table "at_tab1" because index at_tab2_idx stores a constant of its row type
+DETAIL: The stored constant holds a row image that the altered type would misinterpret.
+drop table at_tab2;
+create function at_func1(arg at_tab1 default '(1,42)'::at_tab1)
+ returns int language sql as 'select 1';
+alter table at_tab1 alter column b type varchar; -- fails
+ERROR: cannot alter table "at_tab1" because function at_func1(at_tab1) stores a constant of its row type
+DETAIL: The stored constant holds a row image that the altered type would misinterpret.
+drop function at_func1;
+-- but merely mentioning the row type, with no constant stored, is fine
+create function at_func1(arg at_tab1) returns int language sql as 'select 1';
+alter table at_tab1 alter column b type varchar; -- allowed
+drop function at_func1;
+drop table at_tab1;
+-- the same for a stand-alone composite type, reported as a type
+create type at_type1 as (a int, b text);
+create view at_view1 as select '(1,42)'::at_type1 as x;
+alter type at_type1 alter attribute b type varchar; -- fails
+ERROR: cannot alter type "at_type1" because rule _RETURN on view at_view1 stores a constant of that type
+DETAIL: The stored constant holds a row image that the altered type would misinterpret.
+drop view at_view1;
+drop type at_type1;
-- Check it for a partitioned table, too
create table at_tab1 (a int, b text) partition by list(a);
create table at_tab2 (x int, y at_tab1);
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 9f6c2a4bb08..45399728b38 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1448,6 +1448,38 @@ create table at_tab2 (x int, y text, check((x,y)::at_tab1 = (1,'42')::at_tab1));
alter table at_tab1 alter column b type varchar; -- allowed, but ...
insert into at_tab2 values(1,'42'); -- ... this will fail
drop table at_tab1, at_tab2;
+-- A stored *constant* of the row type is rejected: it holds a tuple image
+-- that ALTER COLUMN TYPE cannot rewrite, and reading it afterwards through
+-- the new descriptor is an out-of-bounds read, not a wrong answer.
+create table at_tab1 (a int, b text);
+create table at_tab2 (x int default (('(1,42)'::at_tab1).a));
+alter table at_tab1 alter column b type varchar; -- fails
+drop table at_tab2;
+create view at_view1 as select '(1,42)'::at_tab1 as x;
+alter table at_tab1 alter column b type varchar; -- fails
+drop view at_view1;
+create table at_tab2 (x int check (x = ('(1,42)'::at_tab1).a));
+alter table at_tab1 alter column b type varchar; -- fails
+drop table at_tab2;
+create table at_tab2 (x int);
+create index at_tab2_idx on at_tab2 ((x + ('(1,42)'::at_tab1).a));
+alter table at_tab1 alter column b type varchar; -- fails
+drop table at_tab2;
+create function at_func1(arg at_tab1 default '(1,42)'::at_tab1)
+ returns int language sql as 'select 1';
+alter table at_tab1 alter column b type varchar; -- fails
+drop function at_func1;
+-- but merely mentioning the row type, with no constant stored, is fine
+create function at_func1(arg at_tab1) returns int language sql as 'select 1';
+alter table at_tab1 alter column b type varchar; -- allowed
+drop function at_func1;
+drop table at_tab1;
+-- the same for a stand-alone composite type, reported as a type
+create type at_type1 as (a int, b text);
+create view at_view1 as select '(1,42)'::at_type1 as x;
+alter type at_type1 alter attribute b type varchar; -- fails
+drop view at_view1;
+drop type at_type1;
-- Check it for a partitioned table, too
create table at_tab1 (a int, b text) partition by list(a);
create table at_tab2 (x int, y at_tab1);
--
2.55.0
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Detaching a child table makes an expression using it unrestorable
@ 2026-09-21 05:48 Laurenz Albe <laurenz.albe@cybertec.at>
parent: Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
0 siblings, 1 reply; 7+ messages in thread
From: Laurenz Albe @ 2026-09-21 05:48 UTC (permalink / raw)
To: Manuel Reyes Bravo <manuelreyesbravo@gmail.com>; Andrey Rachitskiy <pl0h0yp1@gmail.com>; +Cc: Jinqing Kuang <kuangjinqingcn@gmail.com>; Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, 2026-09-21 at 01:39 -0300, Manuel Reyes Bravo wrote:
> Attached is v2 of the patch that refuses ALTER COLUMN TYPE when a
> stored expression holds a constant of the row type.
>
> It fixes the wording for stand-alone composite types. v1 reported
>
> ALTER TYPE t ALTER ATTRIBUTE b TYPE varchar;
> ERROR: cannot alter table "t" because rule _RETURN on view v
> stores a constant of its row type
>
> where the existing column checks in find_composite_type_dependencies()
> correctly say "cannot alter type". Both of the new reporting sites now
> go through one helper that follows those checks (type, foreign table or
> table), and the index case names the object the same way as the others
> ("index at_tab2_idx" instead of 'an expression of "at_tab2_idx"').
> A regression test for the composite type case is added; it fails with
> v1. make check passes.
>
> I found it while reviewing Nikhil's patch in "Dropping a composite
> attribute causes data integrity violations" [1], which makes ALTER TYPE
> ... DROP ATTRIBUTE and ALTER TABLE ... DROP COLUMN call
> find_composite_type_dependencies() as well. The two patches compose:
> with both, a drop is also refused when a stored constant of the row
> type would change its meaning. The details are in that thread.
>
> [1] https://postgr.es/m/CA+UBoq0F2ua2fQEAU3-6w0oxOgFc9BgEN4gRcchiUUZ+WouVrQ@mail.gmail.com
Thanks.
I briefly looked at the patch, and I wondered if you omitted
pg_partitioned_table.partexprs on purpose.
But taking a step back, I wonder if all that is worth the effort.
PostgreSQL expects data types to be immutable, so if you modify a
composite type that is used anywhere in a persistent fashion, things
will break all over the place. I doubt that we can ever plug all the
holes.
I think it is a bad idea to use composite types in table definitions
or anything else that is persistent, but we can hardly go back on
that. Perhaps we should just forbid ALTER TYPE ... ADD/DROP/ALTER?
That would be an incompatibility, but it wouldn't break upgrade.
Yours,
Laurenz Albe
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Detaching a child table makes an expression using it unrestorable
@ 2026-09-21 16:17 Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
parent: Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 0 replies; 7+ messages in thread
From: Manuel Reyes Bravo @ 2026-09-21 16:17 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: Andrey Rachitskiy <pl0h0yp1@gmail.com>; Jinqing Kuang <kuangjinqingcn@gmail.com>; Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Hi Laurenz,
Laurenz Albe <laurenz.albe@cybertec.at> wrote:
> I briefly looked at the patch, and I wondered if you omitted
> pg_partitioned_table.partexprs on purpose.
It is not omitted, but it is easy to miss. Partition key expressions,
like index expressions, are recorded in pg_depend as a dependency of
the whole relation rather than of a column, so they are read in the
branch for whole-relation dependencies (the heap_getattr on
Anum_pg_partitioned_table_partexprs), not in the loop over catalogs
with their own expression column. With the patch:
create type pk as (a int, b int);
create table tp (i int)
partition by list ((('(1,2)'::pk = '(1,3)'::pk) = (i > 0)));
alter type pk alter attribute b type bigint;
ERROR: cannot alter type "pk" because table tp stores a constant
of that type
There is no regression test for that case, though. I will add one in
the next version.
> But taking a step back, I wonder if all that is worth the effort.
> PostgreSQL expects data types to be immutable, so if you modify a
> composite type that is used anywhere in a persistent fashion, things
> will break all over the place. I doubt that we can ever plug all the
> holes.
The partition key case is a good example of how far the damage goes.
On master, with two partitions for true and false:
create table tp_t partition of tp for values in (true);
create table tp_f partition of tp for values in (false);
insert into tp values (5); -- goes to tp_f
alter type pk drop attribute b; -- accepted
insert into tp values (5); -- same session: tp_f
-- new session
insert into tp values (5); -- tp_t
The stored key is now LIST (((('(1)'::pk = '(1)'::pk) = (i > 0)))), so
the same row is routed to different partitions depending on the
session: at the end tp_f holds two rows and tp_t one, all with i = 5.
As for forbidding ALTER TYPE ... ADD/DROP/ALTER ATTRIBUTE outright: that
would also refuse the cases that are safe today, such as a type used
only as a function result or parameter. Looking only for stored values
and stored constants keeps those working, and the set of places
that can store an expression is small and fixed, which is why I think
the holes can be closed. But whether the extra code is worth it
compared to a documented incompatibility is not my call, and I'd be
glad to hear what others think.
Regards,
Manu
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-09-21 16:17 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-09-10 14:12 Detaching a child table makes an expression using it unrestorable Heikki Linnakangas <hlinnaka@iki.fi>
2026-09-15 01:53 ` Jinqing Kuang <kuangjinqingcn@gmail.com>
2026-09-15 04:26 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-09-16 07:14 ` Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
2026-09-21 04:39 ` Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
2026-09-21 05:48 ` Laurenz Albe <laurenz.albe@cybertec.at>
2026-09-21 16:17 ` Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox