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

On 2025-06-05 09:45, jian he wrote:
> hi.
> 
> In the V10 patch, there will be some regression if the partition column
> ordering is different from the root partitioned table.
> 
> because in V10 CopyThisRelTo
> + while (table_scan_getnextslot(scandesc, ForwardScanDirection, slot))
> + {
> + if (map != NULL)
> + {
> + original_slot = slot;
> + root_slot = MakeSingleTupleTableSlot(rootdesc, 
> &TTSOpsBufferHeapTuple);
> + slot = execute_attr_map_slot(map, slot, root_slot);
> + }
> + else
> + slot_getallattrs(slot);
> +
> + if (original_slot != NULL)
> + ExecDropSingleTupleTableSlot(original_slot);
> +}
> as you can see, for each slot in the partition, i called
> MakeSingleTupleTableSlot to get the dumpy root_slot
> and ExecDropSingleTupleTableSlot too.
> that will cause overhead.
> 
> we can call produce root_slot before the main while loop.
> like the following:
> + 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 (table_scan_getnextslot(scandesc, ForwardScanDirection, slot))
> + {
> + TupleTableSlot *copyslot;
> + if (map != NULL)
> + copyslot = execute_attr_map_slot(map, slot, root_slot);
> + else
> + {
> + slot_getallattrs(slot);
> + copyslot = slot;
> + }
> +
> please check CopyThisRelTo in v11. so, with v11, there is no
> regression for case when
> partition column ordering differs from partitioned.

Thanks for working on this improvement.

Here are some minor comments on v11 patch:

> +    For example, if <replaceable class="parameter">table</replaceable> 
> is not partitioned table,
>      <literal>COPY <replaceable class="parameter">table</replaceable>
>      TO</literal> copies the same rows as
>      <literal>SELECT * FROM ONLY <replaceable 
> class="parameter">table</replaceable></literal>

This describes the behavior when the table is not partitioned, but would 
it also be helpful to mention the behavior when the table is a 
partitioned table?
For example:

   If table is a partitioned table, then COPY table TO copies the same 
rows as SELECT * FROM table.


> +        * if COPY TO source table is a partitioned table, then open 
> each

if -> If


> +                       scan_rel = table_open(scan_oid, 
> AccessShareLock);
> 
> -                       /* Format and send the data */
> -                       CopyOneRowTo(cstate, slot);
> +                       CopyThisRelTo(cstate, scan_rel, cstate->rel, 
> &processed);
> 
> -                       /*
> -                        * Increment the number of processed tuples, 
> and report the
> -                        * progress.
> -                        */
> -                       
> pgstat_progress_update_param(PROGRESS_COPY_TUPLES_PROCESSED,
> -                                                                       
>          ++processed);
> +                       table_close(scan_rel, AccessShareLock)


After applying the patch, blank lines exist between these statements as 
below. Do we really need these blank lines?

```
                          scan_rel = table_open(scan_oid, 
AccessShareLock);

                          CopyThisRelTo(cstate, scan_rel, cstate->rel, 
&processed);

                          table_close(scan_rel, AccessShareLock);
``

> +/*
> + * 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
> +CopyThisRelTo(CopyToState cstate, Relation rel, Relation root_rel,

This comment only describes the parameters. Wouldn't it better to add a 
brief summary of what this function does overall?


  +        * A partition's rowtype might differ from the root table's.  
Since we are
  +        * export partitioned table data here, we must convert it back 
to the root

  are export -> are exporting?


-- 
Regards,

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