diff --git a/src/backend/distributed/commands/extension.c b/src/backend/distributed/commands/extension.c index 6b47be681..b97dd941d 100644 --- a/src/backend/distributed/commands/extension.c +++ b/src/backend/distributed/commands/extension.c @@ -12,6 +12,7 @@ #include "citus_version.h" #include "catalog/pg_extension_d.h" +#include "commands/defrem.h" #include "commands/extension.h" #include "distributed/citus_ruleutils.h" #include "distributed/commands.h" @@ -101,13 +102,12 @@ ExtractNewExtensionVersion(Node *parseTree) Assert(false); } - Value *newVersionValue = GetExtensionOption(optionsList, "new_version"); + DefElem *newVersionValue = GetExtensionOption(optionsList, "new_version"); /* return target string safely */ if (newVersionValue) { - const char *newVersion = strVal(newVersionValue); - + const char *newVersion = defGetString(newVersionValue); return pstrdup(newVersion); } else @@ -171,6 +171,9 @@ PostprocessCreateExtensionStmt(Node *node, const char *queryString) */ AddSchemaFieldIfMissing(stmt); + /* always send commands with IF NOT EXISTS */ + stmt->if_not_exists = true; + const char *createExtensionStmtSql = DeparseTreeNode(node); /* @@ -201,7 +204,7 @@ AddSchemaFieldIfMissing(CreateExtensionStmt *createExtensionStmt) { List *optionsList = createExtensionStmt->options; - Value *schemaNameValue = GetExtensionOption(optionsList, "schema"); + DefElem *schemaNameValue = GetExtensionOption(optionsList, "schema"); if (!schemaNameValue) { diff --git a/src/backend/distributed/deparser/deparse_extension_stmts.c b/src/backend/distributed/deparser/deparse_extension_stmts.c index 7b393c90e..bb6b15dbd 100644 --- a/src/backend/distributed/deparser/deparse_extension_stmts.c +++ b/src/backend/distributed/deparser/deparse_extension_stmts.c @@ -13,14 +13,17 @@ #include "postgres.h" #include "catalog/namespace.h" +#include "commands/defrem.h" #include "distributed/deparser.h" +#include "distributed/listutils.h" #include "lib/stringinfo.h" -#include "nodes/pg_list.h" #include "nodes/parsenodes.h" +#include "nodes/pg_list.h" #include "utils/builtins.h" /* Local functions forward declarations for helper functions */ static void AppendCreateExtensionStmt(StringInfo buf, CreateExtensionStmt *stmt); +static void AppendCreateExtensionStmtOptions(StringInfo buf, List *options); static void AppendDropExtensionStmt(StringInfo buf, DropStmt *stmt); static void AppendExtensionNameList(StringInfo buf, List *objects); static void AppendAlterExtensionSchemaStmt(StringInfo buf, @@ -30,36 +33,22 @@ static void AppendAlterExtensionStmt(StringInfo buf, /* - * GetExtensionOption returns Value* of DefElem node with "defname" from "options" list + * GetExtensionOption returns DefElem * node with "defname" from "options" list */ -Value * +DefElem * GetExtensionOption(List *extensionOptions, const char *defname) { - Value *targetValue = NULL; - - ListCell *defElemCell = NULL; - - foreach(defElemCell, extensionOptions) + DefElem *defElement = NULL; + foreach_ptr(defElement, extensionOptions) { - DefElem *defElement = (DefElem *) lfirst(defElemCell); - - if (IsA(defElement, DefElem) && strncmp(defElement->defname, defname, - NAMEDATALEN) == 0) + if (IsA(defElement, DefElem) && + strncmp(defElement->defname, defname, NAMEDATALEN) == 0) { - targetValue = (Value *) defElement->arg; - break; + return defElement; } } - /* return target string safely */ - if (targetValue) - { - return targetValue; - } - else - { - return NULL; - } + return NULL; } @@ -86,54 +75,72 @@ DeparseCreateExtensionStmt(Node *node) static void AppendCreateExtensionStmt(StringInfo buf, CreateExtensionStmt *createExtensionStmt) { - List *optionsList = createExtensionStmt->options; + appendStringInfoString(buf, "CREATE EXTENSION "); - const char *extensionName = createExtensionStmt->extname; - extensionName = quote_identifier(extensionName); - - /* - * We fetch "new_version", "schema" and "cascade" options from - * optionList as we will append "IF NOT EXISTS" clause regardless of - * statement's content before propagating it to worker nodes. - * We also do not care old_version for now. - */ - Value *schemaNameValue = GetExtensionOption(optionsList, "schema"); - - /* these can be NULL hence check before fetching the stored value */ - Value *newVersionValue = GetExtensionOption(optionsList, "new_version"); - Value *cascadeValue = GetExtensionOption(optionsList, "cascade"); - - /* - * We do not check for if schemaName is NULL as we append it in deparse - * logic if it is not specified. - */ - const char *schemaName = strVal(schemaNameValue); - schemaName = quote_identifier(schemaName); - - appendStringInfo(buf, "CREATE EXTENSION IF NOT EXISTS %s WITH SCHEMA %s", - extensionName, schemaName); - - /* "new_version" may not be specified in CreateExtensionStmt */ - if (newVersionValue) + if (createExtensionStmt->if_not_exists) { - const char *newVersion = strVal(newVersionValue); - newVersion = quote_identifier(newVersion); - - appendStringInfo(buf, " VERSION %s", newVersion); + appendStringInfoString(buf, "IF NOT EXISTS "); } - /* "cascade" may not be specified in CreateExtensionStmt */ - if (cascadeValue) - { - bool cascade = intVal(cascadeValue); + /* + * Up until here we have been ending the statement in a space, which makes it possible + * to just append the quoted extname. From here onwards we will not have the string + * ending in a space so appends should begin with a whitespace. + */ + appendStringInfoString(buf, quote_identifier(createExtensionStmt->extname)); + AppendCreateExtensionStmtOptions(buf, createExtensionStmt->options); + appendStringInfoString(buf, ";"); +} - if (cascade) + +/* + * AppendCreateExtensionStmtOptions takes the option list of a CreateExtensionStmt and + * loops over the options to add them to the statement we are building. + * + * An error will be thrown if we run into an unsupported option, comparable to how + * postgres gives an error when parsing this list. + */ +static void +AppendCreateExtensionStmtOptions(StringInfo buf, List *options) +{ + if (list_length(options) > 0) + { + /* only append WITH if we actual will add options to the statement */ + appendStringInfoString(buf, " WITH"); + } + + /* Add the options to the statement */ + DefElem *defElem = NULL; + foreach_ptr(defElem, options) + { + if (strcmp(defElem->defname, "schema") == 0) { - appendStringInfoString(buf, " CASCADE"); + const char *schemaName = defGetString(defElem); + appendStringInfo(buf, " SCHEMA %s", quote_identifier(schemaName)); + } + else if (strcmp(defElem->defname, "new_version") == 0) + { + const char *newVersion = defGetString(defElem); + appendStringInfo(buf, " VERSION %s", quote_identifier(newVersion)); + } + else if (strcmp(defElem->defname, "old_version") == 0) + { + const char *oldVersion = defGetString(defElem); + appendStringInfo(buf, " FROM %s", quote_identifier(oldVersion)); + } + else if (strcmp(defElem->defname, "cascade") == 0) + { + bool cascade = defGetBoolean(defElem); + if (cascade) + { + appendStringInfoString(buf, " CASCADE"); + } + } + else + { + elog(ERROR, "unrecognized option: %s", defElem->defname); } } - - appendStringInfoString(buf, " ;"); } @@ -165,17 +172,25 @@ AppendAlterExtensionStmt(StringInfo buf, AlterExtensionStmt *alterExtensionStmt) const char *extensionName = alterExtensionStmt->extname; extensionName = quote_identifier(extensionName); - Value *newVersionValue = GetExtensionOption(optionsList, "new_version"); + appendStringInfo(buf, "ALTER EXTENSION %s UPDATE", extensionName); - appendStringInfo(buf, "ALTER EXTENSION %s UPDATE ", extensionName); - - /* "new_version" may not be specified in AlterExtensionStmt */ - if (newVersionValue) + /* + * Append the options for ALTER EXTENSION ... UPDATE + * Currently there is only 1 option, but this structure follows how postgres parses + * the options. + */ + DefElem *option = NULL; + foreach_ptr(option, optionsList) { - const char *newVersion = strVal(newVersionValue); - newVersion = quote_identifier(newVersion); - - appendStringInfo(buf, " TO %s", newVersion); + if (strcmp(option->defname, "new_version") == 0) + { + const char *newVersion = defGetString(option); + appendStringInfo(buf, " TO %s", quote_identifier(newVersion)); + } + else + { + elog(ERROR, "unrecognized option: %s", option->defname); + } } appendStringInfoString(buf, ";"); diff --git a/src/include/distributed/deparser.h b/src/include/distributed/deparser.h index c016665b0..ca85cf596 100644 --- a/src/include/distributed/deparser.h +++ b/src/include/distributed/deparser.h @@ -97,8 +97,8 @@ extern void QualifyAlterFunctionDependsStmt(Node *stmt); extern char * DeparseAlterRoleStmt(Node *stmt); /* forward declarations for deparse_extension_stmts.c */ -extern Value * GetExtensionOption(List *extensionOptions, - const char *defname); +extern DefElem * GetExtensionOption(List *extensionOptions, + const char *defname); extern char * DeparseCreateExtensionStmt(Node *stmt); extern char * DeparseDropExtensionStmt(Node *stmt); extern char * DeparseAlterExtensionSchemaStmt(Node *stmt); diff --git a/src/test/regress/expected/propagate_extension_commands.out b/src/test/regress/expected/propagate_extension_commands.out index 64166ac65..de6ded967 100644 --- a/src/test/regress/expected/propagate_extension_commands.out +++ b/src/test/regress/expected/propagate_extension_commands.out @@ -182,6 +182,69 @@ SELECT create_reference_table('ref_table_2'); (1 row) +-- we also add an old style extension from before extensions which we upgrade to an extension +-- by exercising it before the add node we verify it will create the extension (without upgrading) +-- it on the new worker as well. For this we use the dict_int extension which is in contrib, +-- supports FROM unpackaged, and is relatively small +-- create objects for dict_int manually so we can upgrade from unpacked +CREATE FUNCTION dintdict_init(internal) RETURNS internal AS 'dict_int.so' LANGUAGE C STRICT; +CREATE FUNCTION dintdict_lexize(internal, internal, internal, internal) RETURNS internal AS 'dict_int.so' LANGUAGE C STRICT; +CREATE TEXT SEARCH TEMPLATE intdict_template (LEXIZE = dintdict_lexize, INIT = dintdict_init ); +CREATE TEXT SEARCH DICTIONARY intdict (TEMPLATE = intdict_template); +COMMENT ON TEXT SEARCH DICTIONARY intdict IS 'dictionary for integers'; +SELECT run_command_on_workers($$ +CREATE FUNCTION dintdict_init(internal) RETURNS internal AS 'dict_int.so' LANGUAGE C STRICT; +$$); + run_command_on_workers +--------------------------------------------------------------------- + (localhost,57637,t,"CREATE FUNCTION") +(1 row) + +SELECT run_command_on_workers($$ +CREATE FUNCTION dintdict_lexize(internal, internal, internal, internal) RETURNS internal AS 'dict_int.so' LANGUAGE C STRICT; +$$); + run_command_on_workers +--------------------------------------------------------------------- + (localhost,57637,t,"CREATE FUNCTION") +(1 row) + +SELECT run_command_on_workers($$ +CREATE TEXT SEARCH TEMPLATE intdict_template (LEXIZE = dintdict_lexize, INIT = dintdict_init ); +$$); + run_command_on_workers +--------------------------------------------------------------------- + (localhost,57637,t,"CREATE TEXT SEARCH TEMPLATE") +(1 row) + +SELECT run_command_on_workers($$ +CREATE TEXT SEARCH DICTIONARY intdict (TEMPLATE = intdict_template); +$$); + run_command_on_workers +--------------------------------------------------------------------- + (localhost,57637,t,"CREATE TEXT SEARCH DICTIONARY") +(1 row) + +SELECT run_command_on_workers($$ +COMMENT ON TEXT SEARCH DICTIONARY intdict IS 'dictionary for integers'; +$$); + run_command_on_workers +--------------------------------------------------------------------- + (localhost,57637,t,COMMENT) +(1 row) + +CREATE EXTENSION dict_int FROM unpackaged; +SELECT run_command_on_workers($$SELECT count(extnamespace) FROM pg_extension WHERE extname = 'dict_int'$$); + run_command_on_workers +--------------------------------------------------------------------- + (localhost,57637,t,1) +(1 row) + +SELECT run_command_on_workers($$SELECT extversion FROM pg_extension WHERE extname = 'dict_int'$$); + run_command_on_workers +--------------------------------------------------------------------- + (localhost,57637,t,1.0) +(1 row) + -- and add the other node SELECT 1 from master_add_node('localhost', :worker_2_port); NOTICE: Replicating reference table "ref_table_2" to the node localhost:xxxxx @@ -205,6 +268,21 @@ SELECT run_command_on_workers($$SELECT extversion FROM pg_extension WHERE extnam (localhost,57638,t,1.3) (2 rows) +-- check for the unpackaged extension to be created correctly +SELECT run_command_on_workers($$SELECT count(extnamespace) FROM pg_extension WHERE extname = 'dict_int'$$); + run_command_on_workers +--------------------------------------------------------------------- + (localhost,57637,t,1) + (localhost,57638,t,1) +(2 rows) + +SELECT run_command_on_workers($$SELECT extversion FROM pg_extension WHERE extname = 'dict_int'$$); + run_command_on_workers +--------------------------------------------------------------------- + (localhost,57637,t,1.0) + (localhost,57638,t,1.0) +(2 rows) + -- and similarly check for the reference table select count(*) from pg_dist_partition where partmethod='n' and logicalrelid='ref_table_2'::regclass; count diff --git a/src/test/regress/sql/propagate_extension_commands.sql b/src/test/regress/sql/propagate_extension_commands.sql index 14b5f862f..f52233a60 100644 --- a/src/test/regress/sql/propagate_extension_commands.sql +++ b/src/test/regress/sql/propagate_extension_commands.sql @@ -110,6 +110,41 @@ SELECT run_command_on_workers($$SELECT extversion FROM pg_extension WHERE extnam CREATE TABLE ref_table_2 (x seg); SELECT create_reference_table('ref_table_2'); +-- we also add an old style extension from before extensions which we upgrade to an extension +-- by exercising it before the add node we verify it will create the extension (without upgrading) +-- it on the new worker as well. For this we use the dict_int extension which is in contrib, +-- supports FROM unpackaged, and is relatively small + +-- create objects for dict_int manually so we can upgrade from unpacked +CREATE FUNCTION dintdict_init(internal) RETURNS internal AS 'dict_int.so' LANGUAGE C STRICT; +CREATE FUNCTION dintdict_lexize(internal, internal, internal, internal) RETURNS internal AS 'dict_int.so' LANGUAGE C STRICT; +CREATE TEXT SEARCH TEMPLATE intdict_template (LEXIZE = dintdict_lexize, INIT = dintdict_init ); +CREATE TEXT SEARCH DICTIONARY intdict (TEMPLATE = intdict_template); +COMMENT ON TEXT SEARCH DICTIONARY intdict IS 'dictionary for integers'; +SELECT run_command_on_workers($$ +CREATE FUNCTION dintdict_init(internal) RETURNS internal AS 'dict_int.so' LANGUAGE C STRICT; +$$); + +SELECT run_command_on_workers($$ +CREATE FUNCTION dintdict_lexize(internal, internal, internal, internal) RETURNS internal AS 'dict_int.so' LANGUAGE C STRICT; +$$); + +SELECT run_command_on_workers($$ +CREATE TEXT SEARCH TEMPLATE intdict_template (LEXIZE = dintdict_lexize, INIT = dintdict_init ); +$$); + +SELECT run_command_on_workers($$ +CREATE TEXT SEARCH DICTIONARY intdict (TEMPLATE = intdict_template); +$$); + +SELECT run_command_on_workers($$ +COMMENT ON TEXT SEARCH DICTIONARY intdict IS 'dictionary for integers'; +$$); + +CREATE EXTENSION dict_int FROM unpackaged; +SELECT run_command_on_workers($$SELECT count(extnamespace) FROM pg_extension WHERE extname = 'dict_int'$$); +SELECT run_command_on_workers($$SELECT extversion FROM pg_extension WHERE extname = 'dict_int'$$); + -- and add the other node SELECT 1 from master_add_node('localhost', :worker_2_port); @@ -117,6 +152,10 @@ SELECT 1 from master_add_node('localhost', :worker_2_port); SELECT run_command_on_workers($$SELECT count(extnamespace) FROM pg_extension WHERE extname = 'seg'$$); SELECT run_command_on_workers($$SELECT extversion FROM pg_extension WHERE extname = 'seg'$$); +-- check for the unpackaged extension to be created correctly +SELECT run_command_on_workers($$SELECT count(extnamespace) FROM pg_extension WHERE extname = 'dict_int'$$); +SELECT run_command_on_workers($$SELECT extversion FROM pg_extension WHERE extname = 'dict_int'$$); + -- and similarly check for the reference table select count(*) from pg_dist_partition where partmethod='n' and logicalrelid='ref_table_2'::regclass; SELECT count(*) FROM pg_dist_shard WHERE logicalrelid='ref_table_2'::regclass;