Patch: add --if-exists to pg_recvlogical
Hackers,
While doing some scripting around pg_recvlogical at $work, I found a need
for $subject. Attached, find a patch to that effect.
I tried simply to mirror the logic used elsewhere. I don't think there's
anything controversial here, but welcome any comments or suggestions.
This applies and builds successfully against master, and behaves as
designed (i.e., dropping or trying to stream from a nonexistent slot exits
cleanly). It doesn't affect any other behavior I could observe.
If accepted, this will likely need a pgindent run upon merging; I had to
give up on the rabbit hole of getting that working locally.
Thanks,
rls
--
:wq
Attachments:
pg_recvlogical--if-exists-v1.patchapplication/octet-stream; name=pg_recvlogical--if-exists-v1.patchDownload
diff --git a/doc/src/sgml/ref/pg_recvlogical.sgml b/doc/src/sgml/ref/pg_recvlogical.sgml
index eaea94d..2c638d3 100644
--- a/doc/src/sgml/ref/pg_recvlogical.sgml
+++ b/doc/src/sgml/ref/pg_recvlogical.sgml
@@ -199,6 +199,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ Do not error out when <option>--drop-slot</option> or <option>--start<option> are
+ specified and a slot with the specified name does not exist.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>-n</option></term>
<term><option>--no-loop</option></term>
<listitem>
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index 370d871..ace5372 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -686,7 +686,7 @@ main(int argc, char **argv)
_("%s: dropping replication slot \"%s\"\n"),
progname, replication_slot);
- if (!DropReplicationSlot(conn, replication_slot))
+ if (!DropReplicationSlot(conn, replication_slot, false))
disconnect_and_exit(1);
disconnect_and_exit(0);
}
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 6b081bd..46ea5a3 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -33,6 +33,8 @@
/* Time to sleep between reconnection attempts */
#define RECONNECT_SLEEP_TIME 5
+#define ERRCODE_UNKNOWN_OBJECT "42704"
+
/* Global Options */
static char *outfile = NULL;
static int verbose = 0;
@@ -43,6 +45,7 @@ static XLogRecPtr startpos = InvalidXLogRecPtr;
static XLogRecPtr endpos = InvalidXLogRecPtr;
static bool do_create_slot = false;
static bool slot_exists_ok = false;
+static bool slot_not_exists_ok = false;
static bool do_start_slot = false;
static bool do_drop_slot = false;
static char *replication_slot = NULL;
@@ -84,6 +87,7 @@ usage(void)
printf(_(" -f, --file=FILE receive log into this file, - for stdout\n"));
printf(_(" -F --fsync-interval=SECS\n"
" time between fsyncs to the output file (default: %d)\n"), (fsync_interval / 1000));
+ printf(_(" --if-exists do not error if slot does not exist\n"));
printf(_(" --if-not-exists do not error if slot already exists when creating a slot\n"));
printf(_(" -I, --startpos=LSN where in an existing slot should the streaming start\n"));
printf(_(" -E, --endpos=LSN exit after receiving the specified LSN\n"));
@@ -267,6 +271,17 @@ StreamLogicalLog(void)
res = PQexec(conn, query->data);
if (PQresultStatus(res) != PGRES_COPY_BOTH)
{
+ const char* sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
+
+ if (slot_not_exists_ok &&
+ sqlstate &&
+ strcmp(sqlstate, ERRCODE_UNKNOWN_OBJECT) == 0)
+ {
+ destroyPQExpBuffer(query);
+ PQclear(res);
+ disconnect_and_exit(0);
+ }
+
fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
progname, query->data, PQresultErrorMessage(res));
PQclear(res);
@@ -699,6 +716,7 @@ main(int argc, char **argv)
{"start", no_argument, NULL, 2},
{"drop-slot", no_argument, NULL, 3},
{"if-not-exists", no_argument, NULL, 4},
+ {"if-exists", no_argument, NULL, 5},
{NULL, 0, NULL, 0}
};
int c;
@@ -843,6 +861,9 @@ main(int argc, char **argv)
case 4:
slot_exists_ok = true;
break;
+ case 5:
+ slot_not_exists_ok = true;
+ break;
default:
@@ -903,6 +924,14 @@ main(int argc, char **argv)
exit(1);
}
+ if (do_create_slot && slot_not_exists_ok)
+ {
+ fprintf(stderr, _("%s: cannot use --create-slot with --if-exists\n"), progname);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit(1);
+ }
+
if (do_drop_slot && (do_create_slot || do_start_slot))
{
fprintf(stderr, _("%s: cannot use --create-slot or --start together with --drop-slot\n"), progname);
@@ -967,7 +996,7 @@ main(int argc, char **argv)
_("%s: dropping replication slot \"%s\"\n"),
progname, replication_slot);
- if (!DropReplicationSlot(conn, replication_slot))
+ if (!DropReplicationSlot(conn, replication_slot, slot_not_exists_ok))
disconnect_and_exit(1);
}
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 7ea3b0f..84dd135 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -29,6 +29,7 @@
#include "datatype/timestamp.h"
#define ERRCODE_DUPLICATE_OBJECT "42710"
+#define ERRCODE_UNKNOWN_OBJECT "42704"
const char *progname;
char *connection_string = NULL;
@@ -391,7 +392,7 @@ CreateReplicationSlot(PGconn *conn, const char *slot_name, const char *plugin,
* returns true in case of success.
*/
bool
-DropReplicationSlot(PGconn *conn, const char *slot_name)
+DropReplicationSlot(PGconn *conn, const char *slot_name, bool slot_not_exists_ok)
{
PQExpBuffer query;
PGresult *res;
@@ -406,6 +407,17 @@ DropReplicationSlot(PGconn *conn, const char *slot_name)
res = PQexec(conn, query->data);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
{
+ const char *sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
+
+ if (slot_not_exists_ok &&
+ sqlstate &&
+ strcmp(sqlstate, ERRCODE_UNKNOWN_OBJECT) == 0)
+ {
+ destroyPQExpBuffer(query);
+ PQclear(res);
+ return true;
+ }
+
fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
progname, query->data, PQerrorMessage(conn));
diff --git a/src/bin/pg_basebackup/streamutil.h b/src/bin/pg_basebackup/streamutil.h
index 460dcb5..845e48a 100644
--- a/src/bin/pg_basebackup/streamutil.h
+++ b/src/bin/pg_basebackup/streamutil.h
@@ -34,7 +34,8 @@ extern PGconn *GetConnection(void);
extern bool CreateReplicationSlot(PGconn *conn, const char *slot_name,
const char *plugin, bool is_physical,
bool slot_exists_ok);
-extern bool DropReplicationSlot(PGconn *conn, const char *slot_name);
+extern bool DropReplicationSlot(PGconn *conn, const char *slot_name,
+ bool slot_not_exists_ok);
extern bool RunIdentifySystem(PGconn *conn, char **sysid,
TimeLineID *starttli,
XLogRecPtr *startpos,
On Sun, May 21, 2017 at 3:04 PM, Rosser Schwarz
<rosser.schwarz@gmail.com> wrote:
While doing some scripting around pg_recvlogical at $work, I found a need
for $subject. Attached, find a patch to that effect.I tried simply to mirror the logic used elsewhere. I don't think there's
anything controversial here, but welcome any comments or suggestions.This applies and builds successfully against master, and behaves as designed
(i.e., dropping or trying to stream from a nonexistent slot exits cleanly).
It doesn't affect any other behavior I could observe.If accepted, this will likely need a pgindent run upon merging; I had to
give up on the rabbit hole of getting that working locally.
Please add this to commitfest.postgresql.org
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Aug 25, 2017 at 12:34 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Sun, May 21, 2017 at 3:04 PM, Rosser Schwarz
<rosser.schwarz@gmail.com> wrote:While doing some scripting around pg_recvlogical at $work, I found a need
for $subject. Attached, find a patch to that effect...
Please add this to commitfest.postgresql.org
Done, thanks!
https://commitfest.postgresql.org/14/1256/
--
:wq
On 8/26/17 15:40, Rosser Schwarz wrote:
On Fri, Aug 25, 2017 at 12:34 PM, Robert Haas <robertmhaas@gmail.com
<mailto:robertmhaas@gmail.com>> wrote:On Sun, May 21, 2017 at 3:04 PM, Rosser Schwarz
<rosser.schwarz@gmail.com <mailto:rosser.schwarz@gmail.com>> wrote:While doing some scripting around pg_recvlogical at $work, I found a need
for $subject. Attached, find a patch to that effect...Please add this to commitfest.postgresql.org
<http://commitfest.postgresql.org>Done, thanks!
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ Do not error out when <option>--drop-slot</option> or
<option>--start<option> are
+ specified and a slot with the specified name does not exist.
+ </para>
+ </listitem>
+ </varlistentry>I understand the --drop-slot part. But I don't understand what it means
to ignore a missing replication slot when running --start.
The same option should be added to pg_receivewal as well.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Sep 2, 2017 at 5:22 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
<varlistentry> + <term><option>--if-exists</option></term> + <listitem> + <para> + Do not error out when <option>--drop-slot</option> or <option>--start<option> are + specified and a slot with the specified name does not exist. + </para> + </listitem> + </varlistentry>I understand the --drop-slot part. But I don't understand what it means
to ignore a missing replication slot when running --start.
Also "<option>--start<option>" breaks the documentation build (missing
slash on the closing tag).
--
Thomas Munro
http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 09 Sep 2017, at 06:05, Thomas Munro <thomas.munro@enterprisedb.com> wrote:
On Sat, Sep 2, 2017 at 5:22 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:<varlistentry> + <term><option>--if-exists</option></term> + <listitem> + <para> + Do not error out when <option>--drop-slot</option> or <option>--start<option> are + specified and a slot with the specified name does not exist. + </para> + </listitem> + </varlistentry>I understand the --drop-slot part. But I don't understand what it means
to ignore a missing replication slot when running --start.Also "<option>--start<option>" breaks the documentation build (missing
slash on the closing tag).
This patch is "Waiting for Author” due to the above review comments from Peter
and Thomas. Do you think you will have time to address these shortly so we can
move this patch further in the process?
cheers ./daniel
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Sep 15, 2017 at 04:15 Daniel Gustafsson <daniel@yesql.se> wrote:
This patch is "Waiting for Author” due to the above review comments from
Peter
and Thomas. Do you think you will have time to address these shortly so
we can
move this patch further in the process?
I have a patch addressing both comments that I'm testing and tweaking now
(well, not *now* now), which I hope to submit this weekend.
Is that enough time?
--rosser
--
:wq
On 9/15/17 13:18, Rosser Schwarz wrote:
On Fri, Sep 15, 2017 at 04:15 Daniel Gustafsson <daniel@yesql.se
<mailto:daniel@yesql.se>> wrote:This patch is "Waiting for Author” due to the above review comments
from Peter
and Thomas. Do you think you will have time to address these
shortly so we can
move this patch further in the process?I have a patch addressing both comments that I'm testing and tweaking
now (well, not *now* now), which I hope to submit this weekend.Is that enough time?
There is no rush of any kind.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Sep 1, 2017 at 10:22 AM, Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:
I understand the --drop-slot part. But I don't understand what it means
to ignore a missing replication slot when running --start.
I'm not sure I do either, honestly. I followed the Principle of Least
Surprise in making it a no-op when those switches are used and the slot
doesn't exist, over "no one will ever do that". Because someone will.
I'm happy to hear suggestions on what to do in that case beyond exit
cleanly.
The same option should be added to pg_receivewal as well.
Done.
On Fri, Sep 8, 2017 at 21:06 Thomas Munro <thomas.munro@enterprisedb.com>
wrote:
Also "<option>--start<option>" breaks the documentation build
(missing slash on the closing tag).
Fixed, thanks.
Please see revised patch, attached, addressing both comments.
rls
--
:wq
Attachments:
pg_recvlogical--if-exists-v2.patchapplication/octet-stream; name=pg_recvlogical--if-exists-v2.patchDownload
diff --git a/doc/src/sgml/ref/pg_receivewal.sgml b/doc/src/sgml/ref/pg_receivewal.sgml
index a17082b..e70b09a 100644
--- a/doc/src/sgml/ref/pg_receivewal.sgml
+++ b/doc/src/sgml/ref/pg_receivewal.sgml
@@ -93,6 +93,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ Do not error out when <option>--drop-slot</option> is specified
+ and a slot with the specified name does not exist.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--if-not-exists</option></term>
<listitem>
<para>
diff --git a/doc/src/sgml/ref/pg_recvlogical.sgml b/doc/src/sgml/ref/pg_recvlogical.sgml
index eaea94d..8463daa 100644
--- a/doc/src/sgml/ref/pg_recvlogical.sgml
+++ b/doc/src/sgml/ref/pg_recvlogical.sgml
@@ -199,6 +199,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--if-exists</option></term>
+ <listitem>
+ <para>
+ Do not error out when <option>--drop-slot</option> or <option>--start</option> are
+ specified and a slot with the specified name does not exist.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>-n</option></term>
<term><option>--no-loop</option></term>
<listitem>
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index 370d871..1096afb 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -39,6 +39,7 @@ static int standby_message_timeout = 10 * 1000; /* 10 sec = default */
static volatile bool time_to_abort = false;
static bool do_create_slot = false;
static bool slot_exists_ok = false;
+static bool slot_not_exists_ok = false;
static bool do_drop_slot = false;
static bool synchronous = false;
static char *replication_slot = NULL;
@@ -77,6 +78,7 @@ usage(void)
printf(_(" %s [OPTION]...\n"), progname);
printf(_("\nOptions:\n"));
printf(_(" -D, --directory=DIR receive write-ahead log files into this directory\n"));
+ printf(_(" --if-exists do not error if slot does not exist when dropping a slot\n"));
printf(_(" --if-not-exists do not error if slot already exists when creating a slot\n"));
printf(_(" -n, --no-loop do not loop on connection lost\n"));
printf(_(" -s, --status-interval=SECS\n"
@@ -472,8 +474,9 @@ main(int argc, char **argv)
/* action */
{"create-slot", no_argument, NULL, 1},
{"drop-slot", no_argument, NULL, 2},
- {"if-not-exists", no_argument, NULL, 3},
- {"synchronous", no_argument, NULL, 4},
+ {"if-exists", no_argument, NULL, 3},
+ {"if-not-exists", no_argument, NULL, 4},
+ {"synchronous", no_argument, NULL, 5},
{NULL, 0, NULL, 0}
};
@@ -566,9 +569,12 @@ main(int argc, char **argv)
do_drop_slot = true;
break;
case 3:
- slot_exists_ok = true;
+ slot_not_exists_ok = true;
break;
case 4:
+ slot_exists_ok = true;
+ break;
+ case 5:
synchronous = true;
break;
default:
@@ -603,6 +609,14 @@ main(int argc, char **argv)
exit(1);
}
+ if (do_create_slot && slot_not_exists_ok)
+ {
+ fprintf(stderr, _("%s: cannot use --create-slot with --if-exists\n"), progname);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit(1);
+ }
+
if (replication_slot == NULL && (do_drop_slot || do_create_slot))
{
/* translator: second %s is an option name */
@@ -686,7 +700,7 @@ main(int argc, char **argv)
_("%s: dropping replication slot \"%s\"\n"),
progname, replication_slot);
- if (!DropReplicationSlot(conn, replication_slot))
+ if (!DropReplicationSlot(conn, replication_slot, slot_not_exists_ok))
disconnect_and_exit(1);
disconnect_and_exit(0);
}
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 6b081bd..3e0b506 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -33,6 +33,8 @@
/* Time to sleep between reconnection attempts */
#define RECONNECT_SLEEP_TIME 5
+#define ERRCODE_UNKNOWN_OBJECT "42704"
+
/* Global Options */
static char *outfile = NULL;
static int verbose = 0;
@@ -43,6 +45,7 @@ static XLogRecPtr startpos = InvalidXLogRecPtr;
static XLogRecPtr endpos = InvalidXLogRecPtr;
static bool do_create_slot = false;
static bool slot_exists_ok = false;
+static bool slot_not_exists_ok = false;
static bool do_start_slot = false;
static bool do_drop_slot = false;
static char *replication_slot = NULL;
@@ -84,6 +87,7 @@ usage(void)
printf(_(" -f, --file=FILE receive log into this file, - for stdout\n"));
printf(_(" -F --fsync-interval=SECS\n"
" time between fsyncs to the output file (default: %d)\n"), (fsync_interval / 1000));
+ printf(_(" --if-exists do not error if slot does not exist\n"));
printf(_(" --if-not-exists do not error if slot already exists when creating a slot\n"));
printf(_(" -I, --startpos=LSN where in an existing slot should the streaming start\n"));
printf(_(" -E, --endpos=LSN exit after receiving the specified LSN\n"));
@@ -267,6 +271,17 @@ StreamLogicalLog(void)
res = PQexec(conn, query->data);
if (PQresultStatus(res) != PGRES_COPY_BOTH)
{
+ const char* sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
+
+ if (slot_not_exists_ok &&
+ sqlstate &&
+ strcmp(sqlstate, ERRCODE_UNKNOWN_OBJECT) == 0)
+ {
+ destroyPQExpBuffer(query);
+ PQclear(res);
+ disconnect_and_exit(0);
+ }
+
fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
progname, query->data, PQresultErrorMessage(res));
PQclear(res);
@@ -699,6 +714,7 @@ main(int argc, char **argv)
{"start", no_argument, NULL, 2},
{"drop-slot", no_argument, NULL, 3},
{"if-not-exists", no_argument, NULL, 4},
+ {"if-exists", no_argument, NULL, 5},
{NULL, 0, NULL, 0}
};
int c;
@@ -843,6 +859,9 @@ main(int argc, char **argv)
case 4:
slot_exists_ok = true;
break;
+ case 5:
+ slot_not_exists_ok = true;
+ break;
default:
@@ -903,6 +922,14 @@ main(int argc, char **argv)
exit(1);
}
+ if (do_create_slot && slot_not_exists_ok)
+ {
+ fprintf(stderr, _("%s: cannot use --create-slot with --if-exists\n"), progname);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit(1);
+ }
+
if (do_drop_slot && (do_create_slot || do_start_slot))
{
fprintf(stderr, _("%s: cannot use --create-slot or --start together with --drop-slot\n"), progname);
@@ -967,7 +994,7 @@ main(int argc, char **argv)
_("%s: dropping replication slot \"%s\"\n"),
progname, replication_slot);
- if (!DropReplicationSlot(conn, replication_slot))
+ if (!DropReplicationSlot(conn, replication_slot, slot_not_exists_ok))
disconnect_and_exit(1);
}
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 7ea3b0f..84dd135 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -29,6 +29,7 @@
#include "datatype/timestamp.h"
#define ERRCODE_DUPLICATE_OBJECT "42710"
+#define ERRCODE_UNKNOWN_OBJECT "42704"
const char *progname;
char *connection_string = NULL;
@@ -391,7 +392,7 @@ CreateReplicationSlot(PGconn *conn, const char *slot_name, const char *plugin,
* returns true in case of success.
*/
bool
-DropReplicationSlot(PGconn *conn, const char *slot_name)
+DropReplicationSlot(PGconn *conn, const char *slot_name, bool slot_not_exists_ok)
{
PQExpBuffer query;
PGresult *res;
@@ -406,6 +407,17 @@ DropReplicationSlot(PGconn *conn, const char *slot_name)
res = PQexec(conn, query->data);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
{
+ const char *sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
+
+ if (slot_not_exists_ok &&
+ sqlstate &&
+ strcmp(sqlstate, ERRCODE_UNKNOWN_OBJECT) == 0)
+ {
+ destroyPQExpBuffer(query);
+ PQclear(res);
+ return true;
+ }
+
fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
progname, query->data, PQerrorMessage(conn));
diff --git a/src/bin/pg_basebackup/streamutil.h b/src/bin/pg_basebackup/streamutil.h
index 460dcb5..845e48a 100644
--- a/src/bin/pg_basebackup/streamutil.h
+++ b/src/bin/pg_basebackup/streamutil.h
@@ -34,7 +34,8 @@ extern PGconn *GetConnection(void);
extern bool CreateReplicationSlot(PGconn *conn, const char *slot_name,
const char *plugin, bool is_physical,
bool slot_exists_ok);
-extern bool DropReplicationSlot(PGconn *conn, const char *slot_name);
+extern bool DropReplicationSlot(PGconn *conn, const char *slot_name,
+ bool slot_not_exists_ok);
extern bool RunIdentifySystem(PGconn *conn, char **sysid,
TimeLineID *starttli,
XLogRecPtr *startpos,
On 9/17/17 18:21, Rosser Schwarz wrote:
On Fri, Sep 1, 2017 at 10:22 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com
<mailto:peter.eisentraut@2ndquadrant.com>> wrote:I understand the --drop-slot part. But I don't understand what it means
to ignore a missing replication slot when running --start.I'm not sure I do either, honestly. I followed the Principle of Least
Surprise in making it a no-op when those switches are used and the slot
doesn't exist, over "no one will ever do that". Because someone will.I'm happy to hear suggestions on what to do in that case beyond exit
cleanly.
Nonsensical option combinations should result in an error.
It appears that you have removed the interaction of --start and
--if-exists in your last patch, but the documentation patch still
mentions it. Which is correct?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Sep 19, 2017 at 1:12 PM, Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:
On 9/17/17 18:21, Rosser Schwarz wrote:
On Fri, Sep 1, 2017 at 10:22 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com
<mailto:peter.eisentraut@2ndquadrant.com>> wrote:I understand the --drop-slot part. But I don't understand what it means
to ignore a missing replication slot when running --start
I'm not sure I do either, honestly. I followed the Principle of Least
Surprise in making it a no-op when those switches are used and the slot
doesn't exist, over "no one will ever do that". Because someone will.
I'm happy to hear suggestions on what to do in that case beyond exit
cleanly.
Nonsensical option combinations should result in an error.
The more I think about it, I don't think it's nonsensical, though.
--create-slot --if-exists or --drop-slot --if-not-exists, obviously. I
mean, do you even logic?
Those aside, --if-exists just means a non-existent slot isn't an error
condition, doesn't it? --start --if-exists will start, if the slot exists.
Otherwise it won't, in neither case raising an error. Exactly what it says
on the tin. Perhaps the docs could make clear that combination implies
--no-loop (or at least means we'll exit immediately) if the slot does not,
in fact, exist?
Because I started work on this patch in the context of doing some scripting
around pg_recvlogical, I guess I had some vague notion that someone might
have logic in their own scripts where that state was possible (and, of
course, appropriately handled). The program exits either way: one assumes
the operator meant to do that; the other says they were wrong to have done
so.
I'm having trouble seeing a combination of options that does what it prima
facie implies as an error state. If there's broader consensus that's how it
should behave, I'll happily defer, though.
To that end, could I solicit some input from the broader audience?
It appears that you have removed the interaction of --start and
--if-exists in your last patch, but the documentation patch still
mentions it. Which is correct?
Pardon? I've had a bit on my plate recently, so it's entirely possible, if
not likely, that I missed something, but isn't that this block?
@@ -267,6 +271,17 @@ StreamLogicalLog(void)
res = PQexec(conn, query->data);
if (PQresultStatus(res) != PGRES_COPY_BOTH)
{
+ const char* sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
+
+ if (slot_not_exists_ok &&
+ sqlstate &&
+ strcmp(sqlstate, ERRCODE_UNKNOWN_OBJECT) == 0)
+ {
+ destroyPQExpBuffer(query);
+ PQclear(res);
+ disconnect_and_exit(0);
+ }
+
fprintf(stderr, _("%s: could not send replication command \"%s\": %s"),
progname, query->data, PQresultErrorMessage(res));
PQclear(res);--
:wq
On 9/20/17 02:26, Rosser Schwarz wrote:
The more I think about it, I don't think it's nonsensical, though.
--create-slot --if-exists or --drop-slot --if-not-exists, obviously. I
mean, do you even logic?
Those pieces make sense. We have many CREATE IF NOT EXISTS and DROP IF
EXISTS commands. The use is clear.
Those aside, --if-exists just means a non-existent slot isn't an error
condition, doesn't it? --start --if-exists will start, if the slot
exists. Otherwise it won't, in neither case raising an error. Exactly
what it says on the tin. Perhaps the docs could make clear that
combination implies --no-loop (or at least means we'll exit immediately)
if the slot does not, in fact, exist?
A non-terrible definition could perhaps be arrived at here, but it's not
clear why one would need this. We don't have SELECT FROM IF EXISTS, either.
I suggest you create a patch that focuses on the --create part.
You can create a separate follow-on patch for the --start part if you
wish, but I think that that patch would be rejected.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 9/22/17 15:31, Peter Eisentraut wrote:
I suggest you create a patch that focuses on the --create part.
You can create a separate follow-on patch for the --start part if you
wish, but I think that that patch would be rejected.
I have moved this entry to the next commit fest, awaiting your updated
patch.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Sep 29, 2017 at 3:06 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 9/22/17 15:31, Peter Eisentraut wrote:
I suggest you create a patch that focuses on the --create part.
You can create a separate follow-on patch for the --start part if you
wish, but I think that that patch would be rejected.I have moved this entry to the next commit fest, awaiting your updated
patch.
I'm looking for simple patches to review so doing some gardening. Per
Peter's message, moved this to Waiting on author.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Sep 29, 2017 at 10:06 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 9/22/17 15:31, Peter Eisentraut wrote:
I suggest you create a patch that focuses on the --create part.
You can create a separate follow-on patch for the --start part if you
wish, but I think that that patch would be rejected.I have moved this entry to the next commit fest, awaiting your updated
patch.
Still waiting for an update, and marked as returned with feedback. It
seems to me that your patch adds a couple of option interactions, so
this should be double-checked with exiting options and a set of TAP
tests should be added as well.
--
Michael
On 11/27/17 21:11, Michael Paquier wrote:
On Fri, Sep 29, 2017 at 10:06 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:On 9/22/17 15:31, Peter Eisentraut wrote:
I suggest you create a patch that focuses on the --create part.
You can create a separate follow-on patch for the --start part if you
wish, but I think that that patch would be rejected.I have moved this entry to the next commit fest, awaiting your updated
patch.Still waiting for an update, and marked as returned with feedback. It
seems to me that your patch adds a couple of option interactions, so
this should be double-checked with exiting options and a set of TAP
tests should be added as well.
It seems the author hasn't posted in two months, so maybe this patch is
dead for now.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services