From 99876709cc6b189f60e649632b3cb6a2151d688c Mon Sep 17 00:00:00 2001 From: Burak Velioglu Date: Wed, 20 Apr 2022 00:31:40 +0300 Subject: [PATCH] Self review --- src/backend/distributed/commands/view.c | 36 ++++++++++++------- .../alter_table_set_access_method.out | 6 ---- .../sql/alter_table_set_access_method.sql | 7 ---- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/src/backend/distributed/commands/view.c b/src/backend/distributed/commands/view.c index 30a5b103f..46c7a6a04 100644 --- a/src/backend/distributed/commands/view.c +++ b/src/backend/distributed/commands/view.c @@ -38,6 +38,8 @@ static List * FilterNameListForDistributedViews(List *viewNamesList, bool missing_ok); static void AppendAliasesToCreateViewCommandForExistingView(StringInfo createViewCommand, Oid viewOid); +static void AddOptionsToCreateViewCommandForExistingView(StringInfo createViewCommand, + Oid viewOid); /* * PreprocessViewStmt is called during the planning phase for CREATE OR REPLACE VIEW @@ -69,7 +71,8 @@ PreprocessViewStmt(Node *node, const char *queryString, * * If view depends on any undistributable object, Citus can not distribute it. In order to * not to prevent users from creating local views on the coordinator WARNING message will - * be sent to the customer about the case instead of erroring out. + * be sent to the customer about the case instead of erroring out. If no worker nodes exist + * at all, view will be created locally without any WARNING message. * * Besides creating the plan we also make sure all (new) dependencies of the view are * created on all nodes. @@ -124,8 +127,7 @@ PostprocessViewStmt(Node *node, const char *queryString) /* * ViewStmtObjectAddress returns the ObjectAddress for the subject of the - * CREATE [OR REPLACE] VIEW statement. If missing_ok is false it will error with the - * normal postgres error for unfound views. + * CREATE [OR REPLACE] VIEW statement. */ ObjectAddress ViewStmtObjectAddress(Node *node, bool missing_ok) @@ -257,16 +259,7 @@ CreateViewDDLCommandsIdempotent(const ObjectAddress *viewAddress) AddQualifiedViewNameToCreateViewCommand(createViewCommand, viewOid); AppendAliasesToCreateViewCommandForExistingView(createViewCommand, viewOid); - - /* Add rel options to create view command */ - char *relOptions = flatten_reloptions(viewOid); - if (relOptions != NULL) - { - appendStringInfo(createViewCommand, "WITH (%s) ", relOptions); - pfree(relOptions); - } - - /* Add view definition to create view command */ + AddOptionsToCreateViewCommandForExistingView(createViewCommand, viewOid); AddViewDefinitionToCreateViewCommand(createViewCommand, viewOid); /* Add alter owner commmand */ @@ -337,3 +330,20 @@ AppendAliasesToCreateViewCommandForExistingView(StringInfo createViewCommand, Oi index_close(mapidx, AccessShareLock); table_close(maprel, AccessShareLock); } + + +/* + * AddOptionsToCreateViewCommandForExistingView add relation options to create view command + * for an existing view + */ +static void +AddOptionsToCreateViewCommandForExistingView(StringInfo createViewCommand, Oid viewOid) +{ + /* Add rel options to create view command */ + char *relOptions = flatten_reloptions(viewOid); + if (relOptions != NULL) + { + appendStringInfo(createViewCommand, "WITH (%s) ", relOptions); + pfree(relOptions); + } +} diff --git a/src/test/regress/expected/alter_table_set_access_method.out b/src/test/regress/expected/alter_table_set_access_method.out index ccd3a98f5..0023721f9 100644 --- a/src/test/regress/expected/alter_table_set_access_method.out +++ b/src/test/regress/expected/alter_table_set_access_method.out @@ -633,8 +633,6 @@ NOTICE: renaming the new table to alter_table_set_access_method.ref (1 row) --- (TODO: CHECK) Since there is a dependent view, we must use sequential mode -SET citus.multi_shard_modify_mode TO 'sequential'; select alter_table_set_access_method('dist','columnar'); NOTICE: creating a new table for alter_table_set_access_method.dist NOTICE: moving the data of alter_table_set_access_method.dist @@ -663,7 +661,6 @@ NOTICE: renaming the new table to alter_table_set_access_method.dist (1 row) -RESET citus.multi_shard_modify_mode; select alter_table_set_access_method('local','heap'); NOTICE: creating a new table for alter_table_set_access_method.local NOTICE: moving the data of alter_table_set_access_method.local @@ -699,8 +696,6 @@ NOTICE: renaming the new table to alter_table_set_access_method.ref (1 row) --- (TODO: CHECK) Since there is a dependent view, we must use sequential mode -SET citus.multi_shard_modify_mode TO 'sequential'; select alter_table_set_access_method('dist','heap'); NOTICE: creating a new table for alter_table_set_access_method.dist NOTICE: moving the data of alter_table_set_access_method.dist @@ -715,7 +710,6 @@ NOTICE: renaming the new table to alter_table_set_access_method.dist (1 row) -RESET citus.multi_shard_modify_mode; SELECT * FROM m_local; a | b | c --------------------------------------------------------------------- diff --git a/src/test/regress/sql/alter_table_set_access_method.sql b/src/test/regress/sql/alter_table_set_access_method.sql index 0ff88cfd8..306673558 100644 --- a/src/test/regress/sql/alter_table_set_access_method.sql +++ b/src/test/regress/sql/alter_table_set_access_method.sql @@ -223,19 +223,12 @@ create view v_dist as select * from dist; select alter_table_set_access_method('local','columnar'); select alter_table_set_access_method('ref','columnar'); --- (TODO: CHECK) Since there is a dependent view, we must use sequential mode -SET citus.multi_shard_modify_mode TO 'sequential'; select alter_table_set_access_method('dist','columnar'); SELECT alter_distributed_table('dist', shard_count:=1, cascade_to_colocated:=false); -RESET citus.multi_shard_modify_mode; select alter_table_set_access_method('local','heap'); select alter_table_set_access_method('ref','heap'); - --- (TODO: CHECK) Since there is a dependent view, we must use sequential mode -SET citus.multi_shard_modify_mode TO 'sequential'; select alter_table_set_access_method('dist','heap'); -RESET citus.multi_shard_modify_mode; SELECT * FROM m_local; SELECT * FROM m_ref;