On 2021/04/09 11:05, Zhihong Yu wrote:
On Thu, Apr 8, 2021 at 6:47 PM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com
<mailto:bharath.rupireddyforpostg...@gmail.com>> wrote:
On Thu, Apr 8, 2021 at 6:44 PM Fujii Masao <masao.fu...@oss.nttdata.com
<mailto:masao.fu...@oss.nttdata.com>> wrote:
> The followings are the open items and discussion points that I'm
thinking of.
>
> 1. Currently the extra information (TRUNCATE_REL_CONTEXT_NORMAL,
TRUNCATE_REL_CONTEXT_ONLY or TRUNCATE_REL_CONTEXT_CASCADING) about how a foreign
table was specified as the target to truncate in TRUNCATE command is collected and
passed to FDW. Does this really need to be passed to FDW? Seems Stephen, Michael
and I think that's necessary. But Kaigai-san does not. I also think that
TRUNCATE_REL_CONTEXT_CASCADING can be removed because there seems no use case for
that maybe.
I think we should remove the unused enums/macros, instead we could
mention a note of the extensibility of those enums/macros in the
comments section around the enum/macro definitions.
+1
> 2. Currently when the same foreign table is specified multiple times in the command, the
extra information only for the foreign table found first is collected. For example, when
"TRUNCATE ft, ONLY ft" is executed, TRUNCATE_REL_CONTEXT_NORMAL is collected and _ONLY is
ignored because "ft" is found first. Is this OK? Or we should collect all, e.g., both
_NORMAL and _ONLY should be collected in that example? I think that the current approach (i.e.,
collect the extra info about table found first if the same table is specified multiple times) is good
because even local tables are also treated the same way. But Kaigai-san does not.
IMO, the foreign truncate command should be constructed by collecting
all the information i.e. "TRUNCATE ft, ONLY ft" and let the remote
server execute how it wants to execute. That will be consistent and no
extra logic is required to track the already seen foreign tables while
foreign table collection/foreign truncate command is being prepared on
the local server.
But isn't it difficult for remote server to determine how to execute? Please
imagine the case where there are four tables as follows.
- regular table "remote_parent" in the remote server
- regular table "remote_child" inheriting "remote_parent" table in the remote
server
- foreign table "local_parent" in the local server, accessing "remote_parent"
table
- regular table "local_child" inheriting "local_parent" table in the local
server
When "TRUNCATE ONLY local_parent, local_parent" is executed, local_child is not
truncated because of ONLY clause. Then if we collect all the information about context,
both TRUNCATE_REL_CONTEXT_NORMAL and _ONLY are passed to FDW. In this case how should FDW
determine whether to use ONLY when issuing TRUNCATE command to the remote server? Isn't
it difficult to do that? If FDW determines not to use ONLY because _NORMAL flag is
passed, both remote_parent and remote_child tables are truncated. That is, though both
local_child and remote_child are the inheriting tables, isn't it strange that only the
former is ignored and the latter is truncated?
I was thinking that the postgres throws error or warning for commands
such as truncate, vaccum, analyze when the same tables are specified,
but seems like that's not what it does.
> 3. Currently postgres_fdw specifies ONLY clause in TRUNCATE command that
it constructs. That is, if the foreign table is specified with ONLY, postgres_fdw
also issues the TRUNCATE command for the corresponding remote table with ONLY to
the remote server. Then only root table is truncated in remote server side, and
the tables inheriting that are not truncated. Is this behavior desirable? Seems
Michael and I think this behavior is OK. But Kaigai-san does not.
I'm okay with the behaviour as it is consistent with what ONLY does to
local tables. Documenting this behaviour(if not done already) is a
better way I think.
+1
> 4. Tab-completion for TRUNCATE should be updated so that also foreign
tables are displayed.
It will be good to have.
Patch attached.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com <http://www.enterprisedb.com>
w.r.t. point #1:
bq. I think we should remove the unused enums/macros,
I agree. When there is more concrete use case which requires new enum, we can
add enum whose meaning would be clearer.
+1
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 26ac786c51..d34271e3b8 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -549,6 +549,18 @@ static const SchemaQuery Query_for_list_of_selectables = {
.result = "pg_catalog.quote_ident(c.relname)",
};
+/* Relations supporting TRUNCATE */
+static const SchemaQuery Query_for_list_of_truncatables = {
+ .catname = "pg_catalog.pg_class c",
+ .selcondition =
+ "c.relkind IN (" CppAsString2(RELKIND_RELATION) ", "
+ CppAsString2(RELKIND_FOREIGN_TABLE) ", "
+ CppAsString2(RELKIND_PARTITIONED_TABLE) ")",
+ .viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
+ .namespace = "c.relnamespace",
+ .result = "pg_catalog.quote_ident(c.relname)",
+};
+
/* Relations supporting GRANT are currently same as those supporting SELECT */
#define Query_for_list_of_grantables Query_for_list_of_selectables
@@ -3834,14 +3846,14 @@ psql_completion(const char *text, int start, int end)
/* TRUNCATE */
else if (Matches("TRUNCATE"))
- COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables,
+ COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_truncatables,
" UNION
SELECT 'TABLE'"
" UNION
SELECT 'ONLY'");
else if (Matches("TRUNCATE", "TABLE"))
- COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables,
+ COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_truncatables,
" UNION
SELECT 'ONLY'");
else if (HeadMatches("TRUNCATE") && TailMatches("ONLY"))
- COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
+ COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_truncatables,
NULL);
else if (Matches("TRUNCATE", MatchAny) ||
Matches("TRUNCATE", "TABLE|ONLY", MatchAny) ||
Matches("TRUNCATE", "TABLE", "ONLY", MatchAny))
