Обсуждение: docs about FKs referencing partitioned tables

Hello,

I posted this to the "clean up docs for v12" thread and it was
suggested I make a new thread instead, so here it is. Sorry for the
extra noise! :-)

I noticed the docs at
https://www.postgresql.org/docs/devel/ddl-partitioning.html still say
you can't create a foreign key referencing a partitioned table, even
though the docs for
https://www.postgresql.org/docs/devel/sql-createtable.html have been
updated (compared to v11). My understanding is that foreign keys
*still* don't work as expected when pointing at traditional INHERITS
tables, but they *will* work with declaratively-partitioned tables. In
that case I suggest this change:

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index a0a7435a03..3b4f43bbad 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -3966,14 +3966,6 @@ ALTER TABLE measurement ATTACH PARTITION
measurement_y2008m02

-      <listitem>
-       <para>
-       While primary keys are supported on partitioned tables, foreign
-       keys referencing partitioned tables are not supported.  (Foreign key
-       references from a partitioned table to some other table are supported.)
-      </para>
-     </listitem>
-
     <listitem>
      <para>
        <literal>BEFORE ROW</literal> triggers, if necessary, must be defined
        on individual partitions, not the partitioned table.
       </para>
@@ -4366,6 +4358,14 @@ ALTER TABLE measurement_y2008m02 INHERIT measurement;
        </para>
       </listitem>

+     <listitem>
+      <para>
+       While primary keys are supported on inheritance-partitioned
tables, foreign
+       keys referencing these tables are not supported.  (Foreign key
+       references from an inheritance-partitioned table to some other
table are supported.)
+      </para>
+     </listitem>
+
       <listitem>
        <para>
         If you are using manual <command>VACUUM</command> or

(I've also attached it as a patch file.) In other words, we should
move this caveat from the section on declaratively-partitioned tables
to the section on inheritance-partitioned tables.

Yours,
Paul

On 2019/05/21 13:42, Paul A Jungwirth wrote:
> I noticed the docs at
> https://www.postgresql.org/docs/devel/ddl-partitioning.html still say
> you can't create a foreign key referencing a partitioned table, even
> though the docs for
> https://www.postgresql.org/docs/devel/sql-createtable.html have been
> updated (compared to v11). My understanding is that foreign keys
> *still* don't work as expected when pointing at traditional INHERITS
> tables, but they *will* work with declaratively-partitioned tables.

Thanks Paul.

Would you like me to edit the wiki to add this to open items?

Regards,
Amit

On Mon, May 20, 2019 at 9:59 PM Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> Would you like me to edit the wiki to add this to open items?

That would be helpful for sure. Thanks!

Paul

On 2019/05/21 14:05, Paul A Jungwirth wrote:
> On Mon, May 20, 2019 at 9:59 PM Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> Would you like me to edit the wiki to add this to open items?
> 
> That would be helpful for sure. Thanks!

OK, done.

Thanks,
Amit

On Mon, May 20, 2019 at 09:42:52PM -0700, Paul A Jungwirth wrote:
> I noticed the docs at
> https://www.postgresql.org/docs/devel/ddl-partitioning.html still say
> you can't create a foreign key referencing a partitioned table, even
> though the docs for
> https://www.postgresql.org/docs/devel/sql-createtable.html have been
> updated (compared to v11). My understanding is that foreign keys
> *still* don't work as expected when pointing at traditional INHERITS
> tables, but they *will* work with declaratively-partitioned tables. In
> that case I suggest this change:

Could you define what is an "inheritance-partitioned" table?  I know
of partitioned tables, inherited tables and tables which make use
of inheritance for partitioning (hence Inheritance Partitioning), but
the paragraph you are adding introduces a new term in the whole tree.
This makes things confusing and the previous paragraph is not, even if
it is incorrect since f56f8f8 as Amit has noted.

A simple rewording could be "tables using inheritance for
partitioning".

> (I've also attached it as a patch file.) In other words, we should
> move this caveat from the section on declaratively-partitioned tables
> to the section on inheritance-partitioned tables.

As this restriction only applies only to tables partitioned with
inheritance, then this move should be fine.
--
Michael

On Mon, May 20, 2019 at 10:18 PM Michael Paquier <michael@paquier.xyz> wrote:
> Could you define what is an "inheritance-partitioned" table?  I know
> of partitioned tables, inherited tables and tables which make use
> of inheritance for partitioning (hence Inheritance Partitioning), but
> the paragraph you are adding introduces a new term in the whole tree.
> This makes things confusing and the previous paragraph is not, even if
> it is incorrect since f56f8f8 as Amit has noted.
>
> A simple rewording could be "tables using inheritance for
> partitioning".

I agree that sounds better. To avoid repeating it I changed the second
instance to just "inherited tables". New patch attached.

Paul

On Mon, May 20, 2019 at 10:35:31PM -0700, Paul A Jungwirth wrote:
> I agree that sounds better. To avoid repeating it I changed the second
> instance to just "inherited tables". New patch attached.

Looking closer, you are adding that:
+     <listitem>
+      <para>
+       While primary keys are supported on tables using inheritance
+       for partitioning, foreign keys referencing these tables are not
+       supported.  (Foreign key references from an inherited table to
+       some other table are supported.)
+      </para>
+     </listitem>

However that's just fine:
=# create table aa (a int primary key);
CREATE TABLE
=# create table aa_child (a int primary key, inherits aa, foreign key
(a) references aa);
CREATE TABLE
=# create table aa_grandchild (a int primary key, inherits aa_child,
foreign key (a) references aa_child);
CREATE TABLE

