Re: Detaching a child table makes an expression using it unrestorable - Mailing list pgsql-bugs

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
deparsedas 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
thetypcache 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
varlenaread, 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
thatis the data bytes 02 00 00 40. After a becomes text, those four bytes are read as a varlena header. The first byte
is0x02, which on LE is a 4-byte compressed header (VARATT_IS_4B_C), not an uncompressed one. align_fetch_then_add()
stilladvances with VARSIZE_ANY() → VARSIZE_4B(): va_header >> 2. That is exactly 0x10000000 (256MiB). The next
attributeis 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
isenough, 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
onlyembeds 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.
ALTERchanges the rowtype descriptor and leaves the old images in place. 
>
> --
> Regards,
> Rachitskiy Andrey



--
Saludos cordiales,

Manuel Reyes