Re: speedup COPY TO for partitioned table.

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.