From c4644584c13362eb67c9db8f6fbe8c109728fbd4 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Wed, 9 Jun 2021 08:04:12 -0700 Subject: [PATCH v7] 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 | 63 +++++++++++++------------ 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 9ec344cb0f..dd33196e0f 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -261,28 +261,26 @@ parse_subscription_options(List *stmt_options, SubOpts *opts) */ 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(specified_opts, SUBOPT_ENABLED)) + incompat_opt = "enabled = true"; + else if (opts->create_slot && + IsSet(supported_opts, SUBOPT_CREATE_SLOT) && + IsSet(specified_opts, SUBOPT_CREATE_SLOT)) + incompat_opt = "create_slot = true"; + else if (opts->copy_data && IsSet(supported_opts, SUBOPT_COPY_DATA) && + IsSet(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(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(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; @@ -297,35 +295,38 @@ parse_subscription_options(List *stmt_options, SubOpts *opts) if (!opts->slot_name && IsSet(supported_opts, SUBOPT_SLOT_NAME) && IsSet(specified_opts, SUBOPT_SLOT_NAME)) { + char *incompat_opt = NULL; + char *required_opt = NULL; + if (opts->enabled && IsSet(supported_opts, SUBOPT_ENABLED) && IsSet(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(specified_opts, SUBOPT_CREATE_SLOT)) + incompat_opt = "create_slot = true"; - if (opts->create_slot && IsSet(supported_opts, SUBOPT_CREATE_SLOT) && - IsSet(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(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(specified_opts, SUBOPT_CREATE_SLOT)) + required_opt = "create_slot = false"; - if (opts->create_slot && IsSet(supported_opts, SUBOPT_CREATE_SLOT) && - !IsSet(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))); } opts->specified_opts = specified_opts; -- 2.25.1