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

On Tue, 1 Apr 2025 at 06:31, jian he <jian.universality@gmail.com> wrote:
>
> On Mon, Mar 31, 2025 at 4:05 PM Kirill Reshke <reshkekirill@gmail.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