On Wed, Nov 6, 2024 at 9:34 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote: > On 2024-Nov-06, Amit Langote wrote: > > On Tue, Nov 5, 2024 at 9:01 PM Alvaro Herrera <alvhe...@alvh.no-ip.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.
I agree. Since leaf partitions have no children to propagate constraints to, the NO INHERIT mark shouldn't matter. And partitioned partitions already disallow NO INHERIT constraints as you mentioned. Do you think we should apply something like the attached at least in the master? I found that a similar restriction exists in the CREATE TABLE PARTITION OF path too. > > 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> +1 Though if we decide to apply the attached, does the note "not marked NO INHERIT" become unnecessary? -- Thanks, Amit Langote
v1-0001-Allow-inherited-NO-INHERIT-check-constraints-in-l.patch
Description: Binary data
