On Sun, Mar 30, 2025 at 9:14 AM vignesh C <[email protected]> wrote:
>
> On Sat, 29 Mar 2025 at 12:08, jian he <[email protected]> wrote:
> >
> >
> > I consolidated it into a new function: CopyThisRelTo.
>
> Few comments:
> 1) Here the error message is not correct, we are printing the original
> table from where copy was done which is a regular table and not a
> foreign table, we should use childreloid instead of rel.
>
> + if (relkind == RELKIND_FOREIGN_TABLE)
> + ereport(ERROR,
> +
> errcode(ERRCODE_WRONG_OBJECT_TYPE),
> + errmsg("cannot
> copy from foreign table \"%s\"",
> +
> RelationGetRelationName(rel)),
> +
> errdetail("partition \"%s\" is a foreign table",
> RelationGetRelationName(rel)),
> + errhint("Try
> the COPY (SELECT ...) TO variant."));
>
> In the error detail you can include the original table too.
>
I changed it to:
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.%s\"",
relation_name,
RelationGetRelationName(rel),
get_namespace_name(rel->rd_rel->relnamespace)),
errhint("Try the COPY (SELECT ...) TO variant."));
}
>
> 2) 2.a) I felt the comment should be "then copy partitioned rel to
> destionation":
> + * rel: the relation to be copied to.
> + * root_rel: if not null, then the COPY TO partitioned rel.
> + * processed: number of tuple processed.
> +*/
> +static void
> +CopyThisRelTo(CopyToState cstate, Relation rel, Relation root_rel,
> uint64 *processed)
> +{
> + TupleTableSlot *slot;
>
i changed it to:
+/*
+ * rel: the relation to be copied to.
+ * root_rel: if not null, then the COPY partitioned relation to destination.
+ * processed: number of tuple processed.
+*/
+static void
+CopyThisRelTo(CopyToState cstate, Relation rel, Relation root_rel,
+ uint64 *processed)
> 3) There is a small indentation issue here:
> + /*
> + * partition's rowtype might differ from the root table's. We must
> + * convert it back to the root table's rowtype as we are export
> + * partitioned table data here.
> + */
> + if (root_rel != NULL)
>
I am not so sure.
can you check if the attached still has the indentation issue.
From 4e501b7a8e67cffbf6432bdb43985b21d2b635b8 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Sun, 30 Mar 2025 21:47:12 +0800
Subject: [PATCH v7 1/1] support COPY partitioned_table TO
CREATE TABLE pp (id INT, val int ) PARTITION BY RANGE (id);
create table pp_1 (val int, id int);
create table pp_2 (val int, id int);
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);
insert into pp select g, 10 + g from generate_series(1,9) g;
copy pp to stdout(header);
the above case is much slower (around 25% in some case) than
``COPY (select * from pp) to stdout(header);``,
because of column remaping. but this is still a new
feature, since master does not support ``COPY (partitioned_table)``.
reivewed by: vignesh C <[email protected]>
reivewed by: David Rowley <[email protected]>
reivewed by: Melih Mutlu <[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/
---
doc/src/sgml/ref/copy.sgml | 8 +-
src/backend/commands/copyto.c | 143 ++++++++++++++++++++++------
src/test/regress/expected/copy2.out | 16 ++++
src/test/regress/sql/copy2.sql | 11 +++
4 files changed, 147 insertions(+), 31 deletions(-)
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index df093da97c5..f86e0b7ec35 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -521,15 +521,15 @@ COPY <replaceable class="parameter">count</replaceable>
<para>
<command>COPY TO</command> can be used only with plain
- tables, not views, and does not copy rows from child tables
- or child partitions. For example, <literal>COPY <replaceable
+ tables, not views, and does not copy rows from child tables,
+ however <command>COPY TO</command> can be used with partitioned tables.
+ For example, in a table inheritance hierarchy, <literal>COPY <replaceable
class="parameter">table</replaceable> TO</literal> copies
the same rows as <literal>SELECT * FROM ONLY <replaceable
class="parameter">table</replaceable></literal>.
The syntax <literal>COPY (SELECT * FROM <replaceable
class="parameter">table</replaceable>) TO ...</literal> can be used to
- dump all of the rows in an inheritance hierarchy, partitioned table,
- or view.
+ dump all of the rows in an inheritance hierarchy, or view.
</para>
<para>
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 84a3f3879a8..b75bbfea6a4 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 CopyThisRelTo(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)
{
@@ -670,11 +677,35 @@ 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.%s\"",
+ relation_name, RelationGetRelationName(rel),
+ get_namespace_name(rel->rd_rel->relnamespace)),
+ 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),
@@ -710,6 +741,7 @@ BeginCopyTo(ParseState *pstate,
cstate->rel = rel;
tupDesc = RelationGetDescr(cstate->rel);
+ cstate->partitions = list_copy(scan_oids);
}
else
{
@@ -1066,35 +1098,28 @@ 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);
+ scan_rel = table_open(scan_oid, AccessShareLock);
- /* Format and send the data */
- CopyOneRowTo(cstate, slot);
+ CopyThisRelTo(cstate, scan_rel, cstate->rel, &processed);
- /*
- * Increment the number of processed tuples, and report the
- * progress.
- */
- pgstat_progress_update_param(PROGRESS_COPY_TUPLES_PROCESSED,
- ++processed);
+ table_close(scan_rel, AccessShareLock);
}
-
- ExecDropSingleTupleTableSlot(slot);
- table_endscan(scandesc);
+ }
+ else if (cstate->rel)
+ {
+ processed = 0;
+ CopyThisRelTo(cstate, cstate->rel, NULL, &processed);
}
else
{
@@ -1113,6 +1138,70 @@ DoCopyTo(CopyToState cstate)
return processed;
}
+/*
+ * rel: the relation to be copied to.
+ * root_rel: if not null, then the COPY partitioned relation to destination.
+ * processed: number of tuple processed.
+*/
+static void
+CopyThisRelTo(CopyToState cstate, Relation rel, Relation root_rel,
+ uint64 *processed)
+{
+ TupleTableSlot *slot;
+ TableScanDesc scandesc;
+ AttrMap *map = NULL;
+ TupleTableSlot *root_slot = NULL;
+ TupleTableSlot *original_slot = NULL;
+ TupleDesc scan_tupdesc;
+ TupleDesc rootdesc = NULL;
+
+ scan_tupdesc = RelationGetDescr(rel);
+ scandesc = table_beginscan(rel, GetActiveSnapshot(), 0, NULL);
+ slot = table_slot_create(rel, NULL);
+
+ /*
+ * partition's rowtype might differ from the root table's. We must
+ * convert it back to the root table's rowtype as we are export
+ * partitioned table data here.
+ */
+ if (root_rel != NULL)
+ {
+ rootdesc = RelationGetDescr(root_rel);
+ map = build_attrmap_by_name_if_req(rootdesc, scan_tupdesc, false);
+ }
+
+ while (table_scan_getnextslot(scandesc, ForwardScanDirection, slot))
+ {
+ CHECK_FOR_INTERRUPTS();
+
+ /* Deconstruct the tuple ... */
+ if (map != NULL)
+ {
+ original_slot = slot;
+ root_slot = MakeSingleTupleTableSlot(rootdesc, &TTSOpsBufferHeapTuple);
+ slot = execute_attr_map_slot(map, slot, root_slot);
+ }
+ else
+ 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));
+
+ if (original_slot != NULL)
+ ExecDropSingleTupleTableSlot(original_slot);
+ }
+
+ ExecDropSingleTupleTableSlot(slot);
+ table_endscan(scandesc);
+}
+
/*
* Emit one row during DoCopyTo().
*/
diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
index 64ea33aeae8..dcd97ae45b7 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -929,3 +929,19 @@ truncate copy_default;
-- DEFAULT cannot be used in COPY TO
copy (select 1 as test) TO stdout with (default '\D');
ERROR: COPY DEFAULT cannot be used with COPY TO
+-- COPY TO with partitioned table
+CREATE TABLE pp (id INT, val int ) PARTITION BY RANGE (id);
+create table pp_1 (val int, id int);
+create table pp_2 (val int, id int);
+ALTER TABLE pp ATTACH PARTITION pp_1 FOR VALUES FROM (1) TO (3);
+ALTER TABLE pp ATTACH PARTITION pp_2 FOR VALUES FROM (3) TO (7);
+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/copy2.sql b/src/test/regress/sql/copy2.sql
index 45273557ce0..ba984388248 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -707,3 +707,14 @@ truncate copy_default;
-- DEFAULT cannot be used in COPY TO
copy (select 1 as test) TO stdout with (default '\D');
+
+-- COPY TO with partitioned table
+CREATE TABLE pp (id INT, val int ) PARTITION BY RANGE (id);
+create table pp_1 (val int, id int);
+create table pp_2 (val int, id int);
+ALTER TABLE pp ATTACH PARTITION pp_1 FOR VALUES FROM (1) TO (3);
+ALTER TABLE pp ATTACH PARTITION pp_2 FOR VALUES FROM (3) TO (7);
+insert into pp select g, 10 + g from generate_series(1,6) g;
+copy pp to stdout(header);
+
+DROP TABLE PP;
--
2.34.1
