On 02.10.21 16:31, Fabien COELHO wrote:
Attached v9 integrates your tests and makes them work.
Attached v11 is a rebase.
This patch still has a few of the problems reported earlier this year.
In [0], it was reported that certain replication commands result in
infinite loops because of faulty error handling. This still happens. I
wrote a test for it, attached here. (I threw in a few more basic tests,
just to have some more coverage that was lacking, and to have a file to
put the new test in.)
In [1], it was reported that server crashes produce duplicate error
messages. This also still happens. I didn't write a test for it, maybe
you have an idea. (Obviously, we could check whether the error message
is literally there twice in the output, but that doesn't seem very
general.) But it's easy to test manually: just have psql connect, shut
down the server, then run a query.
Additionally, I looked into the Coverity issue reported in [2]. That
one is fixed, but I figured it would be good to be able to check your
patches with a static analyzer in a similar way. I don't have the
ability to run Coverity locally, so I looked at scan-build and fixed a
few minor warnings, also attached as a patch. Your current patch
appears to be okay in that regard.
[0]:
https://www.postgresql.org/message-id/69c0b369-570c-4524-8ee4-bccacecb6...@amazon.com
[1]: https://www.postgresql.org/message-id/2902362.1618244...@sss.pgh.pa.us
[2]: https://www.postgresql.org/message-id/2680034.1618157...@sss.pgh.pa.us
>From e8dbe137737f94a2eaff86dc1676f9df39c60d00 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 7 Oct 2021 22:12:40 +0200
Subject: [PATCH] psql: More tests
---
src/bin/psql/t/001_basic.pl | 42 +++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
create mode 100644 src/bin/psql/t/001_basic.pl
diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
new file mode 100644
index 0000000000..cd899e851e
--- /dev/null
+++ b/src/bin/psql/t/001_basic.pl
@@ -0,0 +1,42 @@
+
+# Copyright (c) 2021, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+
+use PostgresNode;
+use TestLib;
+use Test::More tests => 25;
+
+program_help_ok('psql');
+program_version_ok('psql');
+program_options_handling_ok('psql');
+
+my ($stdout, $stderr);
+my $result;
+
+# test --help=foo, analogous to program_help_ok()
+foreach my $arg (qw(commands variables))
+{
+ $result = IPC::Run::run [ 'psql', "--help=$arg" ], '>', \$stdout, '2>',
\$stderr;
+ ok($result, "psql --help=$arg exit code 0");
+ isnt($stdout, '', "psql --help=$arg goes to stdout");
+ is($stderr, '', "psql --help=$arg nothing to stderr");
+}
+
+my $node = PostgresNode->new('main');
+$node->init;
+$node->append_conf(
+ 'postgresql.conf', q{
+wal_level = 'logical'
+max_replication_slots = 4
+max_wal_senders = 4
+});
+$node->start;
+
+$node->command_like([ 'psql', '-c', '\copyright' ], qr/Copyright/,
'\copyright');
+$node->command_like([ 'psql', '-c', '\help' ], qr/ALTER/, '\help without
arguments');
+$node->command_like([ 'psql', '-c', '\help SELECT' ], qr/SELECT/, '\help');
+
+$node->command_fails_like([ 'psql', 'replication=database', '-c',
'START_REPLICATION 123/456' ],
+ qr/^unexpected PQresultStatus: 8$/, 'handling of unexpected
PQresultStatus');
--
2.33.0
>From 96634e3dc5f775b73a4142e9a5d83190bd9aecbb Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 8 Oct 2021 13:30:15 +0200
Subject: [PATCH] psql: Fix scan-build warnings
---
src/bin/psql/common.c | 32 ++++++++++++++++++--------------
src/bin/psql/copy.c | 1 -
src/bin/psql/describe.c | 1 -
3 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 5640786678..1b224bf9e4 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -594,6 +594,7 @@ PSQLexec(const char *query)
int
PSQLexecWatch(const char *query, const printQueryOpt *opt, FILE
*printQueryFout)
{
+ bool timing = pset.timing;
PGresult *res;
double elapsed_msec = 0;
instr_time before;
@@ -608,7 +609,7 @@ PSQLexecWatch(const char *query, const printQueryOpt *opt,
FILE *printQueryFout)
SetCancelConn(pset.db);
- if (pset.timing)
+ if (timing)
INSTR_TIME_SET_CURRENT(before);
res = PQexec(pset.db, query);
@@ -621,7 +622,7 @@ PSQLexecWatch(const char *query, const printQueryOpt *opt,
FILE *printQueryFout)
return 0;
}
- if (pset.timing)
+ if (timing)
{
INSTR_TIME_SET_CURRENT(after);
INSTR_TIME_SUBTRACT(after, before);
@@ -674,7 +675,7 @@ PSQLexecWatch(const char *query, const printQueryOpt *opt,
FILE *printQueryFout)
fflush(fout);
/* Possible microtiming output */
- if (pset.timing)
+ if (timing)
PrintTiming(elapsed_msec);
return 1;
@@ -1192,6 +1193,7 @@ PrintQueryResults(PGresult *results)
bool
SendQuery(const char *query)
{
+ bool timing = pset.timing;
PGresult *results;
PGTransactionStatusType transaction_status;
double elapsed_msec = 0;
@@ -1300,7 +1302,7 @@ SendQuery(const char *query)
instr_time before,
after;
- if (pset.timing)
+ if (timing)
INSTR_TIME_SET_CURRENT(before);
results = PQexec(pset.db, query);
@@ -1309,7 +1311,7 @@ SendQuery(const char *query)
ResetCancelConn();
OK = ProcessResult(&results);
- if (pset.timing)
+ if (timing)
{
INSTR_TIME_SET_CURRENT(after);
INSTR_TIME_SUBTRACT(after, before);
@@ -1400,7 +1402,7 @@ SendQuery(const char *query)
ClearOrSaveResult(results);
/* Possible microtiming output */
- if (pset.timing)
+ if (timing)
PrintTiming(elapsed_msec);
/* check for events that may occur during query execution */
@@ -1471,6 +1473,7 @@ SendQuery(const char *query)
static bool
DescribeQuery(const char *query, double *elapsed_msec)
{
+ bool timing = pset.timing;
PGresult *results;
bool OK;
instr_time before,
@@ -1478,7 +1481,7 @@ DescribeQuery(const char *query, double *elapsed_msec)
*elapsed_msec = 0;
- if (pset.timing)
+ if (timing)
INSTR_TIME_SET_CURRENT(before);
/*
@@ -1550,7 +1553,7 @@ DescribeQuery(const char *query, double *elapsed_msec)
results = PQexec(pset.db, buf.data);
OK = AcceptResult(results);
- if (pset.timing)
+ if (timing)
{
INSTR_TIME_SET_CURRENT(after);
INSTR_TIME_SUBTRACT(after, before);
@@ -1591,6 +1594,7 @@ ExecQueryUsingCursor(const char *query, double
*elapsed_msec)
PGresult *results;
PQExpBufferData buf;
printQueryOpt my_popt = pset.popt;
+ bool timing = pset.timing;
FILE *fout;
bool is_pipe;
bool is_pager = false;
@@ -1610,7 +1614,7 @@ ExecQueryUsingCursor(const char *query, double
*elapsed_msec)
my_popt.topt.stop_table = false;
my_popt.topt.prior_records = 0;
- if (pset.timing)
+ if (timing)
INSTR_TIME_SET_CURRENT(before);
/* if we're not in a transaction, start one */
@@ -1640,7 +1644,7 @@ ExecQueryUsingCursor(const char *query, double
*elapsed_msec)
if (!OK)
goto cleanup;
- if (pset.timing)
+ if (timing)
{
INSTR_TIME_SET_CURRENT(after);
INSTR_TIME_SUBTRACT(after, before);
@@ -1682,13 +1686,13 @@ ExecQueryUsingCursor(const char *query, double
*elapsed_msec)
for (;;)
{
- if (pset.timing)
+ if (timing)
INSTR_TIME_SET_CURRENT(before);
/* get fetch_count tuples at a time */
results = PQexec(pset.db, fetch_cmd);
- if (pset.timing)
+ if (timing)
{
INSTR_TIME_SET_CURRENT(after);
INSTR_TIME_SUBTRACT(after, before);
@@ -1802,7 +1806,7 @@ ExecQueryUsingCursor(const char *query, double
*elapsed_msec)
}
cleanup:
- if (pset.timing)
+ if (timing)
INSTR_TIME_SET_CURRENT(before);
/*
@@ -1828,7 +1832,7 @@ ExecQueryUsingCursor(const char *query, double
*elapsed_msec)
ClearOrSaveResult(results);
}
- if (pset.timing)
+ if (timing)
{
INSTR_TIME_SET_CURRENT(after);
INSTR_TIME_SUBTRACT(after, before);
diff --git a/src/bin/psql/copy.c b/src/bin/psql/copy.c
index 64ab40c4f7..3c4d862bdf 100644
--- a/src/bin/psql/copy.c
+++ b/src/bin/psql/copy.c
@@ -660,7 +660,6 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary,
PGresult **res)
if (PQputCopyData(conn, buf, buflen) <= 0)
{
OK = false;
- copydone = true;
break;
}
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index a33d77c0ef..ea4ca5c05c 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -635,7 +635,6 @@ describeFunctions(const char *functypes, const char
*func_pattern,
appendPQExpBufferStr(&buf, "p.prokind = 'w'\n");
else
appendPQExpBufferStr(&buf, "p.proiswindow\n");
- needs_or = true;
}
appendPQExpBufferStr(&buf, " )\n");
}
--
2.33.0
