Re: speedup COPY TO for partitioned table.

<jian.universality@gmail.com> wrote:
>> 
>> On Fri, Oct 3, 2025 at 8:31 AM torikoshia <torikoshia@oss.nttdata.com> 
>> wrote:
>> >
>> > It’s been about two months since this discussion, and there don’t seem
>> > to be any further comments.
>> > How about updating the patch accordingly?
>> > If there are no new remarks, I’d like to mark the patch as Ready for
>> > Committer.
>> >
>> > >>   +   List       *children = NIL;
>> > >>   ...
>> > >>   +       {
>> > >>   +           children = find_all_inheritors(RelationGetRelid(rel),
>> > >>
>> > >> Since 'children' is only used inside the else if block, I think we
>> > >> don't
>> > >> need the separate "List *children = NIL;" declaration.
>> > >> Instead, it could just be  "List *children =
>> > >> find_all_inheritors(...)".
>> > >>
>> > > you are right.
>> > > ""List *children = find_all_inheritors(...)"."  should be ok.
>> >
>> 
>> hi.
>> 
>> please check the attached v15.

Thanks for updating the patch!
Here are some minor comments.

>   #include "access/tableam.h"
>  +#include "access/table.h"

As in partbounds.c, I think table.h should come before tableam.h in the 
include order.

> +               char         relkind = get_rel_relkind(childreloid);
> +
> +               if (relkind == RELKIND_FOREIGN_TABLE)
> +               {
> +                   char       *relation_name;
> +
> +                   relation_name = get_rel_name(childreloid);

Similar to how relkind is declared, it might be simpler to combine the 
declaration and assignment here.


On 2025-10-09 10:13, Masahiko Sawada wrote:

Thanks for your review!

> RelationGetRelationName(rel)),
> +                           errhint("Try the COPY (SELECT ...) TO 
> variant."));
> 
> I think we don't need "the" in the error message.

I agree. However, I noticed that some existing messages use “the” in 
similar contexts. For example:

           if (rel->rd_rel->relkind == RELKIND_VIEW)
               ereport(ERROR,
                       (errcode(ERRCODE_WRONG_OBJECT_TYPE),
                        errmsg("cannot copy from view \"%s\"",
                               RelationGetRelationName(rel)),
                        errhint("Try the COPY (SELECT ...) TO 
variant.")));

If we want to fix it, I think we should update all similar messages 
together for consistency.


-- 
Regards,

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