From 9b441c21dac61037eed697ea103977efc02db4fe Mon Sep 17 00:00:00 2001 From: Gokhan Gulbiz Date: Fri, 27 Jan 2023 16:34:11 +0300 Subject: [PATCH] Allow plain pg foreign tables without a table_name option (#6652) (cherry picked from commit 4e264649699815050bf68739b44883cfbf34ee6f) --- .../citus_add_local_table_to_metadata.c | 76 +++++++++++++++++++ .../distributed/commands/utility_hook.c | 59 -------------- .../regress/expected/citus_local_tables.out | 2 +- .../regress/expected/foreign_tables_mx.out | 30 +++++++- src/test/regress/sql/citus_local_tables.sql | 2 +- src/test/regress/sql/foreign_tables_mx.sql | 20 ++++- 6 files changed, 126 insertions(+), 63 deletions(-) diff --git a/src/backend/distributed/commands/citus_add_local_table_to_metadata.c b/src/backend/distributed/commands/citus_add_local_table_to_metadata.c index cceba336a..f07782cf0 100644 --- a/src/backend/distributed/commands/citus_add_local_table_to_metadata.c +++ b/src/backend/distributed/commands/citus_add_local_table_to_metadata.c @@ -46,6 +46,7 @@ #include "utils/lsyscache.h" #include "utils/ruleutils.h" #include "utils/syscache.h" +#include "foreign/foreign.h" /* @@ -61,6 +62,8 @@ static void ErrorIfAddingPartitionTableToMetadata(Oid relationId); static void ErrorIfUnsupportedCreateCitusLocalTable(Relation relation); static void ErrorIfUnsupportedCitusLocalTableKind(Oid relationId); static void ErrorIfUnsupportedCitusLocalColumnDefinition(Relation relation); +static void EnsureIfPostgresFdwHasTableName(Oid relationId); +static void ErrorIfOptionListHasNoTableName(List *optionList); static void NoticeIfAutoConvertingLocalTables(bool autoConverted, Oid relationId); static CascadeOperationType GetCascadeTypeForCitusLocalTables(bool autoConverted); static List * GetShellTableDDLEventsForCitusLocalTable(Oid relationId); @@ -485,6 +488,16 @@ ErrorIfUnsupportedCreateCitusLocalTable(Relation relation) EnsureTableNotDistributed(relationId); ErrorIfUnsupportedCitusLocalColumnDefinition(relation); + /* + * Error out with a hint if the foreign table is using postgres_fdw and + * the option table_name is not provided. + * Citus relays all the Citus local foreign table logic to the placement of the + * Citus local table. If table_name is NOT provided, Citus would try to talk to + * the foreign postgres table over the shard's table name, which would not exist + * on the remote server. + */ + EnsureIfPostgresFdwHasTableName(relationId); + /* * When creating other citus table types, we don't need to check that case as * EnsureTableNotDistributed already errors out if the given relation implies @@ -500,6 +513,69 @@ ErrorIfUnsupportedCreateCitusLocalTable(Relation relation) } +/* + * ServerUsesPostgresFdw gets a foreign server Oid and returns true if the FDW that + * the server depends on is postgres_fdw. Returns false otherwise. + */ +static bool +ServerUsesPostgresFdw(Oid serverId) +{ + ForeignServer *server = GetForeignServer(serverId); + ForeignDataWrapper *fdw = GetForeignDataWrapper(server->fdwid); + + if (strcmp(fdw->fdwname, "postgres_fdw") == 0) + { + return true; + } + + return false; +} + + +/* + * EnsureIfPostgresFdwHasTableName errors out with a hint if the foreign table is using postgres_fdw and + * the option table_name is not provided. + */ +static void +EnsureIfPostgresFdwHasTableName(Oid relationId) +{ + char relationKind = get_rel_relkind(relationId); + if (relationKind == RELKIND_FOREIGN_TABLE) + { + ForeignTable *foreignTable = GetForeignTable(relationId); + if (ServerUsesPostgresFdw(foreignTable->serverid)) + { + ErrorIfOptionListHasNoTableName(foreignTable->options); + } + } +} + + +/* + * ErrorIfOptionListHasNoTableName gets an option list (DefElem) and errors out + * if the list does not contain a table_name element. + */ +static void +ErrorIfOptionListHasNoTableName(List *optionList) +{ + char *table_nameString = "table_name"; + DefElem *option = NULL; + foreach_ptr(option, optionList) + { + char *optionName = option->defname; + if (strcmp(optionName, table_nameString) == 0) + { + return; + } + } + + ereport(ERROR, (errmsg( + "table_name option must be provided when using postgres_fdw with Citus"), + errhint("Provide the option \"table_name\" with value target table's" + " name"))); +} + + /* * ErrorIfUnsupportedCitusLocalTableKind errors out if the relation kind of * relation with relationId is not supported for citus local table creation. diff --git a/src/backend/distributed/commands/utility_hook.c b/src/backend/distributed/commands/utility_hook.c index 5b72770e6..5d3fd1157 100644 --- a/src/backend/distributed/commands/utility_hook.c +++ b/src/backend/distributed/commands/utility_hook.c @@ -112,9 +112,6 @@ static void DecrementUtilityHookCountersIfNecessary(Node *parsetree); static bool IsDropSchemaOrDB(Node *parsetree); static bool ShouldCheckUndistributeCitusLocalTables(void); static bool ShouldAddNewTableToMetadata(Node *parsetree); -static bool ServerUsesPostgresFDW(char *serverName); -static void ErrorIfOptionListHasNoTableName(List *optionList); - /* * ProcessUtilityParseTree is a convenience method to create a PlannedStmt out of @@ -763,18 +760,6 @@ ProcessUtilityInternal(PlannedStmt *pstmt, CreateStmt *createTableStmt = (CreateStmt *) (&createForeignTableStmt->base); - /* - * Error out with a hint if the foreign table is using postgres_fdw and - * the option table_name is not provided. - * Citus relays all the Citus local foreign table logic to the placement of the - * Citus local table. If table_name is NOT provided, Citus would try to talk to - * the foreign postgres table over the shard's table name, which would not exist - * on the remote server. - */ - if (ServerUsesPostgresFDW(createForeignTableStmt->servername)) - { - ErrorIfOptionListHasNoTableName(createForeignTableStmt->options); - } PostprocessCreateTableStmt(createTableStmt, queryString); } @@ -1066,50 +1051,6 @@ ShouldAddNewTableToMetadata(Node *parsetree) } -/* - * ServerUsesPostgresFDW gets a foreign server name and returns true if the FDW that - * the server depends on is postgres_fdw. Returns false otherwise. - */ -static bool -ServerUsesPostgresFDW(char *serverName) -{ - ForeignServer *server = GetForeignServerByName(serverName, false); - ForeignDataWrapper *fdw = GetForeignDataWrapper(server->fdwid); - - if (strcmp(fdw->fdwname, "postgres_fdw") == 0) - { - return true; - } - - return false; -} - - -/* - * ErrorIfOptionListHasNoTableName gets an option list (DefElem) and errors out - * if the list does not contain a table_name element. - */ -static void -ErrorIfOptionListHasNoTableName(List *optionList) -{ - char *table_nameString = "table_name"; - DefElem *option = NULL; - foreach_ptr(option, optionList) - { - char *optionName = option->defname; - if (strcmp(optionName, table_nameString) == 0) - { - return; - } - } - - ereport(ERROR, (errmsg( - "table_name option must be provided when using postgres_fdw with Citus"), - errhint("Provide the option \"table_name\" with value target table's" - " name"))); -} - - /* * NotifyUtilityHookConstraintDropped sets ConstraintDropped to true to tell us * last command dropped a table constraint. diff --git a/src/test/regress/expected/citus_local_tables.out b/src/test/regress/expected/citus_local_tables.out index 1c95f578b..1fe69cc6d 100644 --- a/src/test/regress/expected/citus_local_tables.out +++ b/src/test/regress/expected/citus_local_tables.out @@ -223,7 +223,7 @@ ROLLBACK; CREATE FOREIGN TABLE foreign_table ( id bigint not null, full_name text not null default '' -) SERVER fake_fdw_server OPTIONS (encoding 'utf-8', compression 'true'); +) SERVER fake_fdw_server OPTIONS (encoding 'utf-8', compression 'true', table_name 'foreign_table'); -- observe that we do not create fdw server for shell table, both shard relation -- & shell relation points to the same same server object -- Disable metadata sync since citus doesn't support distributing diff --git a/src/test/regress/expected/foreign_tables_mx.out b/src/test/regress/expected/foreign_tables_mx.out index 17b1c99b8..b2574432c 100644 --- a/src/test/regress/expected/foreign_tables_mx.out +++ b/src/test/regress/expected/foreign_tables_mx.out @@ -4,6 +4,15 @@ SET citus.shard_replication_factor TO 1; SET citus.enable_local_execution TO ON; CREATE SCHEMA foreign_tables_schema_mx; SET search_path TO foreign_tables_schema_mx; +SET client_min_messages to ERROR; +-- ensure that coordinator is added to pg_dist_node +SELECT 1 FROM master_add_node('localhost', :master_port, groupId => 0); + ?column? +--------------------------------------------------------------------- + 1 +(1 row) + +RESET client_min_messages; -- test adding foreign table to metadata with the guc SET citus.use_citus_managed_tables TO ON; CREATE TABLE foreign_table_test (id integer NOT NULL, data text, a bigserial); @@ -379,14 +388,33 @@ SELECT * FROM ref_tbl d JOIN foreign_table_local f ON d.a=f.id ORDER BY f.id; \c - - - :master_port SET search_path TO foreign_tables_schema_mx; --- should error out because doesn't have a table_name field CREATE FOREIGN TABLE foreign_table_local_fails ( id integer NOT NULL, data text ) SERVER foreign_server_local OPTIONS (schema_name 'foreign_tables_schema_mx'); +-- should error out because doesn't have a table_name field +SELECT citus_add_local_table_to_metadata('foreign_table_local_fails'); ERROR: table_name option must be provided when using postgres_fdw with Citus +-- should work since it has a table_name +ALTER FOREIGN TABLE foreign_table_local_fails OPTIONS (table_name 'foreign_table_test'); +SELECT citus_add_local_table_to_metadata('foreign_table_local_fails'); + citus_add_local_table_to_metadata +--------------------------------------------------------------------- + +(1 row) + +INSERT INTO foreign_table_test VALUES (1, 'test'); +SELECT undistribute_table('foreign_table_local_fails'); +NOTICE: creating a new table for foreign_tables_schema_mx.foreign_table_local_fails +NOTICE: dropping the old foreign_tables_schema_mx.foreign_table_local_fails +NOTICE: renaming the new table to foreign_tables_schema_mx.foreign_table_local_fails + undistribute_table +--------------------------------------------------------------------- + +(1 row) + DROP FOREIGN TABLE foreign_table_local; -- cleanup at exit set client_min_messages to error; diff --git a/src/test/regress/sql/citus_local_tables.sql b/src/test/regress/sql/citus_local_tables.sql index 57f93b076..312744dc2 100644 --- a/src/test/regress/sql/citus_local_tables.sql +++ b/src/test/regress/sql/citus_local_tables.sql @@ -176,7 +176,7 @@ ROLLBACK; CREATE FOREIGN TABLE foreign_table ( id bigint not null, full_name text not null default '' -) SERVER fake_fdw_server OPTIONS (encoding 'utf-8', compression 'true'); +) SERVER fake_fdw_server OPTIONS (encoding 'utf-8', compression 'true', table_name 'foreign_table'); -- observe that we do not create fdw server for shell table, both shard relation -- & shell relation points to the same same server object diff --git a/src/test/regress/sql/foreign_tables_mx.sql b/src/test/regress/sql/foreign_tables_mx.sql index fdd391d10..7b6784aab 100644 --- a/src/test/regress/sql/foreign_tables_mx.sql +++ b/src/test/regress/sql/foreign_tables_mx.sql @@ -7,6 +7,14 @@ SET citus.enable_local_execution TO ON; CREATE SCHEMA foreign_tables_schema_mx; SET search_path TO foreign_tables_schema_mx; + +SET client_min_messages to ERROR; + +-- ensure that coordinator is added to pg_dist_node +SELECT 1 FROM master_add_node('localhost', :master_port, groupId => 0); + +RESET client_min_messages; + -- test adding foreign table to metadata with the guc SET citus.use_citus_managed_tables TO ON; CREATE TABLE foreign_table_test (id integer NOT NULL, data text, a bigserial); @@ -219,7 +227,6 @@ SELECT * FROM ref_tbl d JOIN foreign_table_local f ON d.a=f.id ORDER BY f.id; SET search_path TO foreign_tables_schema_mx; --- should error out because doesn't have a table_name field CREATE FOREIGN TABLE foreign_table_local_fails ( id integer NOT NULL, data text @@ -227,6 +234,17 @@ CREATE FOREIGN TABLE foreign_table_local_fails ( SERVER foreign_server_local OPTIONS (schema_name 'foreign_tables_schema_mx'); +-- should error out because doesn't have a table_name field +SELECT citus_add_local_table_to_metadata('foreign_table_local_fails'); + +-- should work since it has a table_name +ALTER FOREIGN TABLE foreign_table_local_fails OPTIONS (table_name 'foreign_table_test'); +SELECT citus_add_local_table_to_metadata('foreign_table_local_fails'); + +INSERT INTO foreign_table_test VALUES (1, 'test'); + +SELECT undistribute_table('foreign_table_local_fails'); + DROP FOREIGN TABLE foreign_table_local; -- cleanup at exit