Fixes review comments

pull/7253/head
gurkanindibay 2023-12-25 18:08:10 +03:00
parent 597154985e
commit 09a7a336d1
4 changed files with 45 additions and 50 deletions

View File

@ -193,17 +193,8 @@ PreprocessGrantOnDatabaseStmt(Node *node, const char *queryString,
/* /*
* IsSetTablespaceStatement checks if the provided ALTER DATABASE statement is a SET TABLESPACE statement. * IsSetTablespaceStatement returns true if the statement is a SET TABLESPACE statement,
* * false otherwise.
* This function takes a Node pointer representing a AlterDatabaseStmt, and checks
* if it is a SET TABLESPACE statement, which is used to move a table to a new
* tablespace.
*
* Parameters:
* stmt: A pointer to a Node representing AlterDatabaseStmt.
*
* Returns:
* true if the statement is a SET TABLESPACE statement, false otherwise.
*/ */
static bool static bool
IsSetTablespaceStatement(AlterDatabaseStmt *stmt) IsSetTablespaceStatement(AlterDatabaseStmt *stmt)

View File

@ -124,17 +124,27 @@ AppendGrantOnDatabaseStmt(StringInfo buf, GrantStmt *stmt)
static void static void
AppendAlterDatabaseStmt(StringInfo buf, AlterDatabaseStmt *stmt) AppendAlterDatabaseStmt(StringInfo buf, AlterDatabaseStmt *stmt)
{ {
if (list_length(stmt->options) == 0)
{
elog(ERROR, "got unexpected number of options for ALTER DATABASE");
}
if (stmt->options) if (stmt->options)
{ {
DefElem *firstOption = linitial(stmt->options); DefElem *firstOption = linitial(stmt->options);
if (strcmp(firstOption->defname, "tablespace") == 0) if (strcmp(firstOption->defname, "tablespace") == 0)
{ {
AppendAlterDatabaseSetTablespace(buf, firstOption, stmt->dbname); AppendAlterDatabaseSetTablespace(buf, firstOption, stmt->dbname);
/* SET tablespace cannot be combined with other options */
return;
} }
else
{
AppendBasicAlterDatabaseOptions(buf, stmt); appendStringInfo(buf, "ALTER DATABASE %s WITH",
} quote_identifier(stmt->dbname));
AppendBasicAlterDatabaseOptions(buf, stmt);
} }
appendStringInfo(buf, ";"); appendStringInfo(buf, ";");
@ -154,7 +164,7 @@ AppendAlterDatabaseSetTablespace(StringInfo buf, DefElem *def, char *dbname)
* AppendBasicAlterDatabaseOptions appends basic ALTER DATABASE options to a string buffer. * AppendBasicAlterDatabaseOptions appends basic ALTER DATABASE options to a string buffer.
* Basic options are those that can be appended to the ALTER DATABASE statement * Basic options are those that can be appended to the ALTER DATABASE statement
* after the "WITH" keyword.(i.e. ALLOW_CONNECTIONS, CONNECTION LIMIT, IS_TEMPLATE) * after the "WITH" keyword.(i.e. ALLOW_CONNECTIONS, CONNECTION LIMIT, IS_TEMPLATE)
* The tablespace option is not a basic option since it is defined with SET option. * For example, the tablespace option is not a basic option since it is defined via SET keyword.
* *
* This function takes a string buffer and an AlterDatabaseStmt as input. * This function takes a string buffer and an AlterDatabaseStmt as input.
* It appends the basic options to the string buffer. * It appends the basic options to the string buffer.
@ -163,17 +173,9 @@ AppendAlterDatabaseSetTablespace(StringInfo buf, DefElem *def, char *dbname)
static void static void
AppendBasicAlterDatabaseOptions(StringInfo buf, AlterDatabaseStmt *stmt) AppendBasicAlterDatabaseOptions(StringInfo buf, AlterDatabaseStmt *stmt)
{ {
ListCell *cell = NULL; DefElem *def = NULL;
bool prefixAppendedForBasicOptions = false; foreach_ptr(def, stmt->options)
foreach(cell, stmt->options)
{ {
DefElem *def = castNode(DefElem, lfirst(cell));
if (!prefixAppendedForBasicOptions)
{
appendStringInfo(buf, "ALTER DATABASE %s WITH", quote_identifier(
stmt->dbname));
prefixAppendedForBasicOptions = true;
}
DefElemOptionToStatement(buf, def, alterDatabaseOptionFormats, lengthof( DefElemOptionToStatement(buf, def, alterDatabaseOptionFormats, lengthof(
alterDatabaseOptionFormats)); alterDatabaseOptionFormats));
} }

View File

@ -140,21 +140,21 @@ DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
NOTICE: issuing ALTER DATABASE regression RESET lock_timeout NOTICE: issuing ALTER DATABASE regression RESET lock_timeout
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
set citus.enable_create_database_propagation=on; set citus.enable_create_database_propagation=on;
create database regression2; create database "regression!'2";
alter database regression2 with CONNECTION LIMIT 100; alter database "regression!'2" with CONNECTION LIMIT 100;
NOTICE: issuing ALTER DATABASE regression2 WITH CONNECTION LIMIT 100; NOTICE: issuing ALTER DATABASE "regression!'2" WITH CONNECTION LIMIT 100;
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
NOTICE: issuing ALTER DATABASE regression2 WITH CONNECTION LIMIT 100; NOTICE: issuing ALTER DATABASE "regression!'2" WITH CONNECTION LIMIT 100;
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
alter database regression2 with IS_TEMPLATE true CONNECTION LIMIT 50; alter database "regression!'2" with IS_TEMPLATE true CONNECTION LIMIT 50;
NOTICE: issuing ALTER DATABASE regression2 WITH IS_TEMPLATE true CONNECTION LIMIT 50; NOTICE: issuing ALTER DATABASE "regression!'2" WITH IS_TEMPLATE true CONNECTION LIMIT 50;
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
NOTICE: issuing ALTER DATABASE regression2 WITH IS_TEMPLATE true CONNECTION LIMIT 50; NOTICE: issuing ALTER DATABASE "regression!'2" WITH IS_TEMPLATE true CONNECTION LIMIT 50;
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
alter database regression2 with IS_TEMPLATE false; alter database "regression!'2" with IS_TEMPLATE false;
NOTICE: issuing ALTER DATABASE regression2 WITH IS_TEMPLATE false; NOTICE: issuing ALTER DATABASE "regression!'2" WITH IS_TEMPLATE false;
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
NOTICE: issuing ALTER DATABASE regression2 WITH IS_TEMPLATE false; NOTICE: issuing ALTER DATABASE "regression!'2" WITH IS_TEMPLATE false;
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
\set alter_db_tablespace :abs_srcdir '/tmp_check/ts3' \set alter_db_tablespace :abs_srcdir '/tmp_check/ts3'
CREATE TABLESPACE alter_db_tablespace LOCATION :'alter_db_tablespace'; CREATE TABLESPACE alter_db_tablespace LOCATION :'alter_db_tablespace';
@ -167,16 +167,16 @@ CREATE TABLESPACE alter_db_tablespace LOCATION :'alter_db_tablespace';
\c - - - :master_port \c - - - :master_port
set citus.log_remote_commands = true; set citus.log_remote_commands = true;
set citus.grep_remote_commands = '%ALTER DATABASE%'; set citus.grep_remote_commands = '%ALTER DATABASE%';
alter database regression2 set TABLESPACE alter_db_tablespace; alter database "regression!'2" set TABLESPACE alter_db_tablespace;
NOTICE: issuing ALTER DATABASE regression2 SET TABLESPACE alter_db_tablespace; NOTICE: issuing ALTER DATABASE "regression!'2" SET TABLESPACE alter_db_tablespace
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
NOTICE: issuing ALTER DATABASE regression2 SET TABLESPACE alter_db_tablespace; NOTICE: issuing ALTER DATABASE "regression!'2" SET TABLESPACE alter_db_tablespace
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
set citus.enable_create_database_propagation=on; set citus.enable_create_database_propagation=on;
alter database regression2 rename to regression3; alter database "regression!'2" rename to regression3;
NOTICE: issuing ALTER DATABASE regression2 RENAME TO regression3 NOTICE: issuing ALTER DATABASE "regression!'2" RENAME TO regression3
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
NOTICE: issuing ALTER DATABASE regression2 RENAME TO regression3 NOTICE: issuing ALTER DATABASE "regression!'2" RENAME TO regression3
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
-- check that the local database rename and alter comnmand is not propagated -- check that the local database rename and alter comnmand is not propagated
set citus.enable_create_database_propagation=off; set citus.enable_create_database_propagation=off;
@ -203,9 +203,9 @@ SELECT result FROM run_command_on_all_nodes(
(3 rows) (3 rows)
alter database regression4 set TABLESPACE "ts-needs\!escape"; alter database regression4 set TABLESPACE "ts-needs\!escape";
NOTICE: issuing ALTER DATABASE regression4 SET TABLESPACE "ts-needs\!escape"; NOTICE: issuing ALTER DATABASE regression4 SET TABLESPACE "ts-needs\!escape"
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
NOTICE: issuing ALTER DATABASE regression4 SET TABLESPACE "ts-needs\!escape"; NOTICE: issuing ALTER DATABASE regression4 SET TABLESPACE "ts-needs\!escape"
DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx
drop database regression4; drop database regression4;
set citus.log_remote_commands = false; set citus.log_remote_commands = false;

View File

@ -49,10 +49,12 @@ alter database regression set lock_timeout to DEFAULT;
alter database regression RESET lock_timeout; alter database regression RESET lock_timeout;
set citus.enable_create_database_propagation=on; set citus.enable_create_database_propagation=on;
create database regression2; create database "regression!'2";
alter database regression2 with CONNECTION LIMIT 100; alter database "regression!'2" with CONNECTION LIMIT 100;
alter database regression2 with IS_TEMPLATE true CONNECTION LIMIT 50; alter database "regression!'2" with IS_TEMPLATE true CONNECTION LIMIT 50;
alter database regression2 with IS_TEMPLATE false; alter database "regression!'2" with IS_TEMPLATE false;
\set alter_db_tablespace :abs_srcdir '/tmp_check/ts3' \set alter_db_tablespace :abs_srcdir '/tmp_check/ts3'
@ -71,10 +73,10 @@ CREATE TABLESPACE alter_db_tablespace LOCATION :'alter_db_tablespace';
set citus.log_remote_commands = true; set citus.log_remote_commands = true;
set citus.grep_remote_commands = '%ALTER DATABASE%'; set citus.grep_remote_commands = '%ALTER DATABASE%';
alter database regression2 set TABLESPACE alter_db_tablespace; alter database "regression!'2" set TABLESPACE alter_db_tablespace;
set citus.enable_create_database_propagation=on; set citus.enable_create_database_propagation=on;
alter database regression2 rename to regression3; alter database "regression!'2" rename to regression3;
-- check that the local database rename and alter comnmand is not propagated -- check that the local database rename and alter comnmand is not propagated
set citus.enable_create_database_propagation=off; set citus.enable_create_database_propagation=off;