diff options
author | Michael Paquier <michael@paquier.xyz> | 2021-12-08 12:36:31 +0900 |
---|---|---|
committer | Michael Paquier <michael@paquier.xyz> | 2021-12-08 12:36:31 +0900 |
commit | 00029deaf65aad47044d9290fe80f2f68601f7ac (patch) | |
tree | 1ab54bd86e605e099366148f5ebee0967b6a6bbc /src | |
parent | f99870dd867331f576a84e37438da86a866559c4 (diff) | |
download | postgresql-00029deaf65aad47044d9290fe80f2f68601f7ac.tar.gz postgresql-00029deaf65aad47044d9290fe80f2f68601f7ac.zip |
Improve parsing of options of CREATE/ALTER SUBSCRIPTION
This simplifies the code so as it is not necessary anymore for the
caller of parse_subscription_options() to zero SubOpts, holding a
bitmaps of the provided options as well as the default/parsed option
values. This also simplifies some checks related to the options
supported by a command when checking for incompatibilities.
While on it, the errors generated for unsupported combinations with
"slot_name = NONE" are reordered. This may generate a different errors
compared to the previous major versions, but users have to go through
all those errors to get a correct command in this case when using
incorrect values for options "enabled" and "create\slot", so at the end
the resulting command would remain the same.
Author: Peter Smith
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/CAHut+PtXHfLgLHDDJ8ZN5f5Be_37mJoxpEsRg8LNmm4XCr06Rw@mail.gmail.com
Diffstat (limited to 'src')
-rw-r--r-- | src/backend/commands/subscriptioncmds.c | 73 | ||||
-rw-r--r-- | src/test/regress/expected/subscription.out | 2 | ||||
-rw-r--r-- | src/test/regress/sql/subscription.sql | 2 |
3 files changed, 35 insertions, 42 deletions
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 9427e86fee1..2b658080fee 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -95,8 +95,6 @@ static void ReportSlotConnectionError(List *rstates, Oid subid, char *slotname, * * Since not all options can be specified in both commands, this function * will report an error if mutually exclusive options are specified. - * - * Caller is expected to have cleared 'opts'. */ static void parse_subscription_options(ParseState *pstate, List *stmt_options, @@ -104,6 +102,9 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, { ListCell *lc; + /* Start out with cleared opts. */ + memset(opts, 0, sizeof(SubOpts)); + /* caller must expect some option */ Assert(supported_opts != 0); @@ -262,7 +263,6 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, { /* Check for incompatible options from the user. */ if (opts->enabled && - IsSet(supported_opts, SUBOPT_ENABLED) && IsSet(opts->specified_opts, SUBOPT_ENABLED)) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -271,7 +271,6 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, "connect = false", "enabled = true"))); if (opts->create_slot && - IsSet(supported_opts, SUBOPT_CREATE_SLOT) && IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT)) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -279,7 +278,6 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, "connect = false", "create_slot = true"))); if (opts->copy_data && - IsSet(supported_opts, SUBOPT_COPY_DATA) && IsSet(opts->specified_opts, SUBOPT_COPY_DATA)) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -297,44 +295,39 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, * was used. */ if (!opts->slot_name && - IsSet(supported_opts, SUBOPT_SLOT_NAME) && IsSet(opts->specified_opts, SUBOPT_SLOT_NAME)) { - if (opts->enabled && - IsSet(supported_opts, SUBOPT_ENABLED) && - IsSet(opts->specified_opts, SUBOPT_ENABLED)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - /*- translator: both %s are strings of the form "option = value" */ - errmsg("%s and %s are mutually exclusive options", - "slot_name = NONE", "enabled = true"))); - - if (opts->create_slot && - IsSet(supported_opts, SUBOPT_CREATE_SLOT) && - IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - /*- translator: both %s are strings of the form "option = value" */ - errmsg("%s and %s are mutually exclusive options", - "slot_name = NONE", "create_slot = true"))); - - if (opts->enabled && - IsSet(supported_opts, SUBOPT_ENABLED) && - !IsSet(opts->specified_opts, SUBOPT_ENABLED)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - /*- translator: both %s are strings of the form "option = value" */ - errmsg("subscription with %s must also set %s", - "slot_name = NONE", "enabled = false"))); + if (opts->enabled) + { + if (IsSet(opts->specified_opts, SUBOPT_ENABLED)) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + /*- translator: both %s are strings of the form "option = value" */ + errmsg("%s and %s are mutually exclusive options", + "slot_name = NONE", "enabled = true"))); + else + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + /*- translator: both %s are strings of the form "option = value" */ + errmsg("subscription with %s must also set %s", + "slot_name = NONE", "enabled = false"))); + } - if (opts->create_slot && - IsSet(supported_opts, SUBOPT_CREATE_SLOT) && - !IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - /*- translator: both %s are strings of the form "option = value" */ - errmsg("subscription with %s must also set %s", - "slot_name = NONE", "create_slot = false"))); + if (opts->create_slot) + { + if (IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT)) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + /*- translator: both %s are strings of the form "option = value" */ + errmsg("%s and %s are mutually exclusive options", + "slot_name = NONE", "create_slot = true"))); + else + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + /*- translator: both %s are strings of the form "option = value" */ + errmsg("subscription with %s must also set %s", + "slot_name = NONE", "create_slot = false"))); + } } } diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index 15a1ac6398b..80aae83562c 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -54,7 +54,7 @@ CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PU ERROR: connect = false and create_slot = true are mutually exclusive options CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = true); ERROR: slot_name = NONE and enabled = true are mutually exclusive options -CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = true); +CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = false, create_slot = true); ERROR: slot_name = NONE and create_slot = true are mutually exclusive options CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE); ERROR: subscription with slot_name = NONE must also set enabled = false diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql index 7faa935a2a7..bd0f4af1e49 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -43,7 +43,7 @@ CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PU CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, enabled = true); CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, create_slot = true); CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = true); -CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = true); +CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = false, create_slot = true); CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE); CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = false); CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = false); |