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 f07782cf0..1dd3fa2b2 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 @@ -517,7 +517,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); @@ -576,6 +576,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. diff --git a/src/backend/distributed/commands/table.c b/src/backend/distributed/commands/table.c index 8b00b70b3..230a99215 100644 --- a/src/backend/distributed/commands/table.c +++ b/src/backend/distributed/commands/table.c @@ -40,6 +40,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" @@ -117,6 +118,8 @@ static char * GetAlterColumnWithNextvalDefaultCmd(Oid sequenceOid, Oid relationI char *colname); static char * GetAddColumnWithNextvalDefaultCmd(Oid sequenceOid, Oid relationId, char *colname, TypeName *typeName); +static void ErrorIfAlterTableDropTableNameFromPostgresFdw(List *optionList, Oid + relationId); /* @@ -2607,6 +2610,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. @@ -2995,6 +3034,8 @@ ErrorIfUnsupportedAlterTableStmt(AlterTableStmt *alterTableStatement) { if (IsForeignTable(relationId)) { + List *optionList = (List *) command->def; + ErrorIfAlterTableDropTableNameFromPostgresFdw(optionList, relationId); break; } } diff --git a/src/include/distributed/metadata_utility.h b/src/include/distributed/metadata_utility.h index e5b0877d7..ac039ce81 100644 --- a/src/include/distributed/metadata_utility.h +++ b/src/include/distributed/metadata_utility.h @@ -285,6 +285,8 @@ extern bool RegularTable(Oid relationId); extern bool TableEmpty(Oid tableId); extern bool IsForeignTable(Oid relationId); extern bool RelationUsesIdentityColumns(TupleDesc relationDesc); +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); diff --git a/src/test/regress/expected/foreign_tables_mx.out b/src/test/regress/expected/foreign_tables_mx.out index b2574432c..4bcddac5a 100644 --- a/src/test/regress/expected/foreign_tables_mx.out +++ b/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; diff --git a/src/test/regress/sql/foreign_tables_mx.sql b/src/test/regress/sql/foreign_tables_mx.sql index 7b6784aab..eec5b7316 100644 --- a/src/test/regress/sql/foreign_tables_mx.sql +++ b/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;