Re: speedup COPY TO for partitioned table.

From: jian he <jian(dot)universality(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: David Rowley <dgrowleyml(at)gmail(dot)com>, Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: speedup COPY TO for partitioned table.
Date: 2025-03-30 13:53:22
Message-ID: CACJufxGCGyeyemNvAtrGQmYx5Q8s+sDsKY8onVmTXfLHXE-eLw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Mar 30, 2025 at 9:14 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Sat, 29 Mar 2025 at 12:08, jian he <jian(dot)universality(at)gmail(dot)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.

Attachment Content-Type Size
v7-0001-support-COPY-partitioned_table-TO.patch text/x-patch 10.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nazir Bilal Yavuz 2025-03-30 14:00:45 Re: Using read stream in autoprewarm
Previous Message Tomas Vondra 2025-03-30 12:14:09 Re: Amcheck verification of GiST and GIN