RE: [POC] Fast COPY FROM command for the table with foreign partitions - Mailing list pgsql-hackers

From Hou, Zhijie
Subject RE: [POC] Fast COPY FROM command for the table with foreign partitions
Date
Msg-id 2b465f4cdeeb4a78b3096d31e8af7c77@G08CNEXMBPEKD05.g08.fujitsu.local
Whole thread Raw
In response to Re: [POC] Fast COPY FROM command for the table with foreign partitions  ("Andrey V. Lepikhov" <a.lepikhov@postgrespro.ru>)
Responses Re: [POC] Fast COPY FROM command for the table with foreign partitions
List pgsql-hackers
Hi

> see new version in attachment.

I took a look into the patch, and have some comments.

1.
+    PG_FINALLY();
+    {
+        copy_fmstate = NULL; /* Detect problems */
I don't quite understand this comment,
does it means we want to detect something like Null reference ?


2.
+    PG_FINALLY();
+    {
    ...
+        if (!OK)
+            PG_RE_THROW();
+    }
Is this PG_RE_THROW() necessary ? 
IMO, PG_FINALLY will reproduce the PG_RE_THROW action if we get to the code block due to an error being thrown.

3.
+            ereport(ERROR,
+                    (errmsg("unexpected extra results during COPY of table: %s",
+                            PQerrorMessage(conn))));

I found some similar message like the following:

            pg_log_warning("unexpected extra results during COPY of table \"%s\"",
                           tocEntryTag);
How about using existing messages style ?

4.
I noticed some not standard code comment[1].
I think it's better to comment like:
/*
 * line 1
 * line 2
 */

[1]-----------
+        /* Finish COPY IN protocol. It is needed to do after successful copy or
+         * after an error.
+         */


+/*
+ *
+ * postgresExecForeignCopy

+/*
+ *
+ * postgresBeginForeignCopy

-----------
Best regards,
Houzj




pgsql-hackers by date:

Previous
From: Magnus Hagander
Date:
Subject: Re: doc review for v14
Next
From: Noah Misch
Date:
Subject: Dump public schema ownership & seclabels