On Mon, Jun 30, 2025 at 3:57 PM torikoshia <[email protected]> wrote:
>
> >> ```
> >> scan_rel = table_open(scan_oid,
> >> AccessShareLock);
> >>
> >> CopyThisRelTo(cstate, scan_rel, cstate->rel,
> >> &processed);
> >>
> >> table_close(scan_rel, AccessShareLock);
> >> ``
> >>
> > we can remove these empty new lines.
> > actually, I realized we don't need to use AccessShareLock here—we can
> > use NoLock
> > instead, since BeginCopyTo has already acquired AccessShareLock via
> > find_all_inheritors.
>
> That makes sense.
> I think it would be helpful to add a comment explaining why NoLock is
> safe here — for example:
>
> /* We already got the needed lock */
>
> In fact, in other places where table_open(..., NoLock) is used, similar
> explanatory comments are often included(Above comment is one of them).
>
hi.
I changed it to:
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);
}
> > what do you think the following
> >
> > /*
> > * CopyThisRelTo:
> > * This will scanning a single table (which may be a partition) and
> > exporting
> > * its rows to a COPY destination.
> > *
> > * rel: the relation from which the actual data will be copied.
> > * root_rel: if not NULL, it indicates that we are copying partitioned
> > relation
> > * data to the destination, and "rel" is the partition of "root_rel".
> > * processed: number of tuples processed.
> > */
> > static void
> > CopyThisRelTo(CopyToState cstate, Relation rel, Relation root_rel,
> > uint64 *processed)
>
> I think it would be better to follow the style of nearby functions in
> the same file. For example:
>
> /*
> * Scan a single table (which may be a partition) and export
> * its rows to the COPY destination.
> */
>
now it is:
/*
* Scan a single table (which may be a partition) and export its rows to the
* COPY destination.
*
* rel: the relation from which the actual data will be copied.
* root_rel: if not NULL, it indicates that we are copying partitioned relation
* 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)
> Also, regarding the function name CopyThisRelTo() — I wonder if the
> "This" is really necessary?
> Maybe something simpler like CopyRelTo() is enough.
>
> What do you think?
>
sure. CopyRelTo looks good to me.
while at it.
I found that in BeginCopyTo:
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot copy from foreign table \"%s\"",
RelationGetRelationName(rel)),
errhint("Try the COPY (SELECT ...) TO variant.")));
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."));
don't have any regress tests on it.
see https://coverage.postgresql.org/src/backend/commands/copyto.c.gcov.html
So I added some tests on contrib/postgres_fdw/sql/postgres_fdw.sql
From 26eb0aa22c091d9f3de6db0433b5202ebe564bdd Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Wed, 2 Jul 2025 12:07:19 +0800
Subject: [PATCH v13 1/1] support COPY partitioned_table TO
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]>
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 | 151 ++++++++++++++----
src/test/regress/expected/copy.out | 18 +++
src/test/regress/sql/copy.sql | 15 ++
6 files changed, 171 insertions(+), 34 deletions(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 2185b42bb4f..05f64157832 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -11475,6 +11475,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 e534b40de3c..d11105a20dc 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3872,6 +3872,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 8433344e5b6..0775a799a5e 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -521,13 +521,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 not partitioned 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,
+ <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 ea6f18f2c80..ca25cba2a15 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -19,6 +19,8 @@
#include <sys/stat.h>
#include "access/tableam.h"
+#include "access/table.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,8 @@ BeginCopyTo(ParseState *pstate,
PROGRESS_COPY_COMMAND_TO,
0
};
+ List *children = NIL;
+ List *scan_oids = NIL;
if (rel != NULL && rel->rd_rel->relkind != RELKIND_RELATION)
{
@@ -673,11 +680,34 @@ 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.")));
+ {
+ 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))
+ continue;
+
+ scan_oids = lappend_oid(scan_oids, childreloid);
+ }
+ }
else
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
@@ -713,6 +743,7 @@ BeginCopyTo(ParseState *pstate,
cstate->rel = rel;
tupDesc = RelationGetDescr(cstate->rel);
+ cstate->partitions = list_copy(scan_oids);
}
else
{
@@ -722,6 +753,7 @@ BeginCopyTo(ParseState *pstate,
DestReceiver *dest;
cstate->rel = NULL;
+ cstate->partitions = NIL;
/*
* Run parse analysis and rewrite. Note this also acquires sufficient
@@ -1030,7 +1062,7 @@ DoCopyTo(CopyToState cstate)
TupleDesc tupDesc;
int num_phys_attrs;
ListCell *cur;
- uint64 processed;
+ uint64 processed = 0;
if (fe_copy)
SendCopyBegin(cstate);
@@ -1068,36 +1100,24 @@ DoCopyTo(CopyToState cstate)
cstate->routine->CopyToStart(cstate, tupDesc);
- if (cstate->rel)
+ /*
+ * 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)
{
- 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))
+ foreach_oid(scan_oid, cstate->partitions)
{
- CHECK_FOR_INTERRUPTS();
+ 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 if (cstate->rel)
+ CopyRelTo(cstate, cstate->rel, NULL, &processed);
else
{
/* run the plan --- the dest receiver will send tuples */
@@ -1115,6 +1135,75 @@ DoCopyTo(CopyToState cstate)
return processed;
}
+/*
+ * Scan a single table (which may be a partition) and export its rows to the
+ * COPY destination.
+ *
+ * rel: the relation from which the actual data will be copied.
+ * root_rel: if not NULL, it indicates that we are copying partitioned relation
+ * 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;
+ TupleDesc tupdesc;
+ TupleDesc rootdesc;
+
+ tupdesc = RelationGetDescr(rel);
+ scandesc = table_beginscan(rel, GetActiveSnapshot(), 0, NULL);
+ slot = table_slot_create(rel, NULL);
+
+ /*
+ * A partition's rowtype might differ from the root table's. Since we are
+ * exporting partitioned table data here, we must convert it back to the
+ * root table's rowtype.
+ */
+ 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);
+ }
+
+ while (table_scan_getnextslot(scandesc, ForwardScanDirection, slot))
+ {
+ TupleTableSlot *copyslot;
+
+ CHECK_FOR_INTERRUPTS();
+
+ /* Deconstruct the tuple ... */
+ if (map != NULL)
+ copyslot = execute_attr_map_slot(map, slot, root_slot);
+ else
+ {
+ 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);
+ 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 8d5a06563c4..05e5649c1bc 100644
--- a/src/test/regress/expected/copy.out
+++ b/src/test/regress/expected/copy.out
@@ -350,3 +350,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 f0b88a23db8..9be7cb6c8dc 100644
--- a/src/test/regress/sql/copy.sql
+++ b/src/test/regress/sql/copy.sql
@@ -375,3 +375,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