Re: ATTACH PARTITION seems to ignore column generation status - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: ATTACH PARTITION seems to ignore column generation status |
Date | |
Msg-id | 378312.1673388786@sss.pgh.pa.us Whole thread Raw |
In response to | Re: ATTACH PARTITION seems to ignore column generation status (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: ATTACH PARTITION seems to ignore column generation status
|
List | pgsql-hackers |
I wrote: > Amit Langote <amitlangote09@gmail.com> writes: >> Thanks for the patch. It looks good, though I guess you said that we >> should also change the error message that CREATE TABLE ... PARTITION >> OF emits to match the other cases while we're here. I've attached a >> delta patch. > Thanks. I hadn't touched that issue because I wasn't entirely sure > which error message(s) you were unhappy with. These changes look > fine offhand. So, after playing with that a bit ... removing the block in parse_utilcmd.c allows you to do regression=# CREATE TABLE gtest_parent (f1 date NOT NULL, f2 bigint, f3 bigint GENERATED ALWAYS AS (f2 * 2) STORED) PARTITIONBY RANGE (f1); CREATE TABLE regression=# CREATE TABLE gtest_child PARTITION OF gtest_parent ( regression(# f3 WITH OPTIONS GENERATED ALWAYS AS (f2 * 3) STORED regression(# ) FOR VALUES FROM ('2016-07-01') TO ('2016-08-01'); CREATE TABLE regression=# \d gtest_child Table "public.gtest_child" Column | Type | Collation | Nullable | Default --------+--------+-----------+----------+------------------------------------- f1 | date | | not null | f2 | bigint | | | f3 | bigint | | | generated always as (f2 * 3) stored Partition of: gtest_parent FOR VALUES FROM ('2016-07-01') TO ('2016-08-01') regression=# insert into gtest_parent values('2016-07-01', 42); INSERT 0 1 regression=# table gtest_parent; f1 | f2 | f3 ------------+----+----- 2016-07-01 | 42 | 126 (1 row) That is, you can make a partition with a different generated expression than the parent has. Do we really want to allow that? I think it works as far as the backend is concerned, but it breaks pg_dump, which tries to dump this state of affairs as CREATE TABLE public.gtest_parent ( f1 date NOT NULL, f2 bigint, f3 bigint GENERATED ALWAYS AS ((f2 * 2)) STORED ) PARTITION BY RANGE (f1); CREATE TABLE public.gtest_child ( f1 date NOT NULL, f2 bigint, f3 bigint GENERATED ALWAYS AS ((f2 * 3)) STORED ); ALTER TABLE ONLY public.gtest_parent ATTACH PARTITION public.gtest_child FOR VALUES FROM ('2016-07-01') TO ('2016-08-01'); and that fails at reload because the ATTACH PARTITION code path checks for equivalence of the generation expressions. This different-generated-expression situation isn't really morally different from different ordinary DEFAULT expressions, which we do endeavor to support. So maybe we should deem this a supported case and remove ATTACH PARTITION's insistence that the generation expressions match ... which I think would be a good thing anyway, because that check-for-same-reverse-compiled-expression business is pretty cheesy in itself. AFAIK, 3f7836ff6 took care of the only problem that the backend would have with this, and pg_dump looks like it will work as long as the backend will take the ATTACH command. I also looked into making CREATE TABLE ... PARTITION OF reject this case; but that's much harder than it sounds, because what we have at the relevant point is a raw (unanalyzed) expression for the child's generation expression but a cooked one for the parent's, so there is no easy way to match the two. In short, it's seeming like the rule for both partitioning and traditional inheritance ought to be "a child column must have the same generated-or-not property as its parent, but their generation expressions need not be the same". Thoughts? regards, tom lane
pgsql-hackers by date: