Disallow COPY FREEZE on partitioned tables

This didn't actually work: COPY would fail to flush the right files, and
instead would try to flush a non-existing file, causing the whole
transaction to fail.

Cope by raising an error as soon as the command is sent instead, to
avoid a nasty later surprise.  Of course, it would be much better to
make it work, but we don't have a patch for that yet, and we don't know
if we'll want to backpatch one when we do.

Reported-by: Tomas Vondra
Author: David Rowley
Reviewed-by: Amit Langote, Steve Singer, Tomas Vondra

diff --git a/doc/src/sgml/perform.sgml b/doc/src/sgml/perform.sgml
index e7f17b9fa9dbdd057276e40fe68ded185454a994..e1e65c057e007f2b9c536827e2795b5c4ada6ac1 100644 (file)
--- a/doc/src/sgml/perform.sgml
+++ b/doc/src/sgml/perform.sgml
@@ -1523,8 +1523,8 @@ SELECT * FROM x, y, a, b, c WHERE something AND somethingelse;
     needs to be written, because in case of an error, the files
     containing the newly loaded data will be removed anyway.
     However, this consideration only applies when
-    <xref linkend="guc-wal-level"> is <literal>minimal</> as all commands
-    must write WAL otherwise.
+    <xref linkend="guc-wal-level"> is <literal>minimal</> for
+    non-partitioned tables as all commands must write WAL otherwise.
    </para>
 
   </sect2>

diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 732efe69e662e047c94c2fe576b6d340c29ea2e6..3054123b7faadede36926b6853ba7d78df612c59 100644 (file)
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -224,7 +224,9 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
       This is intended as a performance option for initial data loading.
       Rows will be frozen only if the table being loaded has been created
       or truncated in the current subtransaction, there are no cursors
-      open and there are no older snapshots held by this transaction.
+      open and there are no older snapshots held by this transaction.  It is
+      currently not possible to perform a <command>COPY FREEZE</command> on
+      a partitioned table.
      </para>
      <para>
       Note that all other sessions will immediately be able to see the data

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 05e05b8fb5847dfb4a09b11521021204b34d375f..d8aa0c92164936d29848d6e7bc2f561ab23e6276 100644 (file)
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2382,11 +2382,20 @@ CopyFrom(CopyState cstate)
     * go into pages containing tuples from any other transactions --- but this
     * must be the case if we have a new table or new relfilenode, so we need
     * no additional work to enforce that.
+    *
+    * We currently don't support this optimization if the COPY target is a
+    * partitioned table as we currently only lazily initialize partition
+    * information when routing the first tuple to the partition.  We cannot
+    * know at this stage if we can perform this optimization.  It should be
+    * possible to improve on this, but it does mean maintaining heap insert
+    * option flags per partition and setting them when we first open the
+    * partition.
     *----------
     */
    /* createSubid is creation check, newRelfilenodeSubid is truncation check */
-   if (cstate->rel->rd_createSubid != InvalidSubTransactionId ||
-       cstate->rel->rd_newRelfilenodeSubid != InvalidSubTransactionId)
+   if (cstate->rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE &&
+       (cstate->rel->rd_createSubid != InvalidSubTransactionId ||
+        cstate->rel->rd_newRelfilenodeSubid != InvalidSubTransactionId))
    {
        hi_options |= HEAP_INSERT_SKIP_FSM;
        if (!XLogIsNeeded())
@@ -2406,6 +2415,22 @@ CopyFrom(CopyState cstate)
     */
    if (cstate->freeze)
    {
+       /*
+        * We currently disallow COPY FREEZE on partitioned tables.  The
+        * reason for this is that we've simply not yet opened the partitions
+        * to determine if the optimization can be applied to them.  We could
+        * go and open them all here, but doing so may be quite a costly
+        * overhead for small copies.  In any case, we may just end up routing
+        * tuples to a small number of partitions.  It seems better just to
+        * raise an ERROR for partitioned tables.
+        */
+       if (cstate->rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+       {
+           ereport(ERROR,
+               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                   errmsg("cannot perform FREEZE on a partitioned table")));
+       }
+
        /*
         * Tolerate one registration for the benefit of FirstXactSnapshot.
         * Scan-bearing queries generally create at least two registrations,

diff --git a/src/test/regress/input/copy.source b/src/test/regress/input/copy.source
index cb13606d1416b3c3764f904ff1c3be318caaecb1..ce285a8f31302d3c08e08ce5d8ed6bc4b6f332b9 100644 (file)
--- a/src/test/regress/input/copy.source
+++ b/src/test/regress/input/copy.source
@@ -133,3 +133,32 @@ this is just a line full of junk that would error out if parsed
 \.
 
 copy copytest3 to stdout csv header;
+
+-- test copy from with a partitioned table
+create table parted_copytest (
+   a int,
+   b int,
+   c text
+) partition by list (b);
+
+create table parted_copytest_a1 (c text, b int, a int);
+create table parted_copytest_a2 (a int, c text, b int);
+
+alter table parted_copytest attach partition parted_copytest_a1 for values in(1);
+alter table parted_copytest attach partition parted_copytest_a2 for values in(2);
+
+-- We must insert enough rows to trigger multi-inserts.  These are only
+-- enabled adaptively when there are few enough partition changes.
+insert into parted_copytest select x,1,'One' from generate_series(1,1000) x;
+insert into parted_copytest select x,2,'Two' from generate_series(1001,1010) x;
+insert into parted_copytest select x,1,'One' from generate_series(1011,1020) x;
+
+copy (select * from parted_copytest order by a) to '@abs_builddir@/results/parted_copytest.csv';
+
+-- Ensure COPY FREEZE errors for partitioned tables.
+begin;
+truncate parted_copytest;
+copy parted_copytest from '@abs_builddir@/results/parted_copytest.csv' (freeze);
+rollback;
+
+drop table parted_copytest;

diff --git a/src/test/regress/output/copy.source b/src/test/regress/output/copy.source
index b7e372d61b4c1f05f21507bd743a669cb1f24d42..cd65f13b4662b9b6fe0a5c345c42d99f47044048 100644 (file)
--- a/src/test/regress/output/copy.source
+++ b/src/test/regress/output/copy.source
@@ -95,3 +95,26 @@ copy copytest3 to stdout csv header;
 c1,"col with , comma","col with "" quote"
 1,a,1
 2,b,2
+-- test copy from with a partitioned table
+create table parted_copytest (
+   a int,
+   b int,
+   c text
+) partition by list (b);
+create table parted_copytest_a1 (c text, b int, a int);
+create table parted_copytest_a2 (a int, c text, b int);
+alter table parted_copytest attach partition parted_copytest_a1 for values in(1);
+alter table parted_copytest attach partition parted_copytest_a2 for values in(2);
+-- We must insert enough rows to trigger multi-inserts.  These are only
+-- enabled adaptively when there are few enough partition changes.
+insert into parted_copytest select x,1,'One' from generate_series(1,1000) x;
+insert into parted_copytest select x,2,'Two' from generate_series(1001,1010) x;
+insert into parted_copytest select x,1,'One' from generate_series(1011,1020) x;
+copy (select * from parted_copytest order by a) to '@abs_builddir@/results/parted_copytest.csv';
+-- Ensure COPY FREEZE errors for partitioned tables.
+begin;
+truncate parted_copytest;
+copy parted_copytest from '@abs_builddir@/results/parted_copytest.csv' (freeze);
+ERROR:  cannot perform FREEZE on a partitioned table
+rollback;
+drop table parted_copytest;