On 27/11/2018 13:29, Peter Eisentraut wrote:
> On 27/11/2018 10:10, Sergei Kornilov wrote:
>> Hello
>>
>>>> - recovery_target = immediate was replaced with recovery_target_immediate
>>>> bool GUC
>>>
>>> Why?
>> Due this comment:
>> https://www.postgresql.org/message-id/20181126172118.GY3415%40tamriel.snowman.net
>>> I've not been following this very closely, but seems like
>>> recovery_target_string is a bad idea.. Why not just make that
>>> 'recovery_target_immediate' and make it a boolean? Having a GUC that's
>>> only got one valid value seems really odd.
>
> It is a bit odd, but that's the way it's been, and I don't see a reason
> to change it as part of this fix. We are attempting to fix the way the
> GUC parameters are parsed, not change the name and meaning of the
> parameters themselves.
The attached seems to be the simplest way to fix this. (Needs
documentation updates, test updates, error message refinement.)
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From d5cb3f3a30bd045283c9848bd39ff012c817d908 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Tue, 27 Nov 2018 13:43:54 +0100
Subject: [PATCH] WIP Only allow one recovery target setting
---
src/backend/utils/misc/guc.c | 29 +++++++++++++++--------------
src/include/access/xlog.h | 2 +-
2 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 6497393c03..dd2caee95b 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -11065,14 +11065,11 @@ check_recovery_target(char **newval, void **extra,
GucSource source)
static void
assign_recovery_target(const char *newval, void *extra)
{
+ if (recoveryTarget)
+ elog(ERROR, "multiple recovery targets specified");
+
if (newval && strcmp(newval, "") != 0)
recoveryTarget = RECOVERY_TARGET_IMMEDIATE;
- else
- /*
- * Reset recoveryTarget to RECOVERY_TARGET_UNSET to proper
handle user
- * setting multiple recovery_target with blank value on last.
- */
- recoveryTarget = RECOVERY_TARGET_UNSET;
}
static bool
@@ -11098,13 +11095,14 @@ check_recovery_target_xid(char **newval, void
**extra, GucSource source)
static void
assign_recovery_target_xid(const char *newval, void *extra)
{
+ if (recoveryTarget)
+ elog(ERROR, "multiple recovery targets specified");
+
if (newval && strcmp(newval, "") != 0)
{
recoveryTarget = RECOVERY_TARGET_XID;
recoveryTargetXid = *((TransactionId *) extra);
}
- else
- recoveryTarget = RECOVERY_TARGET_UNSET;
}
static bool
@@ -11161,13 +11159,14 @@ check_recovery_target_time(char **newval, void
**extra, GucSource source)
static void
assign_recovery_target_time(const char *newval, void *extra)
{
+ if (recoveryTarget)
+ elog(ERROR, "multiple recovery targets specified");
+
if (newval && strcmp(newval, "") != 0)
{
recoveryTarget = RECOVERY_TARGET_TIME;
recoveryTargetTime = *((TimestampTz *) extra);
}
- else
- recoveryTarget = RECOVERY_TARGET_UNSET;
}
static bool
@@ -11186,13 +11185,14 @@ check_recovery_target_name(char **newval, void
**extra, GucSource source)
static void
assign_recovery_target_name(const char *newval, void *extra)
{
+ if (recoveryTarget)
+ elog(ERROR, "multiple recovery targets specified");
+
if (newval && strcmp(newval, "") != 0)
{
recoveryTarget = RECOVERY_TARGET_NAME;
recoveryTargetName = (char *) newval;
}
- else
- recoveryTarget = RECOVERY_TARGET_UNSET;
}
static bool
@@ -11240,13 +11240,14 @@ check_recovery_target_lsn(char **newval, void
**extra, GucSource source)
static void
assign_recovery_target_lsn(const char *newval, void *extra)
{
+ if (recoveryTarget)
+ elog(ERROR, "multiple recovery targets specified");
+
if (newval && strcmp(newval, "") != 0)
{
recoveryTarget = RECOVERY_TARGET_LSN;
recoveryTargetLSN = *((XLogRecPtr *) extra);
}
- else
- recoveryTarget = RECOVERY_TARGET_UNSET;
}
static bool
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index f3a7ba4d42..1fd4aec3c7 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -79,7 +79,7 @@ extern HotStandbyState standbyState;
*/
typedef enum
{
- RECOVERY_TARGET_UNSET,
+ RECOVERY_TARGET_UNSET = 0,
RECOVERY_TARGET_XID,
RECOVERY_TARGET_TIME,
RECOVERY_TARGET_NAME,
--
2.19.1
