Re: TRUNCATE on foreign table
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
To: Kazutaka Onishi <onishi@heterodb.com>
Cc: Fujii Masao <masao.fujii@oss.nttdata.com>,
Kohei KaiGai <kaigai@heterodb.com>, Amit Langote <amitlangote09@gmail.com>, Ibrar Ahmed <ibrar.ahmad@gmail.com>, Zhihong Yu <zyu@yugabyte.com>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: 2021-04-03T14:34:33Z
Lists: pgsql-hackers
On Sat, Apr 3, 2021 at 7:16 PM Kazutaka Onishi <onishi@heterodb.com> wrote: > > Sorry but I found the v7 patch has typo and it can't be built... > I attached fixed one(v8). Thanks for the patch. Here are some comments on v8 patch: 1) We usually have the struct name after "+typedef struct ForeignTruncateInfo", please refer to other struct defs in the code base. 2) We should add ORDER BY clause(probably ORDER BY id?) for data generating select queries in added tests, otherwise tests might become unstable. 3) How about dropping the tables, foreign tables that got created for testing in postgres_fdw.sql? 4) I think it's not "foreign-tables"/"foreign-table", it can be "foreign tables"/"foreign table", other places in the docs use this convention. + the context where the foreign-tables are truncated. It is a list of integers and same length with 5) Can't we use do_sql_command function after making it non static? We could go extra mile, that is we could make do_sql_command little more generic by passing some enum for each of PQsendQuery, PQsendQueryParams, PQsendQueryPrepared and PQsendPrepare and replace the respective code chunks with do_sql_command in postgres_fdw.c. + /* run remote query */ + if (!PQsendQuery(conn, sql.data)) + pgfdw_report_error(ERROR, NULL, conn, false, sql.data); + res = pgfdw_get_result(conn, sql.data); + if (PQresultStatus(res) != PGRES_COMMAND_OK) + pgfdw_report_error(ERROR, res, conn, true, sql.data); + /* clean-up */ + PQclear(res); 6) A white space error when the patch is applied. contrib/postgres_fdw/postgres_fdw.c:2913: trailing whitespace. + 7) I may be missing something here. Why do we need a hash table at all? We could just do it with a linked list right? Is there a specific reason to use a hash table? IIUC, the hash table entries will be lying around until the local session exists since we are not doing hash_destroy. 8) How about having something like this? + <command>TRUNCATE</command> can be used for foreign tables if the foreign data wrapper supports, for instance, see <xref linkend="postgres-fdw"/>. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
Commits
-
doc: Review for "Allow TRUNCATE command to truncate foreign tables".
- 0c8f40863acb 14.0 landed
-
Don't pass "ONLY" options specified in TRUNCATE to foreign data wrapper.
- 8e9ea08bae93 14.0 landed
-
Support tab-complete for TRUNCATE on foreign tables.
- 81e094bdfdd6 14.0 landed
-
Allow TRUNCATE command to truncate foreign tables.
- 8ff1c94649f5 14.0 landed
-
Add support for asynchronous execution.
- 27e1f14563cf 14.0 cited