From 59b26f7c5e61f3bb9614c45fdea89a6452f80128 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Tue, 6 Jul 2021 10:33:30 +0000 Subject: [PATCH v10] Remove similar ereport calls in parse_subscription_options Remove similar code (with the only difference in the option) when throwing errors for mutually exclusive and disallowed combination of options. Have variables for the options and use them in the error messages. It makes the code look better with lesser ereport(ERROR statements. --- src/backend/commands/subscriptioncmds.c | 66 ++++++++++++------------- 1 file changed, 31 insertions(+), 35 deletions(-) diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index eb88d877a5..259919d23e 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -249,31 +249,28 @@ parse_subscription_options(List *stmt_options, bits32 supported_opts, SubOpts *o */ if (!opts->connect && IsSet(supported_opts, SUBOPT_CONNECT)) { + char *incompat_opt = NULL; + /* Check for incompatible options from the user. */ if (opts->enabled && IsSet(supported_opts, SUBOPT_ENABLED) && IsSet(opts->specified_opts, SUBOPT_ENABLED)) + incompat_opt = "enabled = true"; + else if (opts->create_slot && + IsSet(supported_opts, SUBOPT_CREATE_SLOT) && + IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT)) + incompat_opt = "create_slot = true"; + else if (opts->copy_data && + IsSet(supported_opts, SUBOPT_COPY_DATA) && + IsSet(opts->specified_opts, SUBOPT_COPY_DATA)) + incompat_opt = "copy_data = true"; + + if (incompat_opt) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), /*- translator: both %s are strings of the form "option = value" */ errmsg("%s and %s are mutually exclusive 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), - errmsg("%s and %s are mutually exclusive 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), - errmsg("%s and %s are mutually exclusive options", - "connect = false", "copy_data = true"))); + "connect = false", incompat_opt))); /* Change the defaults of other options. */ opts->enabled = false; @@ -289,41 +286,40 @@ parse_subscription_options(List *stmt_options, bits32 supported_opts, SubOpts *o IsSet(supported_opts, SUBOPT_SLOT_NAME) && IsSet(opts->specified_opts, SUBOPT_SLOT_NAME)) { + char *incompat_opt = NULL; + char *required_opt = NULL; + 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"))); + incompat_opt = "enabled = true"; + else if (opts->create_slot && + IsSet(supported_opts, SUBOPT_CREATE_SLOT) && + IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT)) + incompat_opt = "create_slot = true"; - if (opts->create_slot && - IsSet(supported_opts, SUBOPT_CREATE_SLOT) && - IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT)) + if (incompat_opt) 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"))); + "slot_name = NONE", incompat_opt))); 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"))); + required_opt = "enabled = false"; + else if (opts->create_slot && + IsSet(supported_opts, SUBOPT_CREATE_SLOT) && + !IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT)) + required_opt = "create_slot = false"; - if (opts->create_slot && - IsSet(supported_opts, SUBOPT_CREATE_SLOT) && - !IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT)) + if (required_opt) 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"))); + "slot_name = NONE", required_opt))); } } -- 2.25.1