Re: pg_dump emits ALTER TABLE ONLY partitioned_table

On 2017/04/11 0:26, Robert Haas wrote:
> On Sun, Apr 9, 2017 at 10:10 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> While I admit that I've not been paying close attention to the whole
>> table partitioning business, I wonder whether we have any clearly written
>> down specification about (a) how much partition member tables are allowed
>> to deviate schema-wise from their parent, and (b) how DDL semantics on
>> partitioned tables differ from DDL semantics for traditional inheritance.
>> Obviously those are closely related questions. But the fact that this
>> bug exists at all shows that there's been some lack of clarity on (b),
>> and so I wonder whether we have any clarity on (a) either.
>
> Children can have constraints (including NOT NULL constraints) which
> parents lack, and can have a different column order, but must have
> exactly the same column names and types.

Also, what is different in the partitioned parent case is that NOT NULL
constraints must be inherited. That is, one cannot add it only to the parent.

--
-- traditional inheritance
--
create table parent (a int);
create table child () inherits (parent);
alter table only parent add constraint chka check (a > 0);
-- ERROR: constraint must be added to child tables too

-- the following is OK, because of the explicit NO INHERIT request
alter table only parent add constraint chka check (a > 0) no inherit;

-- the following is also OK, because NOT NULL constraints are not
-- inherited with regular inheritance
alter table only parent alter a set not null;

--
-- partitioning inheritance
--
create table parted_parent (a int) partition by list (a);
create table part partition of parted_parent for values in (1);

-- this is same as traditional inheritance
alter table only parted_parent add constraint chka check (a > 0);
-- ERROR: constraint must be added to child tables too

-- requesting NO INHERIT doesn't make sense on what is an empty relation
-- by itself
alter table only parted_parent add constraint chka check (a > 0) no inherit;
-- ERROR: cannot add NO INHERIT constraint to partitioned table
"parted_table"

-- NOT NULL constraint must be inherited too
alter table only parted_parent alter a set not null;
-- ERROR: constraint must be added to child tables too

-- both ok (no inheritance here)
alter table part alter a set not null;
alter table part alter a drop not null;

-- request whole-tree constraint
alter table parted_parent alter a set not null;
-- can't drop it from a partition
alter table part alter a drop not null;
-- ERROR: column "a" is marked NOT NULL in parent table

> In Amit's example from the original post, the child has an implicit
> NOT NULL constraint that does not exist in the parent. p1.b isn't
> declared NOT NULL, but the fact that it is range-partitioned on b
> requires it to be so, just as we would do if b were declared as the
> PRIMARY KEY. Somehow that's not playing nice with pg_dump, but I'm
> still fuzzy on the details.

Actually, I would like to change the problem definition from "ALTER TABLE
ONLY partitioned_table should be avoided" to "Emitting partition's
attributes separately should be avoided".

There is a shouldPrintColumn() which returns true if a table's attribute
should be emitted in the CREATE TABLE command at all. For example, it
won't be necessary if the attribute is purely inherited
(attislocal=false). But once an attribute is determined to not be
emitted, any attribute-level constraints and defaults need to be marked to
be emitted separately (using ALTER TABLE ONLY), which can be undesirable
because of the rules about using ONLY with declarative partitioning
inheritance.

I think we can change shouldPrintColumn() so that it always returns true
if the table is a partition, that is, a partition's attributes should
always be emitted with CREATE TABLE, if necessary (it isn't required if
there isn't a NOT NULL constraint or DEFAULT defined on the attributes).
When dumping a partition using the PARTITION OF syntax, an attribute will
emitted if it has a NOT NULL constraint or a default.

So if we have:

create table p (a int, b int) partition by list (a);
create table p1 partition of p for values in (1) partition by range (b);
create table p11 partition of p1 (
a not null default '1'
) for values from (1) to (10);

Proposed would make pg_dump -s output the following (some lines removed
for clarity):

CREATE TABLE p (
a integer,
b integer
)
PARTITION BY LIST (a);

CREATE TABLE p1 PARTITION OF p (
b NOT NULL
)
FOR VALUES IN (1)
PARTITION BY RANGE (b);

CREATE TABLE p11 PARTITION OF p1 (
a DEFAULT 1 NOT NULL,
b NOT NULL
)
FOR VALUES FROM (1) TO (10);

Which also happens to a legal input to the backend, as far as partitioning
is concerned. With the existing approach, NOT NULL on p1's b attribute is
emitted as ALTER TABLE ONLY p1 ALTER b SET NOT NULL, which would cause
error, because p1 is partitioned.

Attached patch implements that and also updates some comments. I think
some updates to pg_dump's regression tests for partitioned tables will be
required, but my attempt to understand how to go about updating the
expected output failed. I will try again tomorrow.

Thanks,
Amit