Re: speedup COPY TO for partitioned table.

On Thu, Oct 9, 2025 at 12:10 AM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> On Thu, Oct 9, 2025 at 9:14 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> >
> > ---
> > + 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."));
> >
> > I think we don't need "the" in the error message.
> >
> > It's conventional to put all err*() macros in parentheses (i.e.,
> > "(errcode(), ...)", it's technically omittable though.
> >
>
> https://www.postgresql.org/docs/current/error-message-reporting.html
> QUOTE:
> <<<<>>>>>
> The extra parentheses were required before PostgreSQL version 12, but
> are now optional.
> Here is a more complex example:
> .....
> <<<<>>>>>
>
> related commit:
> https://git.postgresql.org/cgit/postgresql.git/commit/?id=e3a87b4991cc2d00b7a3082abb54c5f12baedfd1
> Less parenthesis is generally more readable, I think.

Yes, but I think it's more consistent given that we use the
parentheses in all other places in copyto.c.

>
> > How about doing "slot = execute_attr_map_slot(map, slot, root_slot);"
> > instead? (i.e., no need to have 'copyslot')
> >
>
> I tried but it seems not possible.
> table_scan_getnextslot function require certain type of "slot", if we do
> "slot = execute_attr_map_slot(map, slot, root_slot);"
> then pointer "slot" type becomes virtual slot, then
> it will fail on second time call table_scan_getnextslot

Right. Let's keep as it is.

I've attached a patch for cosmetic changes including comment updates,
indent fixes by pgindent, and renaming variable names. Some fixes are
just my taste, so please check the changes.

Also I have a few comments on new tests:

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

I think it's better to have both cases: partitions' rowtype match the
root's rowtype and partition's rowtype doesn't match the root's
rowtype.

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

async_p3 is a foreign table so it seems not related to this patch.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com