Re: speedup COPY TO for partitioned table.

On Sun, Mar 30, 2025 at 9:14 AM vignesh C <vignesh21@gmail.com> wrote:
>
> On Sat, 29 Mar 2025 at 12:08, jian he <jian.universality@gmail.com> wrote:
> >
> >
> > I consolidated it into a new function: CopyThisRelTo.
>
> Few comments:
> 1) Here the error message is not correct, we are printing the original
> table from where copy was done which is a regular table and not a
> foreign table, we should use childreloid instead of rel.
>
> +                               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."));
>
> In the error detail you can include the original table too.
>
I changed it to:
                if (relkind == RELKIND_FOREIGN_TABLE)
                {
                    char       *relation_name;

                    relation_name = get_rel_name(childreloid);
                    ereport(ERROR,
                            errcode(ERRCODE_WRONG_OBJECT_TYPE),
                            errmsg("cannot copy from foreign table
\"%s\"", relation_name),
                            errdetail("Partition \"%s\" is a foreign
table in the partitioned table \"%s.%s\"",
                                      relation_name,
RelationGetRelationName(rel),

get_namespace_name(rel->rd_rel->relnamespace)),
                            errhint("Try the COPY (SELECT ...) TO variant."));
                }

>
> 2) 2.a) I felt the comment should be "then copy partitioned rel to
> destionation":
> + * rel: the relation to be copied to.
> + * root_rel: if not null, then the COPY TO partitioned rel.
> + * processed: number of tuple processed.
> +*/
> +static void
> +CopyThisRelTo(CopyToState cstate, Relation rel, Relation root_rel,
> uint64 *processed)
> +{
> +       TupleTableSlot *slot;
>

i changed it to:
+/*
+ * rel: the relation to be copied to.
+ * root_rel: if not null, then the COPY partitioned relation to destination.
+ * processed: number of tuple processed.
+*/
+static void
+CopyThisRelTo(CopyToState cstate, Relation rel, Relation root_rel,
+              uint64 *processed)


> 3) There is a small indentation issue here:
> +       /*
> +        * 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.
> +       */
> +       if (root_rel != NULL)
>
I am not so sure.
can you check if the attached still has the indentation issue.