The paragraph you are removing from 5.11.2.3 (limitations of
declarative partitioning) only applies to partitioned tables, not to
plain tables.  And there is no such thing for paritioning based on
inheritance, so we should just remove one paragraph, and not add the
extra one, no?
--
Michael

On Wed, May 22, 2019 at 8:06 PM Michael Paquier <michael@paquier.xyz> wrote:
> Looking closer, you are adding that:
> +     <listitem>
> +      <para>
> +       While primary keys are supported on tables using inheritance
> +       for partitioning, foreign keys referencing these tables are not
> +       supported.  (Foreign key references from an inherited table to
> +       some other table are supported.)
> +      </para>
> +     </listitem>
>
> However that's just fine:
> =# create table aa (a int primary key);
> CREATE TABLE
> =# create table aa_child (a int primary key, inherits aa, foreign key
> (a) references aa);
> CREATE TABLE
> =# create table aa_grandchild (a int primary key, inherits aa_child,
> foreign key (a) references aa_child);
> CREATE TABLE

Postgres will let you define the FK, but it doesn't work in a meaningful way:

paul=# create table t1 (id int primary key, foo text);
CREATE TABLE
paul=# create table t2 (bar text) inherits (t1);
CREATE TABLE
paul=# insert into t2 values (1, 'f', 'b');
INSERT 0 1
paul=# select * from t1;
 id | foo
----+-----
  1 | f
(1 row)

paul=# create table ch (id int, t_id int references t1 (id));
CREATE TABLE
paul=# insert into ch values (1, 1);
ERROR:  insert or update on table "ch" violates foreign key constraint
"ch_t_id_fkey"
DETAIL:  Key (t_id)=(1) is not present in table "t1".

The section in the docs (5.10) just before the one I changed has
similar warnings:

> Other types of constraints (unique, primary key, and foreign key constraints) are not inherited.

and

> A serious limitation of the inheritance feature is that indexes (including unique constraints) and foreign key
constraintsonly apply to single tables, not to their inheritance children.
 

> The paragraph you are removing from 5.11.2.3 (limitations of
> declarative partitioning) only applies to partitioned tables, not to
> plain tables.  And there is no such thing for paritioning based on
> inheritance, so we should just remove one paragraph, and not add the
> extra one, no?

I moved the paragraph to a section describing inheritance as an
alternative partitioning solution to declarative partitions. Since
using inheritance to partition a table requires giving up foreign
keys, it seems worthwhile to include that among the other caveats. (It
wasn't necessary to include it before because declarative partitions
had the same drawback, and it was already expressed in the paragraph I
took out.) In my opinion mentioning this limitation would be helpful
to people.

Perhaps the wording is too strong though:

> +       . . . foreign keys referencing these tables are not
> +       supported. . . .

I was trying to make a minimal change by keeping most of the original
wording, but I agree that different language would be more accurate.
What do you think of something like this?:

+     <listitem>
+      <para>
+       While foreign keys may be defined that reference a parent
+       table, they will not see records from its child tables. Since
+       the parent table is typically empty, adding any record (with a
+       non-null foreign key) to the referencing table will raise an error.
+      </para>
+     </listitem>

Paul

On Thu, May 23, 2019 at 12:02:56AM -0700, Paul A Jungwirth wrote:
> The section in the docs (5.10) just before the one I changed has
> similar warnings:
>
>> Other types of constraints (unique, primary key, and foreign key
>> constraints) are not inherited.
>
> and
>
>> A serious limitation of the inheritance feature is that indexes
>> (including unique constraints) and foreign key constraints only
>> apply to single tables, not to their inheritance children.

Yes.

