Re: speedup COPY TO for partitioned table.

On Tue, 1 Apr 2025 at 06:31, jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> On Mon, Mar 31, 2025 at 4:05 PM Kirill Reshke <reshkekirill(at)gmail(dot)com> wrote:
>
> Thanks for doing the benchmark.

Few comments to improve the comments, error message and remove
redundant assignment:
1) How about we change below:
/*
* partition's rowtype might differ from the root table's. We must
* convert it back to the root table's rowtype as we are export
* partitioned table data here.
*/
To:
/*
* A partition's row type might differ from the root table's.
* Since we're exporting partitioned table data, we must
* convert it back to the root table's row type.
*/

2) How about we change below:
+ if (relkind == RELKIND_FOREIGN_TABLE)
+ ereport(ERROR,
+
errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot
copy from foreign table \"%s\"",
+
RelationGetRelationName(rel)),
+
errdetail("partition \"%s\" is a foreign table",
RelationGetRelationName(rel)),
+ errhint("Try
the COPY (SELECT ...) TO variant."));

To:
+ if (relkind == RELKIND_FOREIGN_TABLE)
+ ereport(ERROR,
+
errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot
copy from a partitioned table having foreign table partition \"%s\"",
+
RelationGetRelationName(rel)),
+
errdetail("partition \"%s\" is a foreign table",
RelationGetRelationName(rel)),
+ errhint("Try
the COPY (SELECT ...) TO variant."));

3) How about we change below:
/*
* rel: the relation to be copied to.
* root_rel: if not NULL, then the COPY partitioned relation to destination.
* processed: number of tuples processed.
*/
To:
/*
* rel: the relation from which data will be copied.
* root_rel: If not NULL, indicates that rel's row type must be
* converted to root_rel's row type.
* processed: number of tuples processed.
*/

4) You can initialize processed to 0 along with declaration in
DoCopyTo function and remove the below:
+ if (cstate->rel && cstate->rel->rd_rel->relkind ==
RELKIND_PARTITIONED_TABLE)
{
...
...
processed = 0;
- while (table_scan_getnextslot(scandesc,
ForwardScanDirection, slot))
...
...
-
- ExecDropSingleTupleTableSlot(slot);
- table_endscan(scandesc);
+ }
+ else if (cstate->rel)
+ {
+ processed = 0;
+ CopyThisRelTo(cstate, cstate->rel, NULL, &processed);
}

Regards,
Vignesh