RE: [POC] Fast COPY FROM command for the table with foreign partitions
Hou, Zhijie <houzj.fnst@cn.fujitsu.com>
From: "Hou, Zhijie" <houzj.fnst@cn.fujitsu.com>
To: "Andrey V. Lepikhov" <a.lepikhov@postgrespro.ru>, Amit Langote <amitlangote09@gmail.com>, "Tsunakawa, Takayuki" <tsunakawa.takay@fujitsu.com>
Cc: Alexey Kondratov <a.kondratov@postgrespro.ru>, Michael Paquier <michael@paquier.xyz>, Ashutosh Bapat <ashutosh.bapat@2ndquadrant.com>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, Etsuro Fujita <etsuro.fujita@gmail.com>, PostgreSQL-Dev <pgsql-hackers@postgresql.org>, "Tang, Haiying" <tanghy.fnst@cn.fujitsu.com>
Date: 2020-12-29T11:20:47Z
Lists: 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
Commits
-
Allow batch insertion during COPY into a foreign table.
- 97da48246d34 16.0 landed
-
postgres_fdw: Disable batch insertion when there are WCO constraints.
- 82593b9a3d5f 16.0 cited
-
Enforce foreign key correctly during cross-partition updates
- ba9a7e392171 15.0 cited
-
Fix permission checks on constraint violation errors on partitions.
- 6214e2b22804 14.0 cited
-
Fix compilation of uuid-ossp
- 525e60b74299 14.0 cited