Re: doc fail about ALTER TABLE ATTACH re. NO INHERIT

On 2024-Nov-06, Amit Langote wrote:

> On Tue, Nov 5, 2024 at 9:01 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:

> > While doing final review for not-null constraints, I noticed that the
> > ALTER TABLE ATTACH PARTITION have this phrase:
> >
> > If any of the CHECK constraints of the table being attached are marked NO
> > INHERIT, the command will fail; such constraints must be recreated without the
> > NO INHERIT clause.

> I think that text might be talking about this case:
>
> create table parent (a int, constraint check_a check (a > 0))
> partition by list (a);
> create table part1 (a int, constraint check_a check (a > 0) no inherit);
> alter table parent attach partition part1 for values in (1);
> ERROR: constraint "check_a" conflicts with non-inherited constraint on child table "part1"

Oh, hmm, that makes sense I guess. Still, while this restriction makes
sense for inheritance, it doesn't IMO for partitioned tables. I would
even suggest that we drop enforcement of this restriction during ATTACH.

> Perhaps the ATTACH PARTITION text should be changed to make clear
> which case it's talking about, say, like:
>
> If any of the CHECK constraints of the table being attached are marked
> NO INHERIT, the command will fail if a constraint with the same name
> exists in the parent table; such constraints must be recreated without
> the NO INHERIT clause.

Hmm, not sure about it; I think we're giving too much prominence to a
detail that's not so important that it warrants four extra lines, when
it could be a parenthical note next to the other mention of those
constraints earlier in that paragraph. I suggest something like this:

<para>
A partition using <literal>FOR VALUES</literal> uses same syntax for
<replaceable class="parameter">partition_bound_spec</replaceable> as
<link linkend="sql-createtable"><command>CREATE TABLE</command></link>. The partition bound specification
must correspond to the partitioning strategy and partition key of the
target table. The table to be attached must have all the same columns
as the target table and no more; moreover, the column types must also
match. Also, it must have all the <literal>NOT NULL</literal> and
<literal>CHECK</literal> constraints of the target table. Currently
<literal>FOREIGN KEY</literal> constraints are not considered.
<literal>UNIQUE</literal> and <literal>PRIMARY KEY</literal> constraints
from the parent table will be created in the partition, if they don't
already exist.
If any of the <literal>CHECK</literal> constraints of the table being
attached are marked <literal>NO INHERIT</literal>, the command will fail;
such constraints must be recreated without the
<literal>NO INHERIT</literal> clause.
</para>

I suggest we change it to

<para>
A partition using <literal>FOR VALUES</literal> uses same syntax for
<replaceable class="parameter">partition_bound_spec</replaceable> as
<link linkend="sql-createtable"><command>CREATE TABLE</command></link>. The partition bound specification
must correspond to the partitioning strategy and partition key of the
target table. The table to be attached must have all the same columns
as the target table and no more; moreover, the column types must also
match. Also, it must have all the <literal>NOT NULL</literal> and
<literal>CHECK</literal> constraints of the target table, not marked
<literal>NO INHERIT</literal>. Currently
<literal>FOREIGN KEY</literal> constraints are not considered.
<literal>UNIQUE</literal> and <literal>PRIMARY KEY</literal> constraints
from the parent table will be created in the partition, if they don't
already exist.
</para>

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"You don't solve a bad join with SELECT DISTINCT" #CupsOfFail
https://twitter.com/connor_mc_d/status/1431240081726115845