pg_receivexlog: spurious error message connecting to 9.3
Hi,
I was testing the compatibility of pg_receivexlog with the previous PostgreSQL versions and I've discovered that in 9.5 and 9.6, although being compatible with 9.3, it prints an ugly but harmless error message.
$ 9.5/bin/pg_receivexlog -D /tmp/testx -v -p 5493
*pg_receivexlog: could not identify system: got 1 rows and 3 fields, expected 1 rows and 4 or more fields*
*column number 3 is out of range 0..2*
pg_receivexlog: starting log streaming at 0/4000000 (timeline 1)
After the error, the streaming starts and continue without issue, as it was printed by the code that checks if the connection is not database specific, and this check is not needed on 9.3.
I've attached a little patch that removes the errors when connected to 9.3.
Regards,
Marco
--
Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco.nenciarini@2ndQuadrant.it | www.2ndQuadrant.it
Attachments:
pg_receivexlog-9.3-compat.patchtext/plain; charset=UTF-8; name=pg_receivexlog-9.3-compat.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 2c963b6..273b2cf 100644
*** a/src/bin/pg_basebackup/streamutil.c
--- b/src/bin/pg_basebackup/streamutil.c
*************** RunIdentifySystem(PGconn *conn, char **s
*** 295,308 ****
if (db_name != NULL)
{
if (PQnfields(res) < 4)
! fprintf(stderr,
! _("%s: could not identify system: got %d rows and %d fields, expected %d rows and %d or more fields\n"),
! progname, PQntuples(res), PQnfields(res), 1, 4);
!
! if (PQgetisnull(res, 0, 3))
! *db_name = NULL;
else
! *db_name = pg_strdup(PQgetvalue(res, 0, 3));
}
PQclear(res);
--- 295,315 ----
if (db_name != NULL)
{
if (PQnfields(res) < 4)
! {
! if (PQserverVersion(conn) >= 90400)
! fprintf(stderr,
! _("%s: could not identify system: got %d rows and %d fields, expected %d rows and %d or more fields\n"),
! progname, PQntuples(res), PQnfields(res), 1, 4);
! else
! *db_name = NULL;
! }
else
! {
! if (PQgetisnull(res, 0, 3))
! *db_name = NULL;
! else
! *db_name = pg_strdup(PQgetvalue(res, 0, 3));
! }
}
PQclear(res);
On 10 November 2015 at 01:47, Marco Nenciarini
<marco.nenciarini@2ndquadrant.it> wrote:
I've attached a little patch that removes the errors when connected to 9.3.
Looks good to me. No point confusing users.
The other callers of RunIdentifySystem are pg_basebackup and
pg_receivelogical.
pg_basebackup doesn't ask for the db_name (passes null).
pg_receivelogical handles it being null already (and if it didn't,
it'd die with or without this patch).
pg_receivexlog expects it to be null and fails gracefully if it isn't.
So this change just removes some pointless noise.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, 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 10/11/15 07:35, Craig Ringer wrote:
On 10 November 2015 at 01:47, Marco Nenciarini
<marco.nenciarini@2ndquadrant.it> wrote:I've attached a little patch that removes the errors when connected to 9.3.
Looks good to me. No point confusing users.
The other callers of RunIdentifySystem are pg_basebackup and
pg_receivelogical.pg_basebackup doesn't ask for the db_name (passes null).
pg_receivelogical handles it being null already (and if it didn't,
it'd die with or without this patch).pg_receivexlog expects it to be null and fails gracefully if it isn't.
So this change just removes some pointless noise.
I've added it to the next commit fest to keep track of it. I've also set Craig as reviewer, as he has already sent a review for it (and it's a really simple patch).
Thanks,
Marco
--
Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco.nenciarini@2ndQuadrant.it | www.2ndQuadrant.it
On Tue, Nov 10, 2015 at 1:35 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
On 10 November 2015 at 01:47, Marco Nenciarini
<marco.nenciarini@2ndquadrant.it> wrote:I've attached a little patch that removes the errors when connected to 9.3.
Looks good to me. No point confusing users.
The other callers of RunIdentifySystem are pg_basebackup and
pg_receivelogical.pg_basebackup doesn't ask for the db_name (passes null).
pg_receivelogical handles it being null already (and if it didn't,
it'd die with or without this patch).pg_receivexlog expects it to be null and fails gracefully if it isn't.
So this change just removes some pointless noise.
The fprintf(stderr, ...) does not cause a non-local exit, so the
"else" just after it should be deleted. Otherwise, when that branch
is taken, *db_name doesn't get initialized at all.
Actually, I'd suggest doing it like the attached instead, which seems
a bit tighter.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:
dbname-warning-fix.patchtext/x-diff; charset=US-ASCII; name=dbname-warning-fix.patchDownload
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 2c963b6..d7b4499 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -294,15 +294,13 @@ RunIdentifySystem(PGconn *conn, char **sysid, TimeLineID *starttli,
/* Get database name, only available in 9.4 and newer versions */
if (db_name != NULL)
{
- if (PQnfields(res) < 4)
+ *db_name = NULL;
+ if (PQnfields(res) >= 4 && !PQgetisnull(res, 0, 3))
+ *db_name = pg_strdup(PQgetvalue(res, 0, 3));
+ else if (PQserverVersion(conn) >= 90400)
fprintf(stderr,
_("%s: could not identify system: got %d rows and %d fields, expected %d rows and %d or more fields\n"),
progname, PQntuples(res), PQnfields(res), 1, 4);
-
- if (PQgetisnull(res, 0, 3))
- *db_name = NULL;
- else
- *db_name = pg_strdup(PQgetvalue(res, 0, 3));
}
PQclear(res);
Hi Robert,
On 17/11/15 20:10, Robert Haas wrote:
On Tue, Nov 10, 2015 at 1:35 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
On 10 November 2015 at 01:47, Marco Nenciarini
<marco.nenciarini@2ndquadrant.it> wrote:I've attached a little patch that removes the errors when connected to 9.3.
Looks good to me. No point confusing users.
The other callers of RunIdentifySystem are pg_basebackup and
pg_receivelogical.pg_basebackup doesn't ask for the db_name (passes null).
pg_receivelogical handles it being null already (and if it didn't,
it'd die with or without this patch).pg_receivexlog expects it to be null and fails gracefully if it isn't.
So this change just removes some pointless noise.
The fprintf(stderr, ...) does not cause a non-local exit, so the
"else" just after it should be deleted. Otherwise, when that branch
is taken, *db_name doesn't get initialized at all.Actually, I'd suggest doing it like the attached instead, which seems
a bit tighter.
I agree, your patch is better.
Regards,
Marco
--
Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco.nenciarini@2ndQuadrant.it | www.2ndQuadrant.it
On Tue, Nov 24, 2015 at 7:00 PM, Marco Nenciarini
<marco.nenciarini@2ndquadrant.it> wrote:
Hi Robert,
On 17/11/15 20:10, Robert Haas wrote:
On Tue, Nov 10, 2015 at 1:35 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
On 10 November 2015 at 01:47, Marco Nenciarini
<marco.nenciarini@2ndquadrant.it> wrote:I've attached a little patch that removes the errors when connected to 9.3.
Looks good to me. No point confusing users.
The other callers of RunIdentifySystem are pg_basebackup and
pg_receivelogical.pg_basebackup doesn't ask for the db_name (passes null).
pg_receivelogical handles it being null already (and if it didn't,
it'd die with or without this patch).pg_receivexlog expects it to be null and fails gracefully if it isn't.
So this change just removes some pointless noise.
The fprintf(stderr, ...) does not cause a non-local exit, so the
"else" just after it should be deleted. Otherwise, when that branch
is taken, *db_name doesn't get initialized at all.Actually, I'd suggest doing it like the attached instead, which seems
a bit tighter.I agree, your patch is better.
+ else if (PQserverVersion(conn) >= 90400)
fprintf(stderr,
_("%s: could not identify system: got %d rows and
%d fields, expected %d rows and %d or more fields\n"),
progname, PQntuples(res), PQnfields(res), 1, 4);
}
In the above case, PQclear(res) should be called and FALSE should be returned?
Regards,
--
Fujii Masao
--
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, Nov 24, 2015 at 8:32 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Tue, Nov 24, 2015 at 7:00 PM, Marco Nenciarini
<marco.nenciarini@2ndquadrant.it> wrote:Hi Robert,
On 17/11/15 20:10, Robert Haas wrote:
On Tue, Nov 10, 2015 at 1:35 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
On 10 November 2015 at 01:47, Marco Nenciarini
<marco.nenciarini@2ndquadrant.it> wrote:I've attached a little patch that removes the errors when connected to 9.3.
Looks good to me. No point confusing users.
The other callers of RunIdentifySystem are pg_basebackup and
pg_receivelogical.pg_basebackup doesn't ask for the db_name (passes null).
pg_receivelogical handles it being null already (and if it didn't,
it'd die with or without this patch).pg_receivexlog expects it to be null and fails gracefully if it isn't.
So this change just removes some pointless noise.
The fprintf(stderr, ...) does not cause a non-local exit, so the
"else" just after it should be deleted. Otherwise, when that branch
is taken, *db_name doesn't get initialized at all.Actually, I'd suggest doing it like the attached instead, which seems
a bit tighter.I agree, your patch is better.
+ else if (PQserverVersion(conn) >= 90400)
fprintf(stderr,
_("%s: could not identify system: got %d rows and
%d fields, expected %d rows and %d or more fields\n"),
progname, PQntuples(res), PQnfields(res), 1, 4);
}In the above case, PQclear(res) should be called and FALSE should be returned?
Hmm, yeah, it looks like that would be more consistent with what the
other parts of this function do.
--
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 Wed, Nov 25, 2015 at 11:34 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Nov 24, 2015 at 8:32 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Tue, Nov 24, 2015 at 7:00 PM, Marco Nenciarini
<marco.nenciarini@2ndquadrant.it> wrote:Hi Robert,
On 17/11/15 20:10, Robert Haas wrote:
On Tue, Nov 10, 2015 at 1:35 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
On 10 November 2015 at 01:47, Marco Nenciarini
<marco.nenciarini@2ndquadrant.it> wrote:I've attached a little patch that removes the errors when connected to 9.3.
Looks good to me. No point confusing users.
The other callers of RunIdentifySystem are pg_basebackup and
pg_receivelogical.pg_basebackup doesn't ask for the db_name (passes null).
pg_receivelogical handles it being null already (and if it didn't,
it'd die with or without this patch).pg_receivexlog expects it to be null and fails gracefully if it isn't.
So this change just removes some pointless noise.
The fprintf(stderr, ...) does not cause a non-local exit, so the
"else" just after it should be deleted. Otherwise, when that branch
is taken, *db_name doesn't get initialized at all.Actually, I'd suggest doing it like the attached instead, which seems
a bit tighter.I agree, your patch is better.
+ else if (PQserverVersion(conn) >= 90400)
fprintf(stderr,
_("%s: could not identify system: got %d rows and
%d fields, expected %d rows and %d or more fields\n"),
progname, PQntuples(res), PQnfields(res), 1, 4);
}In the above case, PQclear(res) should be called and FALSE should be returned?
Hmm, yeah, it looks like that would be more consistent with what the
other parts of this function do.
The latest patch has another problem; pg_receivexlog trying to connect to
the PostgreSQL >= 9.4 always reports the following message unexpectedly.
could not identify system: got 1 rows and 4 fields, expected 1 rows
and 4 or more fields
This problem happens because the patch incorrectly treats the case where
IDENTIFY_SYSTEM command returns NULL as database name, as an error case.
Attached is the updated version of the patch, which fixes the problem.
Comments?
Regards,
--
Fujii Masao
Attachments:
dbname-warning-fix-fujii.patchtext/x-patch; charset=US-ASCII; name=dbname-warning-fix-fujii.patchDownload
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 2c963b6..39c616a 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -294,15 +294,21 @@ RunIdentifySystem(PGconn *conn, char **sysid, TimeLineID *starttli,
/* Get database name, only available in 9.4 and newer versions */
if (db_name != NULL)
{
- if (PQnfields(res) < 4)
- fprintf(stderr,
- _("%s: could not identify system: got %d rows and %d fields, expected %d rows and %d or more fields\n"),
- progname, PQntuples(res), PQnfields(res), 1, 4);
+ *db_name = NULL;
+ if (PQserverVersion(conn) >= 90400)
+ {
+ if (PQnfields(res) < 4)
+ {
+ fprintf(stderr,
+ _("%s: could not identify system: got %d rows and %d fields, expected %d rows and %d or more fields\n"),
+ progname, PQntuples(res), PQnfields(res), 1, 4);
- if (PQgetisnull(res, 0, 3))
- *db_name = NULL;
- else
- *db_name = pg_strdup(PQgetvalue(res, 0, 3));
+ PQclear(res);
+ return false;
+ }
+ if (!PQgetisnull(res, 0, 3))
+ *db_name = pg_strdup(PQgetvalue(res, 0, 3));
+ }
}
PQclear(res);
On Mon, Dec 7, 2015 at 11:37 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
The latest patch has another problem; pg_receivexlog trying to connect to
the PostgreSQL >= 9.4 always reports the following message unexpectedly.could not identify system: got 1 rows and 4 fields, expected 1 rows
and 4 or more fieldsThis problem happens because the patch incorrectly treats the case where
IDENTIFY_SYSTEM command returns NULL as database name, as an error case.Attached is the updated version of the patch, which fixes the problem.
Comments?
The patch looks good. The top comment of RunIdentifySystem is incorrect
though. It should mention that a database name is returned and not a plugin
name.
(not sure what happened with the last message, gmail went crazy for a
second removing the subject).
--
Michael
Michael Paquier wrote:
On Mon, Dec 7, 2015 at 11:37 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
The latest patch has another problem; pg_receivexlog trying to connect to
the PostgreSQL >= 9.4 always reports the following message unexpectedly.could not identify system: got 1 rows and 4 fields, expected 1 rows
and 4 or more fieldsThis problem happens because the patch incorrectly treats the case where
IDENTIFY_SYSTEM command returns NULL as database name, as an error case.Attached is the updated version of the patch, which fixes the problem.
Comments?The patch looks good. The top comment of RunIdentifySystem is incorrect
though. It should mention that a database name is returned and not a plugin
name.
Right. Pushed with that change to 9.5 and master. Thanks!
--
�lvaro Herrera 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