Re: speedup COPY TO for partitioned table. - Mailing list pgsql-hackers

On Mon, Jul 14, 2025 at 9:38 PM torikoshia <torikoshia@oss.nttdata.com> wrote:
>
> Based on the explanations in the glossary, should 'parition' be
> partitioned table/relation?
>
I think "Scan a single table (which may be a partition) and export its
rows to the...."
the word "partition" is correct.


> |  -- https://www.postgresql.org/docs/devel/glossary.html
> |  partition: One of several disjoint (not overlapping) subsets of a
> larger set
> |  Partitioned table(relation): A relation that is in semantic terms the
> same as a table, but whose storage is distributed across several
> partitions
>
> Also, the terms "table" and "relation" seem to be used somewhat
> interchangeably in this patch.
> For consistency, perhaps it's better to pick one term and use it
> consistently throughout the comments.
>
> 249 + * root_rel: if not NULL, it indicates that we are copying
> partitioned relation
> 270 +    * exporting partitioned table data here, we must convert it
> back to the
>

now it's:

+/*
+ * 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)
+{
+
+    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. If we are
+     * exporting partition 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 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.
>
> Hmm, I agree there are no regression tests for this, but is it about
> copying foreign table, isn't it?
>
> Since this patch is primarily about supporting COPY on partitioned
> tables, I’m not sure adding regression tests for foreign tables is in
> scope here.
> It might be better handled in a follow-up patch focused on improving
> test coverage for such unsupported cases, if we decide that's
> worthwhile.
>
i guess it should  be fine.
since we are only adding one somehow related test case.

+-- 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