Re: speedup COPY TO for partitioned table.

On Mon, Jul 14, 2025 at 9:38 PM torikoshia <torikoshia(at)oss(dot)nttdata(dot)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