Re: Detaching a child table makes an expression using it unrestorable

Hi Laurenz,

Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)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