From 3c33c51d5b87beb5daa0c4104628622c0393bcf7 Mon Sep 17 00:00:00 2001 From: Ayush Vatsa Date: Mon, 12 Aug 2024 01:28:13 +0530 Subject: [PATCH v2 1/2] Enhance ALTER statement with extended support for APPEND and REMOVE actions to modify existing options --- .../postgres_fdw/expected/postgres_fdw.out | 10 +++ contrib/postgres_fdw/sql/postgres_fdw.sql | 3 + src/backend/commands/foreigncmds.c | 85 +++++++++++++++++-- src/backend/parser/gram.y | 18 +++- src/common/stringinfo.c | 10 +++ src/include/lib/stringinfo.h | 8 ++ src/include/nodes/parsenodes.h | 9 +- src/include/parser/kwlist.h | 2 + 8 files changed, 132 insertions(+), 13 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 212434711e..85281cf6f8 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -178,10 +178,20 @@ ALTER SERVER testserver1 OPTIONS ( -- Error, invalid list syntax ALTER SERVER testserver1 OPTIONS (ADD extensions 'foo; bar'); ERROR: parameter "extensions" must be a list of extension names +ALTER SERVER testserver1 OPTIONS (APPEND extensions 'foo; bar'); +ERROR: parameter "extensions" must be a list of extension names -- OK but gets a warning ALTER SERVER testserver1 OPTIONS (ADD extensions 'foo, bar'); WARNING: extension "foo" is not installed WARNING: extension "bar" is not installed +ALTER SERVER testserver1 OPTIONS (APPEND extensions 'ext1, ext2'); +WARNING: extension "foo" is not installed +WARNING: extension "bar" is not installed +WARNING: extension "ext1" is not installed +WARNING: extension "ext2" is not installed +ALTER SERVER testserver1 OPTIONS (REMOVE extensions 'foo, ext2, ext3'); +WARNING: extension "bar" is not installed +WARNING: extension "ext1" is not installed ALTER SERVER testserver1 OPTIONS (DROP extensions); ALTER USER MAPPING FOR public SERVER testserver1 OPTIONS (DROP user, DROP password); diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 371e131933..8a5ef92fd4 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -192,9 +192,12 @@ ALTER SERVER testserver1 OPTIONS ( -- Error, invalid list syntax ALTER SERVER testserver1 OPTIONS (ADD extensions 'foo; bar'); +ALTER SERVER testserver1 OPTIONS (APPEND extensions 'foo; bar'); -- OK but gets a warning ALTER SERVER testserver1 OPTIONS (ADD extensions 'foo, bar'); +ALTER SERVER testserver1 OPTIONS (APPEND extensions 'ext1, ext2'); +ALTER SERVER testserver1 OPTIONS (REMOVE extensions 'foo, ext2, ext3'); ALTER SERVER testserver1 OPTIONS (DROP extensions); ALTER USER MAPPING FOR public SERVER testserver1 diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c index cf61bbac1f..8df8774339 100644 --- a/src/backend/commands/foreigncmds.c +++ b/src/backend/commands/foreigncmds.c @@ -38,6 +38,7 @@ #include "utils/lsyscache.h" #include "utils/rel.h" #include "utils/syscache.h" +#include "utils/varlena.h" typedef struct @@ -95,10 +96,10 @@ optionListToArray(List *options) /* * Transform a list of DefElem into text array format. This is substantially - * the same thing as optionListToArray(), except we recognize SET/ADD/DROP - * actions for modifying an existing list of options, which is passed in - * Datum form as oldOptions. Also, if fdwvalidator isn't InvalidOid - * it specifies a validator function to call on the result. + * the same thing as optionListToArray(), except we recognize + * SET/ADD/DROP/APPEND/REMOVE actions for modifying an existing list of + * options, which is passed in Datum form as oldOptions. Also, if fdwvalidator + * isn't InvalidOid it specifies a validator function to call on the result. * * Returns the array in the form of a Datum, or PointerGetDatum(NULL) * if the list is empty. @@ -134,10 +135,10 @@ transformGenericOptions(Oid catalogId, } /* - * It is possible to perform multiple SET/DROP actions on the same - * option. The standard permits this, as long as the options to be - * added are unique. Note that an unspecified action is taken to be - * ADD. + * It is possible to perform multiple SET/DROP/APPEND/REMOVE actions + * on the same option. The standard permits this, as long as the + * options to be added are unique. Note that an unspecified action is + * taken to be ADD. */ switch (od->defaction) { @@ -159,6 +160,74 @@ transformGenericOptions(Oid catalogId, lfirst(cell) = od; break; + case DEFELEM_APPEND: + /* Should act like action ADD */ + if (!cell) + resultOptions = lappend(resultOptions, od); + else + { + StringInfo str = makeStringInfo(); + + appendStringInfoString(str, defGetString(lfirst(cell))); + + if (strlen(str->data) && strlen(defGetString(od))) + appendStringInfoChar(str, ','); + + appendStringInfoString(str, defGetString(od)); + + strVal(od->arg) = str->data; + lfirst(cell) = od; + } + break; + + case DEFELEM_REMOVE: + if (!cell) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("option \"%s\" not found", + od->defname))); + else + { + StringInfo str; + List *currentValues; + List *discardValues; + ListCell *cv; + ListCell *dv; + + SplitIdentifierString(defGetString(lfirst(cell)), ',', ¤tValues); + SplitIdentifierString(defGetString(od), ',', &discardValues); + + foreach(cv, currentValues) + { + foreach(dv, discardValues) + { + if (strcmp((char *) lfirst(cv), (char *) lfirst(dv)) == 0) + { + currentValues = foreach_delete_current(currentValues, cv); + break; + } + } + } + + str = makeStringInfo(); + + foreach(cv, currentValues) + { + appendStringInfoString(str, (char *) lfirst(cv)); + appendStringInfoChar(str, ','); + } + + /* Removing last added , */ + trimLastNCharsStringInfo(str, 1); + + list_free(currentValues); + list_free(discardValues); + + strVal(od->arg) = str->data; + lfirst(cell) = od; + } + break; + case DEFELEM_ADD: case DEFELEM_UNSPEC: if (cell) diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index a043fd4c66..8f55863944 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -706,7 +706,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); /* ordinary key words in alphabetical order */ %token ABORT_P ABSENT ABSOLUTE_P ACCESS ACTION ADD_P ADMIN AFTER - AGGREGATE ALL ALSO ALTER ALWAYS ANALYSE ANALYZE AND ANY ARRAY AS ASC + AGGREGATE ALL ALSO ALTER ALWAYS ANALYSE ANALYZE AND ANY APPEND ARRAY AS ASC ASENSITIVE ASSERTION ASSIGNMENT ASYMMETRIC ATOMIC AT ATTACH ATTRIBUTE AUTHORIZATION BACKWARD BEFORE BEGIN_P BETWEEN BIGINT BINARY BIT @@ -771,7 +771,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); QUOTE QUOTES RANGE READ REAL REASSIGN RECHECK RECURSIVE REF_P REFERENCES REFERENCING - REFRESH REINDEX RELATIVE_P RELEASE RENAME REPEATABLE REPLACE REPLICA + REFRESH REINDEX RELATIVE_P RELEASE REMOVE RENAME REPEATABLE REPLACE REPLICA RESET RESTART RESTRICT RETURN RETURNING RETURNS REVOKE RIGHT ROLE ROLLBACK ROLLUP ROUTINE ROUTINES ROW ROWS RULE @@ -5485,6 +5485,16 @@ alter_generic_option_elem: { $$ = makeDefElemExtended(NULL, $2, NULL, DEFELEM_DROP, @2); } + | APPEND generic_option_elem + { + $$ = $2; + $$->defaction = DEFELEM_APPEND; + } + | REMOVE generic_option_elem + { + $$ = $2; + $$->defaction = DEFELEM_REMOVE; + } ; generic_option_elem: @@ -17576,6 +17586,7 @@ unreserved_keyword: | ALSO | ALTER | ALWAYS + | APPEND | ASENSITIVE | ASSERTION | ASSIGNMENT @@ -17792,6 +17803,7 @@ unreserved_keyword: | REINDEX | RELATIVE_P | RELEASE + | REMOVE | RENAME | REPEATABLE | REPLACE @@ -18121,6 +18133,7 @@ bare_label_keyword: | ANALYZE | AND | ANY + | APPEND | ASC | ASENSITIVE | ASSERTION @@ -18423,6 +18436,7 @@ bare_label_keyword: | REINDEX | RELATIVE_P | RELEASE + | REMOVE | RENAME | REPEATABLE | REPLACE diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c index eb9d6502fc..93d403a7af 100644 --- a/src/common/stringinfo.c +++ b/src/common/stringinfo.c @@ -85,6 +85,16 @@ resetStringInfo(StringInfo str) str->cursor = 0; } +void +trimLastNCharsStringInfo(StringInfo str, const int n) +{ + /* don't allow trimming of read-only StringInfos */ + Assert(str->maxlen != 0); + + str->len = (str->len >= n) ? (str->len - n) : 0; + str->data[str->len] = '\0'; +} + /* * appendStringInfo * diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h index cd9632e3fc..ac7e62033c 100644 --- a/src/include/lib/stringinfo.h +++ b/src/include/lib/stringinfo.h @@ -162,6 +162,14 @@ initStringInfoFromString(StringInfo str, char *data, int len) */ extern void resetStringInfo(StringInfo str); +/*------------------------ + * trimLastNCharsStringInfo + * Clears the last 'n' characters of the StringInfo. If the + * length of the StringInfo is less than 'n', the entire StringInfo + * is cleared. The StringInfo object remains valid after this operation. + */ +extern void trimLastNCharsStringInfo(StringInfo str, const int n); + /*------------------------ * appendStringInfo * Format text data under the control of fmt (an sprintf-style format string) diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 85a62b538e..d928dacd62 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -794,8 +794,8 @@ typedef struct IndexElem * DefElem - a generic "name = value" option definition * * In some contexts the name can be qualified. Also, certain SQL commands - * allow a SET/ADD/DROP action to be attached to option settings, so it's - * convenient to carry a field for that too. (Note: currently, it is our + * allow a SET/ADD/DROP/APPEND/REMOVE action to be attached to option settings, + * so it's convenient to carry a field for that too. (Note: currently, it is our * practice that the grammar allows namespace and action only in statements * where they are relevant; C code can just ignore those fields in other * statements.) @@ -806,6 +806,8 @@ typedef enum DefElemAction DEFELEM_SET, DEFELEM_ADD, DEFELEM_DROP, + DEFELEM_APPEND, + DEFELEM_REMOVE } DefElemAction; typedef struct DefElem @@ -815,7 +817,8 @@ typedef struct DefElem char *defname; Node *arg; /* typically Integer, Float, String, or * TypeName */ - DefElemAction defaction; /* unspecified action, or SET/ADD/DROP */ + DefElemAction defaction; /* unspecified action, or + * SET/ADD/DROP/APPEND/REMOVE */ ParseLoc location; /* token location, or -1 if unknown */ } DefElem; diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h index f7fe834cf4..c7c465e1e5 100644 --- a/src/include/parser/kwlist.h +++ b/src/include/parser/kwlist.h @@ -42,6 +42,7 @@ PG_KEYWORD("analyse", ANALYSE, RESERVED_KEYWORD, BARE_LABEL) /* British spellin PG_KEYWORD("analyze", ANALYZE, RESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("and", AND, RESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("any", ANY, RESERVED_KEYWORD, BARE_LABEL) +PG_KEYWORD("append", APPEND, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("array", ARRAY, RESERVED_KEYWORD, AS_LABEL) PG_KEYWORD("as", AS, RESERVED_KEYWORD, AS_LABEL) PG_KEYWORD("asc", ASC, RESERVED_KEYWORD, BARE_LABEL) @@ -372,6 +373,7 @@ PG_KEYWORD("refresh", REFRESH, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("reindex", REINDEX, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("relative", RELATIVE_P, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("release", RELEASE, UNRESERVED_KEYWORD, BARE_LABEL) +PG_KEYWORD("remove", REMOVE, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("rename", RENAME, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("repeatable", REPEATABLE, UNRESERVED_KEYWORD, BARE_LABEL) PG_KEYWORD("replace", REPLACE, UNRESERVED_KEYWORD, BARE_LABEL) -- 2.41.0