Allow plain pg foreign tables without a table_name option (#6652)

(cherry picked from commit 4e26464969)

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.

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.

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

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;

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

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