> I moved the paragraph to a section describing inheritance as an
> alternative partitioning solution to declarative partitions. Since
> using inheritance to partition a table requires giving up foreign
> keys, it seems worthwhile to include that among the other caveats. (It
> wasn't necessary to include it before because declarative partitions
> had the same drawback, and it was already expressed in the paragraph I
> took out.) In my opinion mentioning this limitation would be helpful
> to people.

Well, the point I would like to outline is that section 5.11.2 about
declarative partitioning and 5.11.3 about partitioning with
inheritance treat about two separate, independent partitioning
methods.  So removing the paragraph from the declarative partitioning
section mentioning foreign keys referencing partitioned tables is
fine, because that's not the case anymore...

> I was trying to make a minimal change by keeping most of the original
> wording, but I agree that different language would be more accurate.
> What do you think of something like this?:
>
> +     <listitem>
> +      <para>
> +       While foreign keys may be defined that reference a parent
> +       table, they will not see records from its child tables. Since
> +       the parent table is typically empty, adding any record (with a
> +       non-null foreign key) to the referencing table will raise an error.
> +      </para>
> +     </listitem>

...  However you are adding a paragraph for something which is
completely unrelated to the issue we are trying to fix.  If I were to
add something, I think that I would be more general than what you are
trying here and just mention a link to the previous paragraph about
the caveats of inheritance as they apply to single table members of an
inheritance tree and not a full set:
"Indexes and foreign key constraint apply to single tables and not
their inheritance children, hence they have some <link>caveats</> to
be aware of."
Still this is a duplicate of a sentence which is just a couple of
paragraphs back.
--
Michael

On Sun, May 26, 2019 at 7:49 PM Michael Paquier <michael@paquier.xyz> wrote:
> Well, the point I would like to outline is that section 5.11.2 about
> declarative partitioning and 5.11.3 about partitioning with
> inheritance treat about two separate, independent partitioning
> methods.  So removing the paragraph from the declarative partitioning
> section mentioning foreign keys referencing partitioned tables is
> fine, because that's not the case anymore...
> [snip]
> ...  However you are adding a paragraph for something which is
> completely unrelated to the issue we are trying to fix.  If I were to
> add something, I think that I would be more general than what you are
> trying here and just mention a link to the previous paragraph about
> the caveats of inheritance as they apply to single table members of an
> inheritance tree and not a full set:
> "Indexes and foreign key constraint apply to single tables and not
> their inheritance children, hence they have some <link>caveats</> to
> be aware of."

That seems reasonable to me. Here is a patch file if that is helpful
(minor typo corrected).

Yours,
Paul

Hi,

On 2019/05/29 14:01, Paul A Jungwirth wrote:
> On Sun, May 26, 2019 at 7:49 PM Michael Paquier <michael@paquier.xyz> wrote:
>> ...  However you are adding a paragraph for something which is
>> completely unrelated to the issue we are trying to fix.  If I were to
>> add something, I think that I would be more general than what you are
>> trying here and just mention a link to the previous paragraph about
>> the caveats of inheritance as they apply to single table members of an
>> inheritance tree and not a full set:
>> "Indexes and foreign key constraint apply to single tables and not
>> their inheritance children, hence they have some <link>caveats</> to
>> be aware of."
> 
> That seems reasonable to me. Here is a patch file if that is helpful
> (minor typo corrected).

The patch looks good, thanks.

Michael commented upthread that the new next might be repeating what's
already said elsewhere.  I did find that to be the case by looking at
5.10. Inheritance.  For example, 5.10.1. Caveats says exactly the same thing:

   A serious limitation of the inheritance feature is that indexes
   (including unique constraints) and foreign key constraints only apply
   to single tables, not to their inheritance children. This is true on
   both the referencing and referenced sides of a foreign key constraint.

But couple of other points mentioned in 5.11.3.3. Caveats (of 5.11. Table
Partitioning) are already repeated in 5.10.1. Caveats; for example, note
the point about VACUUM, ANALYZE, INSERT ON CONFLICT, etc. applying to
single tables.  So, perhaps it won't hurt to repeat the caveat about
indexes and foreign keys too.

Thanks,
Amit

On Wed, May 29, 2019 at 02:33:26PM +0900, Amit Langote wrote:
> But couple of other points mentioned in 5.11.3.3. Caveats (of 5.11. Table
> Partitioning) are already repeated in 5.10.1. Caveats; for example, note
> the point about VACUUM, ANALYZE, INSERT ON CONFLICT, etc. applying to
> single tables.  So, perhaps it won't hurt to repeat the caveat about
> indexes and foreign keys too.

OK, committed as such.  Your patch linked to the top of the
inheritance section, so I redirected that to the actual section about
caveats for clarity.
--
Michael