8000 Improve parsing of options of CREATE/ALTER SUBSCRIPTION · postgrespro/postgres@00029de · GitHub
[go: up one dir, main page]
Skip to content

    • Commit 00029de

    Browse files
    michaelpqmichaelpq
    committed
    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
    1 parent f99870d commit 00029de

    File tree

    3 files changed

    +35
    -42
    lines changed

    3 files changed

    +35
    -42
    lines changed

    ‎src/backend/commands/subscriptioncmds.c

    Lines changed: 33 additions & 40 deletions
    Original file line numberDiff line numberDiff line change
    @@ -95,15 +95,16 @@ static void ReportSlotConnectionError(List *rstates, Oid subid, char *slotname,
    9595
    *
    9696
    * Since not all options can be specified in both commands, this function
    9797
    * will report an error if mutually exclusive options are specified.
    98-
    *
    99-
    * Caller is expected to have cleared 'opts'.
    10098
    */
    10199
    static void
    102100
    parse_subscription_options(ParseState *pstate, List *stmt_options,
    103101
    bits32 supported_opts, SubOpts *opts)
    104102
    {
    105103
    ListCell *lc;
    106104

    105+
    /* Start out with cleared opts. */
    106+
    memset(opts, 0, sizeof(SubOpts));
    107+
    107108
    /* caller must expect some option */
    108109
    Assert(supported_opts != 0);
    109110

    @@ -262,7 +263,6 @@ parse_subscription_options(ParseState *pstate, List *stmt_options,
    262263
    {
    263264
    /* Check for incompatible options from the user. */
    264265
    if (opts->enabled &&
    265-
    IsSet(supported_opts, SUBOPT_ENABLED) &&
    266266
    IsSet(opts->specified_opts, SUBOPT_ENABLED))
    267267
    ereport(ERROR,
    268268
    (errcode(ERRCODE_SYNTAX_ERROR),
    @@ -271,15 +271,13 @@ parse_subscription_options(ParseState *pstate, List *stmt_options,
    271271
    "connect = false", "enabled = true")));
    272272

    273273
    if (opts->create_slot &&
    274-
    IsSet(supported_opts, SUBOPT_CREATE_SLOT) &&
    275274
    IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT))
    276275
    ereport(ERROR,
    277276
    (errcode(ERRCODE_SYNTAX_ERROR),
    278277
    errmsg("%s and %s are mutually exclusive options",
    279278
    "connect = false", "create_slot = true")));
    280279

    281280
    if (opts->copy_data &&
    282-
    IsSet(supported_opts, SUBOPT_COPY_DATA) &&
    283281
    IsSet(opts->specified_opts, SUBOPT_COPY_DATA))
    284282
    ereport(ERROR,
    285283
    (errcode(ERRCODE_SYNTAX_ERROR),
    @@ -297,44 +295,39 @@ parse_subscription_options(ParseState *pstate, List *stmt_options,
    297295
    * was used.
    298296
    */
    299297
    if (!opts->slot_name &&
    300-
    IsSet(supported_opts, SUBOPT_SLOT_NAME) &&
    301298
    IsSet(opts->specified_opts, SUBOPT_SLOT_NAME))
    302299
    {
    303-
    if (opts->enabled &&
    304-
    IsSet(supported_opts, SUBOPT_ENABLED) &&
    305-
    IsSet(opts->specified_opts, SUBOPT_ENABLED))
    306-
    ereport(ERROR,
    307-
    (errcode(ERRCODE_SYNTAX_ERROR),
    308-
    /*- translator: both %s are strings of the form "option = value" */
    309-
    errmsg("%s and %s are mutually exclusive options",
    310-
    "slot_name = NONE", "enabled = true")));
    311-
    312-
    if (opts->create_slot &&
    313-
    IsSet(supported_opts, SUBOPT_CREATE_SLOT) &&
    314-
    IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT))
    315-
    ereport(ERROR,
    316-
    (errcode(ERRCODE_SYNTAX_ERROR),
    317-
    /*- translator: both %s are strings of the form "option = value" */
    318-
    errmsg("%s and %s are mutually exclusive options",
    319-
    "slot_name = NONE", "create_slot = true")));
    320-
    321-
    if (opts->enabled &&
    322-
    IsSet(supported_opts, SUBOPT_ENABLED) &&
    323-
    !IsSet(opts->specified_opts, SUBOPT_ENABLED))
    324-
    ereport(ERROR,
    325-
    (errcode(ERRCODE_SYNTAX_ERROR),
    326-
    /*- translator: both %s are strings of the form "option = value" */
    327-
    errmsg("subscription with %s must also set %s",
    328-
    "slot_name = NONE", "enabled = false")));
    300+
    if (opts->enabled)
    301+
    {
    302+
    if (IsSet(opts->specified_opts, SUBOPT_ENABLED))
    303+
    ereport(ERROR,
    304+
    (errcode(ERRCODE_SYNTAX_ERROR),
    305+
    /*- translator: both %s are strings of the form "option = value" */
    306+
    errmsg("%s and %s are mutually exclusive options",
    307+
    "slot_name = NONE", "enabled = true")));
    308+
    else
    309+
    ereport(ERROR,
    310+
    (errcode(ERRCODE_SYNTAX_ERROR),
    311+
    /*- translator: both %s are strings of the form "option = value" */
    312+
    errmsg("subscription with %s must also set %s",
    313+
    "slot_name = NONE", "enabled = false")));
    314+
    }
    329315

    330-
    if (opts->create_slot &&
    331-
    IsSet(supported_opts, SUBOPT_CREATE_SLOT) &&
    332-
    !IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT))
    333-
    ereport(ERROR,
    334-
    (errcode(ERRCODE_SYNTAX_ERROR),
    335-
    /*- translator: both %s are strings of the form "option = value" */
    336-
    errmsg("subscription with %s must also set %s",
    337-
    "slot_name = NONE", "create_slot = false")));
    316+
    if (opts->create_slot)
    317+
    {
    318+
    if (IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT))
    319+
    ereport(ERROR,
    320+
    (errcode(ERRCODE_SYNTAX_ERROR),
    321+
    /*- translator: both %s are strings of the form "option = value" */
    322+
    errmsg("%s and %s are mutually exclusive options",
    323+
    "slot_name = NONE", "create_slot = true")));
    324+
    else
    325+
    ereport(ERROR,
    326+
    (errcode(ERRCODE_SYNTAX_ERROR),
    327+
    /*- translator: both %s are strings of the form "option = value" */
    328+
    errmsg("subscription with %s must also set %s",
    329+
    "slot_name = NONE", "create_slot = false")));
    330+
    }
    338331
    }
    339332
    }
    340333

    ‎src/test/regress/expected/subscription.out

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -54,7 +54,7 @@ CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PU
    5454
    ERROR: connect = false and create_slot = true are mutually exclusive options
    5555
    CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = true);
    5656
    ERROR: slot_name = NONE and enabled = true are mutually exclusive options
    57-
    CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = true);
    57+
    CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = false, create_slot = true);
    5858
    ERROR: slot_name = NONE and create_slot = true are mutually exclusive options
    5959
    CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE);
    6060
    ERROR: subscription with slot_name = NONE must also set enabled = false

    ‎src/test/regress/sql/subscription.sql

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -43,7 +43,7 @@ CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PU
    4343
    CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, enabled = true);
    4444
    CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, create_slot = true);
    4545
    CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = true);
    46-
    CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = true);
    46+
    CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = false, create_slot = true);
    4747
    CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE);
    4848
    CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, enabled = false);
    4949
    CREATE SUBSCRIPTION regress_testsub2 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, create_slot = false);

    0 commit comments

    Comments
     (0)
    0