fix dropping table_name option from foreign table (#6669)

We should disallow dropping table_name option if foreign table is in
metadata. Otherwise, we get table not found error which contains
shardid.

DESCRIPTION: Fixes an unexpected foreign table error by disallowing to drop the table_name option.

Fixes #6663

src/backend/distributed/commands/citus_add_local_table_to_metadata.c

@@ -526,7 +526,7 @@ 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
+bool
 ServerUsesPostgresFdw(Oid serverId)
 {
 	ForeignServer *server = GetForeignServer(serverId);
@@ -585,6 +585,30 @@ ErrorIfOptionListHasNoTableName(List *optionList)
 }
 
 
+/*
+ * ForeignTableDropsTableNameOption returns true if given option list contains
+ * (DROP table_name).
+ */
+bool
+ForeignTableDropsTableNameOption(List *optionList)
+{
+	char *table_nameString = "table_name";
+	DefElem *option = NULL;
+	foreach_ptr(option, optionList)
+	{
+		char *optionName = option->defname;
+		DefElemAction optionAction = option->defaction;
+		if (strcmp(optionName, table_nameString) == 0 &&
+			optionAction == DEFELEM_DROP)
+		{
+			return true;
+		}
+	}
+
+	return false;
+}
+
+
 /*
  * ErrorIfUnsupportedCitusLocalTableKind errors out if the relation kind of
  * relation with relationId is not supported for citus local table creation.

src/backend/distributed/commands/table.c

@@ -41,6 +41,7 @@
 #include "distributed/resource_lock.h"
 #include "distributed/version_compat.h"
 #include "distributed/worker_shard_visibility.h"
+#include "foreign/foreign.h"
 #include "lib/stringinfo.h"
 #include "nodes/parsenodes.h"
 #include "parser/parse_expr.h"
@@ -119,6 +120,8 @@ static Oid get_attrdef_oid(Oid relationId, AttrNumber attnum);
 
 static char * GetAddColumnWithNextvalDefaultCmd(Oid sequenceOid, Oid relationId,
 												char *colname, TypeName *typeName);
+static void ErrorIfAlterTableDropTableNameFromPostgresFdw(List *optionList, Oid
+														  relationId);
 
 
 /*
@@ -3078,6 +3081,42 @@ ErrorIfUnsupportedConstraint(Relation relation, char distributionMethod,
 }
 
 
+/*
+ * ErrorIfAlterTableDropTableNameFromPostgresFdw errors if given alter foreign table
+ * option list drops 'table_name' from a postgresfdw foreign table which is
+ * inside metadata.
+ */
+static void
+ErrorIfAlterTableDropTableNameFromPostgresFdw(List *optionList, Oid relationId)
+{
+	char relationKind PG_USED_FOR_ASSERTS_ONLY =
+		get_rel_relkind(relationId);
+	Assert(relationKind == RELKIND_FOREIGN_TABLE);
+
+	ForeignTable *foreignTable = GetForeignTable(relationId);
+	Oid serverId = foreignTable->serverid;
+	if (!ServerUsesPostgresFdw(serverId))
+	{
+		return;
+	}
+
+	if (IsCitusTableType(relationId, CITUS_LOCAL_TABLE) &&
+		ForeignTableDropsTableNameOption(optionList))
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg(
+					 "alter foreign table alter options (drop table_name) command "
+					 "is not allowed for Citus tables"),
+				 errdetail(
+					 "Table_name option can not be dropped from a foreign table "
+					 "which is inside metadata."),
+				 errhint(
+					 "Try to undistribute foreign table before dropping table_name option.")));
+	}
+}
+
+
 /*
  * ErrorIfUnsupportedAlterTableStmt checks if the corresponding alter table
  * statement is supported for distributed tables and errors out if it is not.
@@ -3490,6 +3529,8 @@ ErrorIfUnsupportedAlterTableStmt(AlterTableStmt *alterTableStatement)
 			{
 				if (IsForeignTable(relationId))
 				{
+					List *optionList = (List *) command->def;
+					ErrorIfAlterTableDropTableNameFromPostgresFdw(optionList, relationId);
 					break;
 				}
 			}

src/include/distributed/metadata_utility.h

@@ -353,6 +353,8 @@ extern void EnsureRelationExists(Oid relationId);
 extern bool RegularTable(Oid relationId);
 extern bool TableEmpty(Oid tableId);
 extern bool IsForeignTable(Oid relationId);
+extern bool ForeignTableDropsTableNameOption(List *optionList);
+extern bool ServerUsesPostgresFdw(Oid serverId);
 extern char * ConstructQualifiedShardName(ShardInterval *shardInterval);
 extern uint64 GetFirstShardId(Oid relationId);
 extern Datum StringToDatum(char *inputString, Oid dataType);

src/test/regress/expected/foreign_tables_mx.out

@@ -416,6 +416,65 @@ NOTICE:  renaming the new table to foreign_tables_schema_mx.foreign_table_local_
 (1 row)
 
 DROP FOREIGN TABLE foreign_table_local;
+-- disallow dropping table_name when foreign table is in metadata
+CREATE TABLE table_name_drop(id int);
+CREATE FOREIGN TABLE foreign_table_name_drop_fails (
+        id INT
+)
+        SERVER foreign_server_local
+        OPTIONS (schema_name 'foreign_tables_schema_mx', table_name 'table_name_drop');
+SELECT citus_add_local_table_to_metadata('foreign_table_name_drop_fails');
+ citus_add_local_table_to_metadata
+---------------------------------------------------------------------
+
+(1 row)
+
+-- table_name option is already added
+ALTER FOREIGN TABLE foreign_table_name_drop_fails OPTIONS (ADD table_name 'table_name_drop');
+ERROR:  option "table_name" provided more than once
+-- throw error if user tries to drop table_name option from a foreign table inside metadata
+ALTER FOREIGN TABLE foreign_table_name_drop_fails OPTIONS (DROP table_name);
+ERROR:  alter foreign table alter options (drop table_name) command is not allowed for Citus tables
+-- case sensitive option name
+ALTER FOREIGN TABLE foreign_table_name_drop_fails OPTIONS (DROP Table_Name);
+ERROR:  alter foreign table alter options (drop table_name) command is not allowed for Citus tables
+-- other options are allowed to drop
+ALTER FOREIGN TABLE foreign_table_name_drop_fails OPTIONS (DROP schema_name);
+CREATE FOREIGN TABLE foreign_table_name_drop (
+        id INT
+)
+        SERVER foreign_server_local
+        OPTIONS (schema_name 'foreign_tables_schema_mx', table_name 'table_name_drop');
+-- user can drop table_option if foreign table is not in metadata
+ALTER FOREIGN TABLE foreign_table_name_drop OPTIONS (DROP table_name);
+-- we should not intercept data wrappers other than postgres_fdw
+CREATE EXTENSION file_fdw;
+-- remove validator method to add table_name option; otherwise, table_name option is not allowed
+SELECT result FROM run_command_on_all_nodes('ALTER FOREIGN DATA WRAPPER file_fdw NO VALIDATOR');
+           result
+---------------------------------------------------------------------
+ ALTER FOREIGN DATA WRAPPER
+ ALTER FOREIGN DATA WRAPPER
+ ALTER FOREIGN DATA WRAPPER
+(3 rows)
+
+CREATE SERVER citustest FOREIGN DATA WRAPPER file_fdw;
+\copy (select i from generate_series(0,100)i) to '/tmp/test_file_fdw.data';
+CREATE FOREIGN TABLE citustest_filefdw (
+        data text
+)
+        SERVER citustest
+        OPTIONS ( filename '/tmp/test_file_fdw.data');
+-- add non-postgres_fdw table into metadata even if it does not have table_name option
+SELECT citus_add_local_table_to_metadata('citustest_filefdw');
+ citus_add_local_table_to_metadata
+---------------------------------------------------------------------
+
+(1 row)
+
+ALTER FOREIGN TABLE citustest_filefdw OPTIONS (ADD table_name 'unused_table_name_option');
+-- drop table_name option of non-postgres_fdw table even if it is inside metadata
+ALTER FOREIGN TABLE citustest_filefdw OPTIONS (DROP table_name);
 -- cleanup at exit
 set client_min_messages to error;
 DROP SCHEMA foreign_tables_schema_mx CASCADE;

src/test/regress/sql/foreign_tables_mx.sql

@@ -247,6 +247,62 @@ SELECT undistribute_table('foreign_table_local_fails');
 
 DROP FOREIGN TABLE foreign_table_local;
 
+-- disallow dropping table_name when foreign table is in metadata
+CREATE TABLE table_name_drop(id int);
+CREATE FOREIGN TABLE foreign_table_name_drop_fails (
+        id INT
+)
+        SERVER foreign_server_local
+        OPTIONS (schema_name 'foreign_tables_schema_mx', table_name 'table_name_drop');
+
+SELECT citus_add_local_table_to_metadata('foreign_table_name_drop_fails');
+
+-- table_name option is already added
+ALTER FOREIGN TABLE foreign_table_name_drop_fails OPTIONS (ADD table_name 'table_name_drop');
+
+-- throw error if user tries to drop table_name option from a foreign table inside metadata
+ALTER FOREIGN TABLE foreign_table_name_drop_fails OPTIONS (DROP table_name);
+
+-- case sensitive option name
+ALTER FOREIGN TABLE foreign_table_name_drop_fails OPTIONS (DROP Table_Name);
+
+-- other options are allowed to drop
+ALTER FOREIGN TABLE foreign_table_name_drop_fails OPTIONS (DROP schema_name);
+
+CREATE FOREIGN TABLE foreign_table_name_drop (
+        id INT
+)
+        SERVER foreign_server_local
+        OPTIONS (schema_name 'foreign_tables_schema_mx', table_name 'table_name_drop');
+
+-- user can drop table_option if foreign table is not in metadata
+ALTER FOREIGN TABLE foreign_table_name_drop OPTIONS (DROP table_name);
+
+-- we should not intercept data wrappers other than postgres_fdw
+CREATE EXTENSION file_fdw;
+
+-- remove validator method to add table_name option; otherwise, table_name option is not allowed
+SELECT result FROM run_command_on_all_nodes('ALTER FOREIGN DATA WRAPPER file_fdw NO VALIDATOR');
+
+CREATE SERVER citustest FOREIGN DATA WRAPPER file_fdw;
+
+\copy (select i from generate_series(0,100)i) to '/tmp/test_file_fdw.data';
+CREATE FOREIGN TABLE citustest_filefdw (
+        data text
+)
+        SERVER citustest
+        OPTIONS ( filename '/tmp/test_file_fdw.data');
+
+
+-- add non-postgres_fdw table into metadata even if it does not have table_name option
+SELECT citus_add_local_table_to_metadata('citustest_filefdw');
+
+ALTER FOREIGN TABLE citustest_filefdw OPTIONS (ADD table_name 'unused_table_name_option');
+
+-- drop table_name option of non-postgres_fdw table even if it is inside metadata
+ALTER FOREIGN TABLE citustest_filefdw OPTIONS (DROP table_name);
+
+
 -- cleanup at exit
 set client_min_messages to error;
 DROP SCHEMA foreign_tables_schema_mx CASCADE;