On Thu, Oct 9, 2025 at 9:14 AM Masahiko Sawada <[email protected]> wrote:
>
> > please check the attached v15.
> >
> Thank you for working on this! I've reviewed the v15 patch, and here
> are review comments:
>
> ---
> + children = find_all_inheritors(RelationGetRelid(rel),
> + AccessShareLock,
> + NULL);
> +
> + foreach_oid(childreloid, children)
> + {
> + char relkind = get_rel_relkind(childreloid);
>
> I think it's better to write some comments summarizing what we're
> doing in the loop.
sure.
>
> ---
> + relation_name = get_rel_name(childreloid);
> + ereport(ERROR,
> + errcode(ERRCODE_WRONG_OBJECT_TYPE),
> + errmsg("cannot copy from foreign table
> \"%s\"", relation_name),
> + errdetail("Partition \"%s\" is a foreign
> table in the partitioned table \"%s\"",
> + relation_name,
> RelationGetRelationName(rel)),
> + errhint("Try the COPY (SELECT ...) TO variant."));
>
> I think we don't need "the" in the error message.
>
> It's conventional to put all err*() macros in parentheses (i.e.,
> "(errcode(), ...)", it's technically omittable though.
>
https://www.postgresql.org/docs/current/error-message-reporting.html
QUOTE:
<<<<>>>>>
The extra parentheses were required before PostgreSQL version 12, but
are now optional.
Here is a more complex example:
.....
<<<<>>>>>
related commit:
https://git.postgresql.org/cgit/postgresql.git/commit/?id=e3a87b4991cc2d00b7a3082abb54c5f12baedfd1
Less parenthesis is generally more readable, I think.
> ---
> + if (RELKIND_HAS_PARTITIONS(relkind))
> + continue;
> +
> + scan_oids = lappend_oid(scan_oids, childreloid);
>
> find_all_inheritors() returns a list of OIDs of child relations. I
> think we can delete relations whose kind is RELKIND_HAS_PARTITIONS()
> from the list instead of creating a new list scan_oids. Then, we can
> set cstate->partition to the list.
>
yech, we can use foreach_delete_current to delete list elements on the fly.
> ---
> tupDesc = RelationGetDescr(cstate->rel);
> + cstate->partitions = list_copy(scan_oids);
> }
>
> Why do we need to copy the list here?
>
yech, list_copy is not needed.
> ---
> With the patch we have:
>
> /*
> * If COPY TO source table is a partitioned table, then open each
> * partition and process each individual partition.
> */
> if (cstate->rel && cstate->rel->rd_rel->relkind ==
> RELKIND_PARTITIONED_TABLE)
> {
> foreach_oid(scan_oid, cstate->partitions)
> {
> Relation scan_rel;
>
> /* We already got the needed lock in BeginCopyTo */
> scan_rel = table_open(scan_oid, NoLock);
> CopyRelTo(cstate, scan_rel, cstate->rel, &processed);
> table_close(scan_rel, NoLock);
> }
> }
> else if (cstate->rel)
> CopyRelTo(cstate, cstate->rel, NULL, &processed);
> else
> {
> /* run the plan --- the dest receiver will send tuples */
>
> I think we can refactor the code structure as follow:
>
> if (cstate->rel)
> {
> if (cstate->rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
> {
> do CopyRelTo() for each OIDs in cstate->partition here.
> }
> else
> CopyRelTo(cstate, cstate->rel, NULL, &processed);
> }
> else
> {
> ...
sure, this may increase readability.
> + if (root_rel != NULL)
> + {
> + rootdesc = RelationGetDescr(root_rel);
> + root_slot = table_slot_create(root_rel, NULL);
> + map = build_attrmap_by_name_if_req(rootdesc, tupdesc, false);
> + }
>
> rootdesc can be declared inside this if statement or we can directly
> pass 'RelationGetDescr(root_rel)' to build_attrmap_by_name_if_req().
>
sure. good idea.
> ---
> + /* Deconstruct the tuple ... */
> + if (map != NULL)
> + copyslot = execute_attr_map_slot(map, slot, root_slot);
> + else
> + {
> + slot_getallattrs(slot);
> + copyslot = slot;
> + }
>
> ISTM that the comment "Deconstruct the tuple" needs to move to before
> slot_getallattrs(slot).
>
ok.
> How about doing "slot = execute_attr_map_slot(map, slot, root_slot);"
> instead? (i.e., no need to have 'copyslot')
>
I tried but it seems not possible.
table_scan_getnextslot function require certain type of "slot", if we do
"slot = execute_attr_map_slot(map, slot, root_slot);"
then pointer "slot" type becomes virtual slot, then
it will fail on second time call table_scan_getnextslot
> ---
> + if (root_slot != NULL)
> + ExecDropSingleTupleTableSlot(root_slot);
> + table_endscan(scandesc);
>
> We might want to pfree 'map' if we create it.
>
ok.
Please check the attached v16.
From a9189e99050a0a9387df797d3a0bfb11afed5887 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Thu, 9 Oct 2025 15:05:52 +0800
Subject: [PATCH v16 1/1] support COPY partitioned_table TO
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This is for implementation of ``COPY (partitioned_table) TO``. it will be
faster than ``COPY (select * from partitioned_table) TO``.
If the destination table is a partitioned table, COPY table TO copies the same
rows as SELECT * FROM table.
reivewed by: vignesh C <[email protected]>
reivewed by: David Rowley <[email protected]>
reivewed by: Melih Mutlu <[email protected]>
reivewed by: Kirill Reshke <[email protected]>
reivewed by: Atsushi Torikoshi <[email protected]>
reivewed by: Álvaro Herrera <[email protected]>
reivewed by: Masahiko Sawada <[email protected]>
discussion: https://postgr.es/m/CACJufxEZt+G19Ors3bQUq-42-61__C=y5k2wk=sHEFRusu7=i...@mail.gmail.com
commitfest entry: https://commitfest.postgresql.org/patch/5467
---
.../postgres_fdw/expected/postgres_fdw.out | 8 +
contrib/postgres_fdw/sql/postgres_fdw.sql | 4 +
doc/src/sgml/ref/copy.sgml | 9 +-
src/backend/commands/copyto.c | 155 ++++++++++++++----
src/test/regress/expected/copy.out | 18 ++
src/test/regress/sql/copy.sql | 15 ++
6 files changed, 176 insertions(+), 33 deletions(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 91bbd0d8c73..2f9e315fd57 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -11599,6 +11599,14 @@ SELECT * FROM result_tbl ORDER BY a;
(3 rows)
DELETE FROM result_tbl;
+-- Test COPY TO with a foreign table or when the foreign table is a partition
+COPY async_p3 TO stdout; --error
+ERROR: cannot copy from foreign table "async_p3"
+HINT: Try the COPY (SELECT ...) TO variant.
+COPY async_pt TO stdout; --error
+ERROR: cannot copy from foreign table "async_p1"
+DETAIL: Partition "async_p1" is a foreign table in the partitioned table "async_pt"
+HINT: Try the COPY (SELECT ...) TO variant.
DROP FOREIGN TABLE async_p3;
DROP TABLE base_tbl3;
-- Check case where the partitioned table has local/remote partitions
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 3b7da128519..8a672f05039 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3941,6 +3941,10 @@ INSERT INTO result_tbl SELECT * FROM async_pt WHERE b === 505;
SELECT * FROM result_tbl ORDER BY a;
DELETE FROM result_tbl;
+-- Test COPY TO with a foreign table or when the foreign table is a partition
+COPY async_p3 TO stdout; --error
+COPY async_pt TO stdout; --error
+
DROP FOREIGN TABLE async_p3;
DROP TABLE base_tbl3;
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index c2d1fbc1fbe..ecd300097fc 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -539,13 +539,16 @@ COPY <replaceable class="parameter">count</replaceable>
<para>
<command>COPY TO</command> can be used with plain
- tables and populated materialized views.
- For example,
+ tables, populated materialized views and partitioned tables.
+ For example, if <replaceable class="parameter">table</replaceable> is a plain table,
<literal>COPY <replaceable class="parameter">table</replaceable>
TO</literal> copies the same rows as
<literal>SELECT * FROM ONLY <replaceable class="parameter">table</replaceable></literal>.
+ If <replaceable class="parameter">table</replaceable> is a partitioned table or a materialized view
+ <literal>COPY <replaceable class="parameter">table</replaceable> TO</literal>
+ copies the same rows as <literal>SELECT * FROM <replaceable class="parameter">table</replaceable></literal>.
However it doesn't directly support other relation types,
- such as partitioned tables, inheritance child tables, or views.
+ such as inheritance child tables, or views.
To copy all rows from such relations, use <literal>COPY (SELECT * FROM
<replaceable class="parameter">table</replaceable>) TO</literal>.
</para>
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index e5781155cdf..aba76eb0173 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -18,7 +18,9 @@
#include <unistd.h>
#include <sys/stat.h>
+#include "access/table.h"
#include "access/tableam.h"
+#include "catalog/pg_inherits.h"
#include "commands/copyapi.h"
#include "commands/progress.h"
#include "executor/execdesc.h"
@@ -82,6 +84,7 @@ typedef struct CopyToStateData
List *attnumlist; /* integer list of attnums to copy */
char *filename; /* filename, or NULL for STDOUT */
bool is_program; /* is 'filename' a program to popen? */
+ List *partitions; /* oid list of partition oid for copy to */
copy_data_dest_cb data_dest_cb; /* function for writing data */
CopyFormatOptions opts;
@@ -116,6 +119,8 @@ static void CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot);
static void CopyAttributeOutText(CopyToState cstate, const char *string);
static void CopyAttributeOutCSV(CopyToState cstate, const char *string,
bool use_quote);
+static void CopyRelTo(CopyToState cstate, Relation rel, Relation root_rel,
+ uint64 *processed);
/* built-in format-specific routines */
static void CopyToTextLikeStart(CopyToState cstate, TupleDesc tupDesc);
@@ -643,6 +648,7 @@ BeginCopyTo(ParseState *pstate,
PROGRESS_COPY_COMMAND_TO,
0
};
+ List *children = NIL;
if (rel != NULL && rel->rd_rel->relkind != RELKIND_RELATION)
{
@@ -673,11 +679,36 @@ BeginCopyTo(ParseState *pstate,
errmsg("cannot copy from sequence \"%s\"",
RelationGetRelationName(rel))));
else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot copy from partitioned table \"%s\"",
- RelationGetRelationName(rel)),
- errhint("Try the COPY (SELECT ...) TO variant.")));
+ {
+ /*
+ * Collect a list of partitions containing data, so that later
+ * DoCopyTo can copy the data from them.
+ */
+ children = find_all_inheritors(RelationGetRelid(rel),
+ AccessShareLock,
+ NULL);
+
+ foreach_oid(childreloid, children)
+ {
+ char relkind = get_rel_relkind(childreloid);
+
+ if (relkind == RELKIND_FOREIGN_TABLE)
+ {
+ char *relation_name;
+
+ relation_name = get_rel_name(childreloid);
+ ereport(ERROR,
+ errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot copy from foreign table \"%s\"", relation_name),
+ errdetail("Partition \"%s\" is a foreign table in the partitioned table \"%s\"",
+ relation_name, RelationGetRelationName(rel)),
+ errhint("Try the COPY (SELECT ...) TO variant."));
+ }
+
+ if (RELKIND_HAS_PARTITIONS(relkind))
+ children = foreach_delete_current(children, childreloid);
+ }
+ }
else
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
@@ -713,6 +744,7 @@ BeginCopyTo(ParseState *pstate,
cstate->rel = rel;
tupDesc = RelationGetDescr(cstate->rel);
+ cstate->partitions = children;
}
else
{
@@ -722,6 +754,7 @@ BeginCopyTo(ParseState *pstate,
DestReceiver *dest;
cstate->rel = NULL;
+ cstate->partitions = NIL;
/*
* Run parse analysis and rewrite. Note this also acquires sufficient
@@ -1030,7 +1063,7 @@ DoCopyTo(CopyToState cstate)
TupleDesc tupDesc;
int num_phys_attrs;
ListCell *cur;
- uint64 processed;
+ uint64 processed = 0;
if (fe_copy)
SendCopyBegin(cstate);
@@ -1070,33 +1103,24 @@ DoCopyTo(CopyToState cstate)
if (cstate->rel)
{
- TupleTableSlot *slot;
- TableScanDesc scandesc;
-
- scandesc = table_beginscan(cstate->rel, GetActiveSnapshot(), 0, NULL);
- slot = table_slot_create(cstate->rel, NULL);
-
- processed = 0;
- while (table_scan_getnextslot(scandesc, ForwardScanDirection, slot))
+ /*
+ * If COPY TO source table is a partitioned table, then open each
+ * partition and process each individual partition.
+ */
+ if (cstate->rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
{
- CHECK_FOR_INTERRUPTS();
+ foreach_oid(scan_oid, cstate->partitions)
+ {
+ Relation scan_rel;
- /* Deconstruct the tuple ... */
- slot_getallattrs(slot);
-
- /* Format and send the data */
- CopyOneRowTo(cstate, slot);
-
- /*
- * Increment the number of processed tuples, and report the
- * progress.
- */
- pgstat_progress_update_param(PROGRESS_COPY_TUPLES_PROCESSED,
- ++processed);
+ /* We already got the needed lock in BeginCopyTo */
+ scan_rel = table_open(scan_oid, NoLock);
+ CopyRelTo(cstate, scan_rel, cstate->rel, &processed);
+ table_close(scan_rel, NoLock);
+ }
}
-
- ExecDropSingleTupleTableSlot(slot);
- table_endscan(scandesc);
+ else
+ CopyRelTo(cstate, cstate->rel, NULL, &processed);
}
else
{
@@ -1115,6 +1139,77 @@ DoCopyTo(CopyToState cstate)
return processed;
}
+/*
+ * Scan a single table (which may be a partition) and export its rows to the
+ * COPY destination.
+ *
+ * rel: the table from which the actual data will be copied.
+ * root_rel: if not NULL, it indicates that COPY TO command copy partitioned
+ * table data to the destination, and "rel" is the partition of "root_rel".
+ * processed: number of tuples processed.
+*/
+static void
+CopyRelTo(CopyToState cstate, Relation rel, Relation root_rel,
+ uint64 *processed)
+{
+ TupleTableSlot *slot;
+ TableScanDesc scandesc;
+ AttrMap *map = NULL;
+ TupleTableSlot *root_slot = NULL;
+
+ scandesc = table_beginscan(rel, GetActiveSnapshot(), 0, NULL);
+ slot = table_slot_create(rel, NULL);
+
+ /*
+ * A partition's rowtype might differ from the root table's. If we are
+ * exporting partition data here, we must convert it back to the root
+ * table's rowtype.
+ */
+ if (root_rel != NULL)
+ {
+ root_slot = table_slot_create(root_rel, NULL);
+ map = build_attrmap_by_name_if_req(RelationGetDescr(root_rel),
+ RelationGetDescr(rel),
+ false);
+ }
+
+ while (table_scan_getnextslot(scandesc, ForwardScanDirection, slot))
+ {
+ TupleTableSlot *copyslot;
+
+ CHECK_FOR_INTERRUPTS();
+
+ if (map != NULL)
+ copyslot = execute_attr_map_slot(map, slot, root_slot);
+ else
+ {
+ /* Deconstruct the tuple ... */
+ slot_getallattrs(slot);
+ copyslot = slot;
+ }
+
+ /* Format and send the data */
+ CopyOneRowTo(cstate, copyslot);
+
+ /*
+ * Increment the number of processed tuples, and report the
+ * progress.
+ */
+ pgstat_progress_update_param(PROGRESS_COPY_TUPLES_PROCESSED,
+ ++(*processed));
+ }
+
+ ExecDropSingleTupleTableSlot(slot);
+
+ if (root_slot != NULL)
+ ExecDropSingleTupleTableSlot(root_slot);
+
+ if (map != NULL)
+ free_attrmap(map);
+
+ table_endscan(scandesc);
+}
+
/*
* Emit one row during DoCopyTo().
*/
diff --git a/src/test/regress/expected/copy.out b/src/test/regress/expected/copy.out
index ac66eb55aee..3bf5ecf469e 100644
--- a/src/test/regress/expected/copy.out
+++ b/src/test/regress/expected/copy.out
@@ -373,3 +373,21 @@ COPY copytest_mv(id) TO stdout WITH (header);
id
1
DROP MATERIALIZED VIEW copytest_mv;
+-- Tests for COPY TO with partitioned tables.
+CREATE TABLE pp (id int,val int) PARTITION BY RANGE (id);
+CREATE TABLE pp_1 (val int, id int) PARTITION BY RANGE (id);
+CREATE TABLE pp_2 (val int, id int) PARTITION BY RANGE (id);
+ALTER TABLE pp ATTACH PARTITION pp_1 FOR VALUES FROM (1) TO (5);
+ALTER TABLE pp ATTACH PARTITION pp_2 FOR VALUES FROM (5) TO (10);
+CREATE TABLE pp_15 PARTITION OF pp_1 FOR VALUES FROM (1) TO (5);
+CREATE TABLE pp_510 PARTITION OF pp_2 FOR VALUES FROM (5) TO (10);
+INSERT INTO pp SELECT g, 10 + g FROM generate_series(1,6) g;
+COPY pp TO stdout(header);
+id val
+1 11
+2 12
+3 13
+4 14
+5 15
+6 16
+DROP TABLE PP;
diff --git a/src/test/regress/sql/copy.sql b/src/test/regress/sql/copy.sql
index a1316c73bac..3d84764c65f 100644
--- a/src/test/regress/sql/copy.sql
+++ b/src/test/regress/sql/copy.sql
@@ -405,3 +405,18 @@ COPY copytest_mv(id) TO stdout WITH (header);
REFRESH MATERIALIZED VIEW copytest_mv;
COPY copytest_mv(id) TO stdout WITH (header);
DROP MATERIALIZED VIEW copytest_mv;
+
+-- Tests for COPY TO with partitioned tables.
+CREATE TABLE pp (id int,val int) PARTITION BY RANGE (id);
+CREATE TABLE pp_1 (val int, id int) PARTITION BY RANGE (id);
+CREATE TABLE pp_2 (val int, id int) PARTITION BY RANGE (id);
+ALTER TABLE pp ATTACH PARTITION pp_1 FOR VALUES FROM (1) TO (5);
+ALTER TABLE pp ATTACH PARTITION pp_2 FOR VALUES FROM (5) TO (10);
+
+CREATE TABLE pp_15 PARTITION OF pp_1 FOR VALUES FROM (1) TO (5);
+CREATE TABLE pp_510 PARTITION OF pp_2 FOR VALUES FROM (5) TO (10);
+
+INSERT INTO pp SELECT g, 10 + g FROM generate_series(1,6) g;
+
+COPY pp TO stdout(header);
+DROP TABLE PP;
--
2.34.1
