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

On 2025-07-02 13:10, jian he wrote:
Thanks for updating the patch.

> now it is:
> /*
>  * Scan a single table (which may be a partition) and export its rows
> to the
>  * COPY destination.

Based on the explanations in the glossary, should 'parition' be
partitioned table/relation?

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

>  *
>  * 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.

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.



--https://coverage.postgresql.org/src/backend/commands/copyto.c.gcov.html
   670           0 :         else if (rel->rd_rel->relkind ==
RELKIND_SEQUENCE)
   671           0 :             ereport(ERROR,
   672             :
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
   673             :                      errmsg("cannot copy from
sequence \"%s\"",
   674             :
RelationGetRelationName(rel))));

Also, I’m not entirely sure how much value such tests would bring,
especially if the error paths are straightforward and unlikely to
regress.


Regarding performance: it's already confirmed that COPY
partitioned_table performs better than COPY (SELECT * FROM
partitioned_table) as expected [1].
I was a bit curious, though, whether this patch might introduce any
performance regression when copying a regular (non-partitioned) table.
To check this, I ran a simple benchmark and did not observe any
degradation.
To minimize I/O overhead, I used a tmpfs mount:

   % mkdir /tmp/mem
   % sudo mount_tmpfs -s 500M /tmp/mem
   % pgbench -i

Then I ran the following command several times on both patched and
unpatched builds:
   =# COPY pgbench_accounts TO '/tmp/mem/accounts';


[1]
https://www.postgresql.org/message-id/174219852967.294107.6195385625494034792.pgcf%40coridan.postgresql.org


--
Regards,

--
Atsushi Torikoshi
Seconded from NTT DATA Japan Corporation to SRA OSS K.K.