Hi,
parse_subscription_options function has some similar code when
throwing errors [with the only difference in the option]. I feel we
could just use a variable for the option and use it in the error.
While this has no benefit at all, it saves some LOC and makes the code
look better with lesser ereport(ERROR statements. PSA patch.
Thoughts?
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
From 6e4824653d2849631cba3cfe1fe562e1b2823c4a Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Sat, 1 May 2021 20:26:30 +0530
Subject: [PATCH v1] Refactoring of error code in parse_subscription_options
---
src/backend/commands/subscriptioncmds.c | 50 ++++++++++++-------------
1 file changed, 23 insertions(+), 27 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 517c8edd3b..c4c9b4bd8c 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -227,25 +227,21 @@ parse_subscription_options(List *options,
*/
if (connect && !*connect)
{
+ char *option = NULL;
+
/* Check for incompatible options from the user. */
if (enabled && *enabled_given && *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",
- "connect = false", "enabled = true")));
-
- if (create_slot && create_slot_given && *create_slot)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("%s and %s are mutually exclusive options",
- "connect = false", "create_slot = true")));
+ option = "enabled = true";
+ else if (create_slot && create_slot_given && *create_slot)
+ option = "create_slot = true";
+ else if (copy_data && copy_data_given && *copy_data)
+ option = "copy_data = true";
- if (copy_data && copy_data_given && *copy_data)
+ if (option)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("%s and %s are mutually exclusive options",
- "connect = false", "copy_data = true")));
+ "connect = false", option)));
/* Change the defaults of other options. */
*enabled = false;
@@ -259,31 +255,31 @@ parse_subscription_options(List *options,
*/
if (slot_name && *slot_name_given && !*slot_name)
{
+ char *option = NULL;
+
if (enabled && *enabled_given && *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")));
+ option = "enabled = true";
+ else if (create_slot && create_slot_given && *create_slot)
+ option = "create_slot = true";
- if (create_slot && create_slot_given && *create_slot)
+ if (option)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("%s and %s are mutually exclusive options",
- "slot_name = NONE", "create_slot = true")));
+ "slot_name = NONE", option)));
+
+ option = NULL;
if (enabled && !*enabled_given && *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")));
+ option = "enabled = false";
+ else if (create_slot && !create_slot_given && *create_slot)
+ option = "create_slot = false";
- if (create_slot && !create_slot_given && *create_slot)
+ if (option)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("subscription with %s must also set %s",
- "slot_name = NONE", "create_slot = false")));
+ "slot_name = NONE", option)));
}
}
--
2.25.1
