public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v7 2/2] Add a --outdated option to reindexdb
7+ messages / 4 participants
[nested] [flat]
* [PATCH v7 2/2] Add a --outdated option to reindexdb
@ 2021-02-24 17:33 Julien Rouhaud <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Julien Rouhaud @ 2021-02-24 17:33 UTC (permalink / raw)
This uses the new OUTDATED option for REINDEX. If user asks for multiple job,
the list of tables to process will be sorted by the total size of underlying
indexes that have outdated dependency using a new
pg_index_has_outdated_dependency(regclass) SQL function.
Catversion (should be) bumped.
Author: Julien Rouhaud <[email protected]>
Reviewed-by: Michael Paquier, Mark Dilger
Discussion: https://postgr.es/m/20201203093143.GA64934%40nol
---
doc/src/sgml/func.sgml | 27 ++++--
src/backend/catalog/index.c | 22 +++++
src/bin/scripts/reindexdb.c | 143 ++++++++++++++++++++++++-----
src/bin/scripts/t/090_reindexdb.pl | 34 ++++++-
src/include/catalog/pg_proc.dat | 4 +
5 files changed, 198 insertions(+), 32 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 9492a3c6b9..0eda6678ac 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -26448,12 +26448,13 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size
<para>
<xref linkend="functions-admin-index-table"/> shows the functions
- available for index maintenance tasks. (Note that these maintenance
- tasks are normally done automatically by autovacuum; use of these
- functions is only required in special cases.)
- These functions cannot be executed during recovery.
- Use of these functions is restricted to superusers and the owner
- of the given index.
+ available for index maintenance tasks. (Note that the maintenance
+ tasks performing actions on indexes are normally done automatically by
+ autovacuum; use of these functions is only required in special cases.)
+ The functions performing actions on indexes cannot be executed during
+ recovery.
+ Use of the functions performing actions on indexes is restricted to
+ superusers and the owner of the given index.
</para>
<table id="functions-admin-index-table">
@@ -26538,6 +26539,20 @@ SELECT pg_size_pretty(sum(pg_relation_size(relid))) AS total_size
option.
</para></entry>
</row>
+
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>pg_index_has_outdated_dependency</primary>
+ </indexterm>
+ <function>pg_index_has_outdated_dependency</function> ( <parameter>index</parameter> <type>regclass</type> )
+ <returnvalue>boolean</returnvalue>
+ </para>
+ <para>
+ Check if the specified index has any outdated dependency. For now only
+ dependency on collations are supported.
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 47e6a54149..6848e61df3 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1390,6 +1390,28 @@ do_check_index_has_outdated_collation(const ObjectAddress *otherObject,
return false;
}
+/*
+ * SQL wrapper around index_has_outdated_dependency.
+ */
+Datum
+pg_index_has_outdated_dependency(PG_FUNCTION_ARGS)
+{
+ Oid indexOid = PG_GETARG_OID(0);
+ Relation rel;
+ bool res;
+
+ rel = try_index_open(indexOid, AccessShareLock);
+
+ if (rel == NULL)
+ PG_RETURN_NULL();
+
+ res = index_has_outdated_dependency(indexOid);
+
+ relation_close(rel, AccessShareLock);
+
+ PG_RETURN_BOOL(res);
+}
+
/*
* Check whether the given index has a dependency with an outdated
* collation version.
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index fc0681538a..d252b1454f 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -35,20 +35,23 @@ typedef enum ReindexType
static SimpleStringList *get_parallel_object_list(PGconn *conn,
ReindexType type,
SimpleStringList *user_list,
+ bool outdated,
bool echo);
static void reindex_one_database(ConnParams *cparams, ReindexType type,
SimpleStringList *user_list,
const char *progname,
bool echo, bool verbose, bool concurrently,
- int concurrentCons, const char *tablespace);
+ int concurrentCons, const char *tablespace,
+ bool outdated);
static void reindex_all_databases(ConnParams *cparams,
const char *progname, bool echo,
bool quiet, bool verbose, bool concurrently,
- int concurrentCons, const char *tablespace);
+ int concurrentCons, const char *tablespace,
+ bool outdated);
static void run_reindex_command(PGconn *conn, ReindexType type,
const char *name, bool echo, bool verbose,
bool concurrently, bool async,
- const char *tablespace);
+ const char *tablespace, bool outdated);
static void help(const char *progname);
@@ -74,6 +77,7 @@ main(int argc, char *argv[])
{"concurrently", no_argument, NULL, 1},
{"maintenance-db", required_argument, NULL, 2},
{"tablespace", required_argument, NULL, 3},
+ {"outdated", no_argument, NULL, 4},
{NULL, 0, NULL, 0}
};
@@ -95,6 +99,7 @@ main(int argc, char *argv[])
bool quiet = false;
bool verbose = false;
bool concurrently = false;
+ bool outdated = false;
SimpleStringList indexes = {NULL, NULL};
SimpleStringList tables = {NULL, NULL};
SimpleStringList schemas = {NULL, NULL};
@@ -170,6 +175,9 @@ main(int argc, char *argv[])
case 3:
tablespace = pg_strdup(optarg);
break;
+ case 4:
+ outdated = true;
+ break;
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit(1);
@@ -234,7 +242,8 @@ main(int argc, char *argv[])
cparams.dbname = maintenance_db;
reindex_all_databases(&cparams, progname, echo, quiet, verbose,
- concurrently, concurrentCons, tablespace);
+ concurrently, concurrentCons, tablespace,
+ outdated);
}
else if (syscatalog)
{
@@ -253,12 +262,17 @@ main(int argc, char *argv[])
pg_log_error("cannot reindex specific index(es) and system catalogs at the same time");
exit(1);
}
-
if (concurrentCons > 1)
{
pg_log_error("cannot use multiple jobs to reindex system catalogs");
exit(1);
}
+ if (outdated)
+ {
+ pg_log_error("cannot filter indexes having outdated dependencies "
+ "and reindex system catalogs at the same time");
+ exit(1);
+ }
if (dbname == NULL)
{
@@ -274,7 +288,7 @@ main(int argc, char *argv[])
reindex_one_database(&cparams, REINDEX_SYSTEM, NULL,
progname, echo, verbose,
- concurrently, 1, tablespace);
+ concurrently, 1, tablespace, outdated);
}
else
{
@@ -304,17 +318,20 @@ main(int argc, char *argv[])
if (schemas.head != NULL)
reindex_one_database(&cparams, REINDEX_SCHEMA, &schemas,
progname, echo, verbose,
- concurrently, concurrentCons, tablespace);
+ concurrently, concurrentCons, tablespace,
+ outdated);
if (indexes.head != NULL)
reindex_one_database(&cparams, REINDEX_INDEX, &indexes,
progname, echo, verbose,
- concurrently, 1, tablespace);
+ concurrently, 1, tablespace,
+ outdated);
if (tables.head != NULL)
reindex_one_database(&cparams, REINDEX_TABLE, &tables,
progname, echo, verbose,
- concurrently, concurrentCons, tablespace);
+ concurrently, concurrentCons, tablespace,
+ outdated);
/*
* reindex database only if neither index nor table nor schema is
@@ -323,7 +340,8 @@ main(int argc, char *argv[])
if (indexes.head == NULL && tables.head == NULL && schemas.head == NULL)
reindex_one_database(&cparams, REINDEX_DATABASE, NULL,
progname, echo, verbose,
- concurrently, concurrentCons, tablespace);
+ concurrently, concurrentCons, tablespace,
+ outdated);
}
exit(0);
@@ -334,7 +352,7 @@ reindex_one_database(ConnParams *cparams, ReindexType type,
SimpleStringList *user_list,
const char *progname, bool echo,
bool verbose, bool concurrently, int concurrentCons,
- const char *tablespace)
+ const char *tablespace, bool outdated)
{
PGconn *conn;
SimpleStringListCell *cell;
@@ -363,6 +381,14 @@ reindex_one_database(ConnParams *cparams, ReindexType type,
exit(1);
}
+ if (outdated && PQserverVersion(conn) < 140000)
+ {
+ PQfinish(conn);
+ pg_log_error("cannot use the \"%s\" option on server versions older than PostgreSQL %s",
+ "outdated", "14");
+ exit(1);
+ }
+
if (!parallel)
{
switch (process_type)
@@ -399,14 +425,24 @@ reindex_one_database(ConnParams *cparams, ReindexType type,
*/
if (concurrently)
pg_log_warning("cannot reindex system catalogs concurrently, skipping all");
+ else if (outdated)
+ {
+ /*
+ * The only supported kind of object that can be outdated
+ * is collation. No system catalog has any index that can
+ * depend on an outdated collation, so skip system
+ * catalogs.
+ */
+ }
else
run_reindex_command(conn, REINDEX_SYSTEM, PQdb(conn), echo,
verbose, concurrently, false,
- tablespace);
+ tablespace, outdated);
/* Build a list of relations from the database */
process_list = get_parallel_object_list(conn, process_type,
- user_list, echo);
+ user_list, outdated,
+ echo);
process_type = REINDEX_TABLE;
/* Bail out if nothing to process */
@@ -419,7 +455,8 @@ reindex_one_database(ConnParams *cparams, ReindexType type,
/* Build a list of relations from all the schemas */
process_list = get_parallel_object_list(conn, process_type,
- user_list, echo);
+ user_list, outdated,
+ echo);
process_type = REINDEX_TABLE;
/* Bail out if nothing to process */
@@ -485,7 +522,8 @@ reindex_one_database(ConnParams *cparams, ReindexType type,
ParallelSlotSetHandler(free_slot, TableCommandResultHandler, NULL);
run_reindex_command(free_slot->connection, process_type, objname,
- echo, verbose, concurrently, true, tablespace);
+ echo, verbose, concurrently, true, tablespace,
+ outdated);
cell = cell->next;
} while (cell != NULL);
@@ -510,7 +548,7 @@ finish:
static void
run_reindex_command(PGconn *conn, ReindexType type, const char *name,
bool echo, bool verbose, bool concurrently, bool async,
- const char *tablespace)
+ const char *tablespace, bool outdated)
{
const char *paren = "(";
const char *comma = ", ";
@@ -537,6 +575,12 @@ run_reindex_command(PGconn *conn, ReindexType type, const char *name,
sep = comma;
}
+ if (outdated)
+ {
+ appendPQExpBuffer(&sql, "%sOUTDATED", sep);
+ sep = comma;
+ }
+
if (sep != paren)
appendPQExpBufferStr(&sql, ") ");
@@ -642,7 +686,7 @@ run_reindex_command(PGconn *conn, ReindexType type, const char *name,
*/
static SimpleStringList *
get_parallel_object_list(PGconn *conn, ReindexType type,
- SimpleStringList *user_list, bool echo)
+ SimpleStringList *user_list, bool outdated, bool echo)
{
PQExpBufferData catalog_query;
PQExpBufferData buf;
@@ -661,16 +705,41 @@ get_parallel_object_list(PGconn *conn, ReindexType type,
{
case REINDEX_DATABASE:
Assert(user_list == NULL);
+
appendPQExpBufferStr(&catalog_query,
"SELECT c.relname, ns.nspname\n"
" FROM pg_catalog.pg_class c\n"
" JOIN pg_catalog.pg_namespace ns"
- " ON c.relnamespace = ns.oid\n"
+ " ON c.relnamespace = ns.oid\n");
+
+ if (outdated)
+ {
+ appendPQExpBufferStr(&catalog_query,
+ " JOIN pg_catalog.pg_index i"
+ " ON c.oid = i.indrelid\n"
+ " JOIN pg_catalog.pg_class ci"
+ " ON i.indexrelid = ci.oid\n");
+ }
+
+ appendPQExpBufferStr(&catalog_query,
" WHERE ns.nspname != 'pg_catalog'\n"
" AND c.relkind IN ("
CppAsString2(RELKIND_RELATION) ", "
- CppAsString2(RELKIND_MATVIEW) ")\n"
- " ORDER BY c.relpages DESC;");
+ CppAsString2(RELKIND_MATVIEW) ")\n");
+
+ if (outdated)
+ {
+ appendPQExpBufferStr(&catalog_query,
+ " GROUP BY c.relname, ns.nspname\n"
+ " ORDER BY pg_catalog.sum(ci.relpages)"
+ " FILTER (WHERE pg_catalog.pg_index_has_outdated_dependency(ci.oid)) DESC;");
+ }
+ else
+ {
+ appendPQExpBufferStr(&catalog_query,
+ " ORDER BY c.relpages DESC;");
+ }
+
break;
case REINDEX_SCHEMA:
@@ -688,7 +757,18 @@ get_parallel_object_list(PGconn *conn, ReindexType type,
"SELECT c.relname, ns.nspname\n"
" FROM pg_catalog.pg_class c\n"
" JOIN pg_catalog.pg_namespace ns"
- " ON c.relnamespace = ns.oid\n"
+ " ON c.relnamespace = ns.oid\n");
+
+ if (outdated)
+ {
+ appendPQExpBufferStr(&catalog_query,
+ " JOIN pg_catalog.pg_index i"
+ " ON c.oid = i.indrelid\n"
+ " JOIN pg_catalog.pg_class ci"
+ " ON i.indexrelid = ci.oid\n");
+ }
+
+ appendPQExpBufferStr(&catalog_query,
" WHERE c.relkind IN ("
CppAsString2(RELKIND_RELATION) ", "
CppAsString2(RELKIND_MATVIEW) ")\n"
@@ -706,8 +786,20 @@ get_parallel_object_list(PGconn *conn, ReindexType type,
appendStringLiteralConn(&catalog_query, nspname, conn);
}
- appendPQExpBufferStr(&catalog_query, ")\n"
- " ORDER BY c.relpages DESC;");
+ appendPQExpBufferStr(&catalog_query, ")\n");
+
+ if (outdated)
+ {
+ appendPQExpBufferStr(&catalog_query,
+ " GROUP BY c.relname, ns.nspname\n"
+ " ORDER BY pg_catalog.sum(ci.relpages)"
+ " FILTER (WHERE pg_catalog.pg_index_has_outdated_dependency(ci.oid)) DESC;");
+ }
+ else
+ {
+ appendPQExpBufferStr(&catalog_query,
+ " ORDER BY c.relpages DESC;");
+ }
}
break;
@@ -755,7 +847,7 @@ static void
reindex_all_databases(ConnParams *cparams,
const char *progname, bool echo, bool quiet, bool verbose,
bool concurrently, int concurrentCons,
- const char *tablespace)
+ const char *tablespace, bool outdated)
{
PGconn *conn;
PGresult *result;
@@ -779,7 +871,7 @@ reindex_all_databases(ConnParams *cparams,
reindex_one_database(cparams, REINDEX_DATABASE, NULL,
progname, echo, verbose, concurrently,
- concurrentCons, tablespace);
+ concurrentCons, tablespace, outdated);
}
PQclear(result);
@@ -798,6 +890,7 @@ help(const char *progname)
printf(_(" -e, --echo show the commands being sent to the server\n"));
printf(_(" -i, --index=INDEX recreate specific index(es) only\n"));
printf(_(" -j, --jobs=NUM use this many concurrent connections to reindex\n"));
+ printf(_(" --outdated only process indexes having outdated dependencies\n"));
printf(_(" -q, --quiet don't write any messages\n"));
printf(_(" -s, --system reindex system catalogs\n"));
printf(_(" -S, --schema=SCHEMA reindex specific schema(s) only\n"));
diff --git a/src/bin/scripts/t/090_reindexdb.pl b/src/bin/scripts/t/090_reindexdb.pl
index 159b637230..91fd602041 100644
--- a/src/bin/scripts/t/090_reindexdb.pl
+++ b/src/bin/scripts/t/090_reindexdb.pl
@@ -3,7 +3,7 @@ use warnings;
use PostgresNode;
use TestLib;
-use Test::More tests => 58;
+use Test::More tests => 70;
program_help_ok('reindexdb');
program_version_ok('reindexdb');
@@ -174,6 +174,9 @@ $node->command_fails(
$node->command_fails(
[ 'reindexdb', '-j', '2', '-i', 'i1', 'postgres' ],
'parallel reindexdb cannot process indexes');
+$node->command_fails(
+ [ 'reindexdb', '-s', '--outdated' ],
+ 'cannot reindex system catalog and filter indexes having outdated dependencies');
$node->issues_sql_like(
[ 'reindexdb', '-j', '2', 'postgres' ],
qr/statement:\ REINDEX SYSTEM postgres;
@@ -196,3 +199,32 @@ $node->command_checks_all(
qr/^reindexdb: warning: cannot reindex system catalogs concurrently, skipping all/s
],
'parallel reindexdb for system with --concurrently skips catalogs');
+
+# Temporarily downgrade client-min-message to get the no-op report
+$ENV{PGOPTIONS} = '--client-min-messages=NOTICE';
+$node->command_checks_all(
+ [ 'reindexdb', '--outdated', '-v', '-t', 's1.t1', 'postgres' ],
+ 0,
+ [qr/^$/],
+ [qr/table "t1" has no indexes to reindex/],
+ 'verbose reindexdb for outdated dependencies on a specific table reports no-op tables');
+
+$node->command_checks_all(
+ [ 'reindexdb', '--outdated', '-v', '-d', 'postgres' ],
+ 0,
+ [qr/^$/],
+ [qr/index "t2_id_idx" has no outdated dependency/],
+ 'verbose reindexdb for outdated dependencies database wide reports all ignored indexes');
+$node->command_checks_all(
+ [ 'reindexdb', '--outdated', '-v', '-j', '2', '-d', 'postgres' ],
+ 0,
+ [qr/^$/],
+ [qr/table "t1" has no indexes to reindex/],
+ 'parallel verbose reindexdb for outdated dependencies database wide reports no-op tables');
+
+# Switch back to WARNING client-min-message
+$ENV{PGOPTIONS} = '--client-min-messages=WARNING';
+$node->issues_sql_like(
+ [ 'reindexdb', '--outdated', '-t', 's1.t1', 'postgres' ],
+ qr/.*statement: REINDEX \(OUTDATED\) TABLE s1\.t1;/,
+ 'reindexdb for outdated dependencies specify the OUTDATED keyword');
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 93393fcfd4..911c12ee2c 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -949,6 +949,10 @@
proname => 'pg_indexam_has_property', provolatile => 's',
prorettype => 'bool', proargtypes => 'oid text',
prosrc => 'pg_indexam_has_property' },
+{ oid => '8102', descr => 'test property of an index',
+ proname => 'pg_index_has_outdated_dependency', provolatile => 's',
+ prorettype => 'bool', proargtypes => 'regclass',
+ prosrc => 'pg_index_has_outdated_dependency' },
{ oid => '637', descr => 'test property of an index',
proname => 'pg_index_has_property', provolatile => 's', prorettype => 'bool',
proargtypes => 'regclass text', prosrc => 'pg_index_has_property' },
--
2.30.1
--rpt4sswdwkhtp4fw--
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: convert libpq uri-regress tests to tap test
@ 2022-02-24 12:31 Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Peter Eisentraut @ 2022-02-24 12:31 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; pgsql-hackers
On 24.02.22 02:52, Tom Lane wrote:
> Peter Eisentraut <[email protected]> writes:
>> On 23.02.22 23:58, Tom Lane wrote:
>>> Peter Eisentraut <[email protected]> writes:
>>>> libpq TAP tests should be in src/interfaces/libpq/t/.
>
>>> That's failing to account for the fact that a libpq test can't
>>> really be a pure-perl TAP test; you need some C code to drive the
>>> library.
>
>> Such things could be put under src/interfaces/libpq/test, or some other
>> subdirectory. We already have src/interfaces/ecpg/test.
>
> OK, but then the TAP scripts are under src/interfaces/libpq/test/t,
> which isn't what you said. I have no great objection to moving
> src/test/modules/libpq_pipeline/ to src/interfaces/libpq/test/,
> though, as long as the buildfarm will cope.
I think the TAP scripts should be in src/interfaces/libpq/t/, as usual.
The supporting code snippets could live in some other directory under
src/interfaces/libpq/, which might be called "test" or something else,
not that important.
I think we should pick a layout that is proper and future-proof and then
adjust the buildfarm client as necessary. The issue of writing
libpq-specific tests has come up a few times recently; I think it would
be worth finding a proper solution to this that would facilitate that
work in the future.
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: convert libpq uri-regress tests to tap test
@ 2022-02-24 14:43 Andres Freund <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Andres Freund @ 2022-02-24 14:43 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Tom Lane <[email protected]>; pgsql-hackers
Hi,
On 2022-02-24 13:31:40 +0100, Peter Eisentraut wrote:
> I think the TAP scripts should be in src/interfaces/libpq/t/, as usual. The
> supporting code snippets could live in some other directory under
> src/interfaces/libpq/, which might be called "test" or something else, not
> that important.
Why not in t/? We can't easily build the test programs in libpq/ itself, but
libpq/t should be fairly doable.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: convert libpq uri-regress tests to tap test
@ 2022-02-24 15:17 Tom Lane <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 2 replies; 7+ messages in thread
From: Tom Lane @ 2022-02-24 15:17 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers
Andres Freund <[email protected]> writes:
> On 2022-02-24 13:31:40 +0100, Peter Eisentraut wrote:
>> I think the TAP scripts should be in src/interfaces/libpq/t/, as usual. The
>> supporting code snippets could live in some other directory under
>> src/interfaces/libpq/, which might be called "test" or something else, not
>> that important.
> Why not in t/? We can't easily build the test programs in libpq/ itself, but
> libpq/t should be fairly doable.
I think that having t/ directories contain only Perl test scripts
is a good convention that we should stick to. Peter's proposal
of a separate test/ subdirectory for C test scaffolding is
probably fine.
regards, tom lane
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: convert libpq uri-regress tests to tap test
@ 2022-02-24 16:46 Andres Freund <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 1 reply; 7+ messages in thread
From: Andres Freund @ 2022-02-24 16:46 UTC (permalink / raw)
To: Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers
Hi,
On 2022-02-24 10:17:28 -0500, Tom Lane wrote:
> Andres Freund <[email protected]> writes:
> > On 2022-02-24 13:31:40 +0100, Peter Eisentraut wrote:
> >> I think the TAP scripts should be in src/interfaces/libpq/t/, as usual. The
> >> supporting code snippets could live in some other directory under
> >> src/interfaces/libpq/, which might be called "test" or something else, not
> >> that important.
>
> > Why not in t/? We can't easily build the test programs in libpq/ itself, but
> > libpq/t should be fairly doable.
>
> I think that having t/ directories contain only Perl test scripts
> is a good convention that we should stick to. Peter's proposal
> of a separate test/ subdirectory for C test scaffolding is
> probably fine.
That exists today and continues to exist in the patch upthread, so it's easy
;). I just need to move the libpq/test/t to libpq/t and adjust the binary
path.
One annoying bit is that our current tap invocation infrastructure for msvc
won't know how to deal with that. We put the build directory containing t/
onto PATH, but that won't work for test/. But we also don't want to install
test binaries. Not sure what the solution for that is.
Even on !windows, we only know how to find "test executables" in tap tests via
PATH. We're in the source dir, so we can't just do test/executable.
I probably just need another coffee, but right now I don't even see how to add
anything to PATH given $(prove_check)'s definition - it ends up with multiple
shells. We can fix that by using && in the definition, which might be a good
thing anyway?
Attached three patches:
0001: Convert src/interfaces/libpq/test to a tap test
0002: Add tap test infrastructure to src/interfaces/libpq
0003: Move libpq_pipeline test into src/interfaces/libpq.
I did 0001 and 0002 in that order because prove errors out with a stacktrace
if no tap tests exist... It might make more sense to commit them together, but
for review it's easier to keep them separate I think.
Andrew, do you have an idea about the feasibility of supporting any of this
with the msvc build?
I'm mildly inclined to only do 0001 and 0002 for now. We'd not loose msvc
coverage, because it already doesn't build the test. Once we've ironed that
stuff out, we could do 0003?
Greetings,
Andres Freund
Attachments:
[text/x-diff] v2-0001-Convert-src-interfaces-libpq-test-to-a-tap-test.patch (18.0K, ../../[email protected]/2-v2-0001-Convert-src-interfaces-libpq-test-to-a-tap-test.patch)
download | inline diff:
From 71fa1532a1540e8bbf8bdee3ec0b64e863f212f2 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Wed, 23 Feb 2022 12:22:56 -0800
Subject: [PATCH v2 1/3] Convert src/interfaces/libpq/test to a tap test.
The invocation of the tap test will be added in the next commit.
---
src/interfaces/libpq/t/001_uri.pl | 265 +++++++++++++++++++++++++
src/interfaces/libpq/test/.gitignore | 2 -
src/interfaces/libpq/test/Makefile | 7 +-
src/interfaces/libpq/test/README | 7 -
src/interfaces/libpq/test/expected.out | 171 ----------------
src/interfaces/libpq/test/regress.in | 57 ------
src/interfaces/libpq/test/regress.pl | 65 ------
7 files changed, 267 insertions(+), 307 deletions(-)
create mode 100644 src/interfaces/libpq/t/001_uri.pl
delete mode 100644 src/interfaces/libpq/test/README
delete mode 100644 src/interfaces/libpq/test/expected.out
delete mode 100644 src/interfaces/libpq/test/regress.in
delete mode 100644 src/interfaces/libpq/test/regress.pl
diff --git a/src/interfaces/libpq/t/001_uri.pl b/src/interfaces/libpq/t/001_uri.pl
new file mode 100644
index 00000000000..0225666d9f6
--- /dev/null
+++ b/src/interfaces/libpq/t/001_uri.pl
@@ -0,0 +1,265 @@
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Utils;
+use Test::More;
+use IPC::Run;
+
+my @tests = (
+ [q{postgresql://uri-user:secret@host:12345/db},
+ q{user='uri-user' password='secret' dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://uri-user@host:12345/db},
+ q{user='uri-user' dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://uri-user@host/db},
+ q{user='uri-user' dbname='db' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host:12345/db},
+ q{dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db},
+ q{dbname='db' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://uri-user@host:12345/},
+ q{user='uri-user' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://uri-user@host/},
+ q{user='uri-user' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://uri-user@},
+ q{user='uri-user' (local)},
+ q{},
+ ],
+ [q{postgresql://host:12345/},
+ q{host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host:12345},
+ q{host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db},
+ q{dbname='db' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://},
+ q{(local)},
+ q{},
+ ],
+ [q{postgresql://?hostaddr=127.0.0.1},
+ q{hostaddr='127.0.0.1' (inet)},
+ q{},
+ ],
+ [q{postgresql://example.com?hostaddr=63.1.2.4},
+ q{host='example.com' hostaddr='63.1.2.4' (inet)},
+ q{},
+ ],
+ [q{postgresql://%68ost/},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db?user=uri-user},
+ q{user='uri-user' dbname='db' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db?user=uri-user&port=12345},
+ q{user='uri-user' dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db?u%73er=someotheruser&port=12345},
+ q{user='someotheruser' dbname='db' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host/db?u%7aer=someotheruser&port=12345},
+ q{},
+ q{uri-regress: invalid URI query parameter: "uzer"},
+ ],
+ [q{postgresql://host:12345?user=uri-user},
+ q{user='uri-user' host='host' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://host?user=uri-user},
+ q{user='uri-user' host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://host?},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgresql://[::1]:12345/db},
+ q{dbname='db' host='::1' port='12345' (inet)},
+ q{},
+ ],
+ [q{postgresql://[::1]/db},
+ q{dbname='db' host='::1' (inet)},
+ q{},
+ ],
+ [q{postgresql://[2001:db8::1234]/},
+ q{host='2001:db8::1234' (inet)},
+ q{},
+ ],
+ [q{postgresql://[200z:db8::1234]/},
+ q{host='200z:db8::1234' (inet)},
+ q{},
+ ],
+ [q{postgresql://[::1]},
+ q{host='::1' (inet)},
+ q{},
+ ],
+ [q{postgres://},
+ q{(local)},
+ q{},
+ ],
+ [q{postgres:///},
+ q{(local)},
+ q{},
+ ],
+ [q{postgres:///db},
+ q{dbname='db' (local)},
+ q{},
+ ],
+ [q{postgres://uri-user@/db},
+ q{user='uri-user' dbname='db' (local)},
+ q{},
+ ],
+ [q{postgres://?host=/path/to/socket/dir},
+ q{host='/path/to/socket/dir' (local)},
+ q{},
+ ],
+ [q{postgresql://host?uzer=},
+ q{},
+ q{uri-regress: invalid URI query parameter: "uzer"},
+ ],
+ [q{postgre://},
+ q{},
+ q{uri-regress: missing "=" after "postgre://" in connection info string},
+ ],
+ [q{postgres://[::1},
+ q{},
+ q{uri-regress: end of string reached when looking for matching "]" in IPv6 host address in URI: "postgres://[::1"},
+ ],
+ [q{postgres://[]},
+ q{},
+ q{uri-regress: IPv6 host address may not be empty in URI: "postgres://[]"},
+ ],
+ [q{postgres://[::1]z},
+ q{},
+ q{uri-regress: unexpected character "z" at position 17 in URI (expected ":" or "/"): "postgres://[::1]z"},
+ ],
+ [q{postgresql://host?zzz},
+ q{},
+ q{uri-regress: missing key/value separator "=" in URI query parameter: "zzz"},
+ ],
+ [q{postgresql://host?value1&value2},
+ q{},
+ q{uri-regress: missing key/value separator "=" in URI query parameter: "value1"},
+ ],
+ [q{postgresql://host?key=key=value},
+ q{},
+ q{uri-regress: extra key/value separator "=" in URI query parameter: "key"},
+ ],
+ [q{postgres://host?dbname=%XXfoo},
+ q{},
+ q{uri-regress: invalid percent-encoded token: "%XXfoo"},
+ ],
+ [q{postgresql://a%00b},
+ q{},
+ q{uri-regress: forbidden value %00 in percent-encoded value: "a%00b"},
+ ],
+ [q{postgresql://%zz},
+ q{},
+ q{uri-regress: invalid percent-encoded token: "%zz"},
+ ],
+ [q{postgresql://%1},
+ q{},
+ q{uri-regress: invalid percent-encoded token: "%1"},
+ ],
+ [q{postgresql://%},
+ q{},
+ q{uri-regress: invalid percent-encoded token: "%"},
+ ],
+ [q{postgres://@host},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgres://host:/},
+ q{host='host' (inet)},
+ q{},
+ ],
+ [q{postgres://:12345/},
+ q{port='12345' (local)},
+ q{},
+ ],
+ [q{postgres://otheruser@?host=/no/such/directory},
+ q{user='otheruser' host='/no/such/directory' (local)},
+ q{},
+ ],
+ [q{postgres://otheruser@/?host=/no/such/directory},
+ q{user='otheruser' host='/no/such/directory' (local)},
+ q{},
+ ],
+ [q{postgres://otheruser@:12345?host=/no/such/socket/path},
+ q{user='otheruser' host='/no/such/socket/path' port='12345' (local)},
+ q{},
+ ],
+ [q{postgres://otheruser@:12345/db?host=/path/to/socket},
+ q{user='otheruser' dbname='db' host='/path/to/socket' port='12345' (local)},
+ q{},
+ ],
+ [q{postgres://:12345/db?host=/path/to/socket},
+ q{dbname='db' host='/path/to/socket' port='12345' (local)},
+ q{},
+ ],
+ [q{postgres://:12345?host=/path/to/socket},
+ q{host='/path/to/socket' port='12345' (local)},
+ q{},
+ ],
+ [q{postgres://%2Fvar%2Flib%2Fpostgresql/dbname},
+ q{dbname='dbname' host='/var/lib/postgresql' (local)},
+ q{},
+ ]
+ );
+
+sub test_uri
+{
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
+ my ($uri, $expect_stdout, $expect_stderr) = @$_;
+
+ my $stdout;
+ my $stderr;
+ my $result;
+
+ $result = IPC::Run::run ['uri-regress', $uri], '>', \$stdout, '2>', \$stderr;
+ chomp($stdout);
+ chomp($stderr);
+ is_deeply([$result, $stdout, $stderr],
+ [$expect_stderr eq '', $expect_stdout, $expect_stderr],
+ $uri);
+}
+
+foreach (@tests)
+{
+ my $in =$_;
+ my $uri = @$in[0];
+
+ test_uri $in;
+}
+
+done_testing();
diff --git a/src/interfaces/libpq/test/.gitignore b/src/interfaces/libpq/test/.gitignore
index 5387b3b6d94..5e803d8816a 100644
--- a/src/interfaces/libpq/test/.gitignore
+++ b/src/interfaces/libpq/test/.gitignore
@@ -1,3 +1 @@
/uri-regress
-/regress.diff
-/regress.out
diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile
index 4832fab9d23..54212159065 100644
--- a/src/interfaces/libpq/test/Makefile
+++ b/src/interfaces/libpq/test/Makefile
@@ -1,3 +1,5 @@
+# src/interfaces/libpq/test/Makefile
+
subdir = src/interfaces/libpq/test
top_builddir = ../../../..
include $(top_builddir)/src/Makefile.global
@@ -13,10 +15,5 @@ PROGS = uri-regress
all: $(PROGS)
-installcheck: all
- SRCDIR='$(top_srcdir)' SUBDIR='$(subdir)' \
- $(PERL) $(top_srcdir)/$(subdir)/regress.pl
-
clean distclean maintainer-clean:
rm -f $(PROGS) *.o
- rm -f regress.out regress.diff
diff --git a/src/interfaces/libpq/test/README b/src/interfaces/libpq/test/README
deleted file mode 100644
index a05eb6bb3bc..00000000000
--- a/src/interfaces/libpq/test/README
+++ /dev/null
@@ -1,7 +0,0 @@
-This is a testsuite for testing libpq URI connection string syntax.
-
-To run the suite, use 'make installcheck' command. It works by
-running 'regress.pl' from this directory with appropriate environment
-set up, which in turn feeds up lines from 'regress.in' to
-'uri-regress' test program and compares the output against the correct
-one in 'expected.out' file.
diff --git a/src/interfaces/libpq/test/expected.out b/src/interfaces/libpq/test/expected.out
deleted file mode 100644
index d375e82b4ac..00000000000
--- a/src/interfaces/libpq/test/expected.out
+++ /dev/null
@@ -1,171 +0,0 @@
-trying postgresql://uri-user:secret@host:12345/db
-user='uri-user' password='secret' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://uri-user@host:12345/db
-user='uri-user' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://uri-user@host/db
-user='uri-user' dbname='db' host='host' (inet)
-
-trying postgresql://host:12345/db
-dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://host/db
-dbname='db' host='host' (inet)
-
-trying postgresql://uri-user@host:12345/
-user='uri-user' host='host' port='12345' (inet)
-
-trying postgresql://uri-user@host/
-user='uri-user' host='host' (inet)
-
-trying postgresql://uri-user@
-user='uri-user' (local)
-
-trying postgresql://host:12345/
-host='host' port='12345' (inet)
-
-trying postgresql://host:12345
-host='host' port='12345' (inet)
-
-trying postgresql://host/db
-dbname='db' host='host' (inet)
-
-trying postgresql://host/
-host='host' (inet)
-
-trying postgresql://host
-host='host' (inet)
-
-trying postgresql://
-(local)
-
-trying postgresql://?hostaddr=127.0.0.1
-hostaddr='127.0.0.1' (inet)
-
-trying postgresql://example.com?hostaddr=63.1.2.4
-host='example.com' hostaddr='63.1.2.4' (inet)
-
-trying postgresql://%68ost/
-host='host' (inet)
-
-trying postgresql://host/db?user=uri-user
-user='uri-user' dbname='db' host='host' (inet)
-
-trying postgresql://host/db?user=uri-user&port=12345
-user='uri-user' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://host/db?u%73er=someotheruser&port=12345
-user='someotheruser' dbname='db' host='host' port='12345' (inet)
-
-trying postgresql://host/db?u%7aer=someotheruser&port=12345
-uri-regress: invalid URI query parameter: "uzer"
-
-trying postgresql://host:12345?user=uri-user
-user='uri-user' host='host' port='12345' (inet)
-
-trying postgresql://host?user=uri-user
-user='uri-user' host='host' (inet)
-
-trying postgresql://host?
-host='host' (inet)
-
-trying postgresql://[::1]:12345/db
-dbname='db' host='::1' port='12345' (inet)
-
-trying postgresql://[::1]/db
-dbname='db' host='::1' (inet)
-
-trying postgresql://[2001:db8::1234]/
-host='2001:db8::1234' (inet)
-
-trying postgresql://[200z:db8::1234]/
-host='200z:db8::1234' (inet)
-
-trying postgresql://[::1]
-host='::1' (inet)
-
-trying postgres://
-(local)
-
-trying postgres:///
-(local)
-
-trying postgres:///db
-dbname='db' (local)
-
-trying postgres://uri-user@/db
-user='uri-user' dbname='db' (local)
-
-trying postgres://?host=/path/to/socket/dir
-host='/path/to/socket/dir' (local)
-
-trying postgresql://host?uzer=
-uri-regress: invalid URI query parameter: "uzer"
-
-trying postgre://
-uri-regress: missing "=" after "postgre://" in connection info string
-
-trying postgres://[::1
-uri-regress: end of string reached when looking for matching "]" in IPv6 host address in URI: "postgres://[::1"
-
-trying postgres://[]
-uri-regress: IPv6 host address may not be empty in URI: "postgres://[]"
-
-trying postgres://[::1]z
-uri-regress: unexpected character "z" at position 17 in URI (expected ":" or "/"): "postgres://[::1]z"
-
-trying postgresql://host?zzz
-uri-regress: missing key/value separator "=" in URI query parameter: "zzz"
-
-trying postgresql://host?value1&value2
-uri-regress: missing key/value separator "=" in URI query parameter: "value1"
-
-trying postgresql://host?key=key=value
-uri-regress: extra key/value separator "=" in URI query parameter: "key"
-
-trying postgres://host?dbname=%XXfoo
-uri-regress: invalid percent-encoded token: "%XXfoo"
-
-trying postgresql://a%00b
-uri-regress: forbidden value %00 in percent-encoded value: "a%00b"
-
-trying postgresql://%zz
-uri-regress: invalid percent-encoded token: "%zz"
-
-trying postgresql://%1
-uri-regress: invalid percent-encoded token: "%1"
-
-trying postgresql://%
-uri-regress: invalid percent-encoded token: "%"
-
-trying postgres://@host
-host='host' (inet)
-
-trying postgres://host:/
-host='host' (inet)
-
-trying postgres://:12345/
-port='12345' (local)
-
-trying postgres://otheruser@?host=/no/such/directory
-user='otheruser' host='/no/such/directory' (local)
-
-trying postgres://otheruser@/?host=/no/such/directory
-user='otheruser' host='/no/such/directory' (local)
-
-trying postgres://otheruser@:12345?host=/no/such/socket/path
-user='otheruser' host='/no/such/socket/path' port='12345' (local)
-
-trying postgres://otheruser@:12345/db?host=/path/to/socket
-user='otheruser' dbname='db' host='/path/to/socket' port='12345' (local)
-
-trying postgres://:12345/db?host=/path/to/socket
-dbname='db' host='/path/to/socket' port='12345' (local)
-
-trying postgres://:12345?host=/path/to/socket
-host='/path/to/socket' port='12345' (local)
-
-trying postgres://%2Fvar%2Flib%2Fpostgresql/dbname
-dbname='dbname' host='/var/lib/postgresql' (local)
-
diff --git a/src/interfaces/libpq/test/regress.in b/src/interfaces/libpq/test/regress.in
deleted file mode 100644
index de034f39141..00000000000
--- a/src/interfaces/libpq/test/regress.in
+++ /dev/null
@@ -1,57 +0,0 @@
-postgresql://uri-user:secret@host:12345/db
-postgresql://uri-user@host:12345/db
-postgresql://uri-user@host/db
-postgresql://host:12345/db
-postgresql://host/db
-postgresql://uri-user@host:12345/
-postgresql://uri-user@host/
-postgresql://uri-user@
-postgresql://host:12345/
-postgresql://host:12345
-postgresql://host/db
-postgresql://host/
-postgresql://host
-postgresql://
-postgresql://?hostaddr=127.0.0.1
-postgresql://example.com?hostaddr=63.1.2.4
-postgresql://%68ost/
-postgresql://host/db?user=uri-user
-postgresql://host/db?user=uri-user&port=12345
-postgresql://host/db?u%73er=someotheruser&port=12345
-postgresql://host/db?u%7aer=someotheruser&port=12345
-postgresql://host:12345?user=uri-user
-postgresql://host?user=uri-user
-postgresql://host?
-postgresql://[::1]:12345/db
-postgresql://[::1]/db
-postgresql://[2001:db8::1234]/
-postgresql://[200z:db8::1234]/
-postgresql://[::1]
-postgres://
-postgres:///
-postgres:///db
-postgres://uri-user@/db
-postgres://?host=/path/to/socket/dir
-postgresql://host?uzer=
-postgre://
-postgres://[::1
-postgres://[]
-postgres://[::1]z
-postgresql://host?zzz
-postgresql://host?value1&value2
-postgresql://host?key=key=value
-postgres://host?dbname=%XXfoo
-postgresql://a%00b
-postgresql://%zz
-postgresql://%1
-postgresql://%
-postgres://@host
-postgres://host:/
-postgres://:12345/
-postgres://otheruser@?host=/no/such/directory
-postgres://otheruser@/?host=/no/such/directory
-postgres://otheruser@:12345?host=/no/such/socket/path
-postgres://otheruser@:12345/db?host=/path/to/socket
-postgres://:12345/db?host=/path/to/socket
-postgres://:12345?host=/path/to/socket
-postgres://%2Fvar%2Flib%2Fpostgresql/dbname
diff --git a/src/interfaces/libpq/test/regress.pl b/src/interfaces/libpq/test/regress.pl
deleted file mode 100644
index 70691dabe68..00000000000
--- a/src/interfaces/libpq/test/regress.pl
+++ /dev/null
@@ -1,65 +0,0 @@
-#!/usr/bin/perl
-
-# Copyright (c) 2021-2022, PostgreSQL Global Development Group
-
-use strict;
-use warnings;
-
-# use of SRCDIR/SUBDIR is required for supporting VPath builds
-my $srcdir = $ENV{'SRCDIR'} or die 'SRCDIR environment variable is not set';
-my $subdir = $ENV{'SUBDIR'} or die 'SUBDIR environment variable is not set';
-
-my $regress_in = "$srcdir/$subdir/regress.in";
-my $expected_out = "$srcdir/$subdir/expected.out";
-
-# the output file should land in the build_dir of VPath, or just in
-# the current dir, if VPath isn't used
-my $regress_out = "regress.out";
-
-# open input file first, so possible error isn't sent to redirected STDERR
-open(my $regress_in_fh, "<", $regress_in)
- or die "can't open $regress_in for reading: $!";
-
-# save STDOUT/ERR and redirect both to regress.out
-open(my $oldout_fh, ">&", \*STDOUT) or die "can't dup STDOUT: $!";
-open(my $olderr_fh, ">&", \*STDERR) or die "can't dup STDERR: $!";
-
-open(STDOUT, ">", $regress_out)
- or die "can't open $regress_out for writing: $!";
-open(STDERR, ">&", \*STDOUT) or die "can't dup STDOUT: $!";
-
-# read lines from regress.in and run uri-regress on them
-while (<$regress_in_fh>)
-{
- chomp;
- print "trying $_\n";
- system("./uri-regress \"$_\"");
- print "\n";
-}
-
-# restore STDOUT/ERR so we can print the outcome to the user
-open(STDERR, ">&", $olderr_fh)
- or die; # can't complain as STDERR is still duped
-open(STDOUT, ">&", $oldout_fh) or die "can't restore STDOUT: $!";
-
-# just in case
-close $regress_in_fh;
-
-my $diff_status = system(
- "diff -c \"$srcdir/$subdir/expected.out\" regress.out >regress.diff");
-
-print "=" x 70, "\n";
-if ($diff_status == 0)
-{
- print "All tests passed\n";
- exit 0;
-}
-else
-{
- print <<EOF;
-FAILED: the test result differs from the expected output
-
-Review the difference in "$subdir/regress.diff"
-EOF
- exit 1;
-}
--
2.34.0
[text/x-diff] v2-0002-Add-tap-test-infrastructure-to-src-interfaces-lib.patch (3.1K, ../../[email protected]/3-v2-0002-Add-tap-test-infrastructure-to-src-interfaces-lib.patch)
download | inline diff:
From d66a339e051cee97847b6e5c1e1b5b9d846562c3 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Thu, 24 Feb 2022 08:20:06 -0800
Subject: [PATCH v2 2/3] Add tap test infrastructure to src/interfaces/libpq.
---
src/interfaces/libpq/.gitignore | 1 +
src/interfaces/libpq/Makefile | 13 ++++++++++---
src/Makefile.global.in | 12 ++++++------
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/src/interfaces/libpq/.gitignore b/src/interfaces/libpq/.gitignore
index 7478dc344ac..829d683ed27 100644
--- a/src/interfaces/libpq/.gitignore
+++ b/src/interfaces/libpq/.gitignore
@@ -1,2 +1,3 @@
/exports.list
/libpq-refs-stamp
+/tmp_check/
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 844c95d47de..1061547980f 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -137,8 +137,14 @@ install: all installdirs install-lib
$(INSTALL_DATA) $(srcdir)/pqexpbuffer.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pg_service.conf.sample '$(DESTDIR)$(datadir)/pg_service.conf.sample'
-installcheck:
- $(MAKE) -C test $@
+test-build:
+ $(MAKE) -C test all
+
+check: test-build all
+ PATH="$(CURDIR)/test:$$PATH" && $(prove_check)
+
+installcheck: test-build all
+ PATH="$(CURDIR)/test:$$PATH" && $(prove_installcheck)
installdirs: installdirs-lib
$(MKDIR_P) '$(DESTDIR)$(includedir)' '$(DESTDIR)$(includedir_internal)' '$(DESTDIR)$(datadir)'
@@ -151,8 +157,9 @@ uninstall: uninstall-lib
rm -f '$(DESTDIR)$(includedir_internal)/pqexpbuffer.h'
rm -f '$(DESTDIR)$(datadir)/pg_service.conf.sample'
-clean distclean: clean-lib
+clean distclean: clean-lib test-clean
$(MAKE) -C test $@
+ rm -rf tmp_check
rm -f $(OBJS) pthread.h libpq-refs-stamp
# Might be left over from a Win32 client-only build
rm -f pg_config_paths.h
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index c980444233f..bbdc1c4bda6 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -448,8 +448,8 @@ ifeq ($(enable_tap_tests),yes)
ifndef PGXS
define prove_installcheck
-rm -rf '$(CURDIR)'/tmp_check
-$(MKDIR_P) '$(CURDIR)'/tmp_check
+rm -rf '$(CURDIR)'/tmp_check && \
+$(MKDIR_P) '$(CURDIR)'/tmp_check && \
cd $(srcdir) && \
TESTDIR='$(CURDIR)' PATH="$(bindir):$(CURDIR):$$PATH" \
PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' \
@@ -458,8 +458,8 @@ cd $(srcdir) && \
endef
else # PGXS case
define prove_installcheck
-rm -rf '$(CURDIR)'/tmp_check
-$(MKDIR_P) '$(CURDIR)'/tmp_check
+rm -rf '$(CURDIR)'/tmp_check && \
+$(MKDIR_P) '$(CURDIR)'/tmp_check && \
cd $(srcdir) && \
TESTDIR='$(CURDIR)' PATH="$(bindir):$(CURDIR):$$PATH" \
PGPORT='6$(DEF_PGPORT)' top_builddir='$(top_builddir)' \
@@ -469,8 +469,8 @@ endef
endif # PGXS
define prove_check
-rm -rf '$(CURDIR)'/tmp_check
-$(MKDIR_P) '$(CURDIR)'/tmp_check
+rm -rf '$(CURDIR)'/tmp_check && \
+$(MKDIR_P) '$(CURDIR)'/tmp_check && \
cd $(srcdir) && \
TESTDIR='$(CURDIR)' $(with_temp_install) PGPORT='6$(DEF_PGPORT)' \
PG_REGRESS='$(CURDIR)/$(top_builddir)/src/test/regress/pg_regress' \
--
2.34.0
[text/x-diff] v2-0003-Move-libpq_pipeline-test-into-src-interfaces-libp.patch (7.3K, ../../[email protected]/4-v2-0003-Move-libpq_pipeline-test-into-src-interfaces-libp.patch)
download | inline diff:
From 6f06e37bdfd9947339d377b5184194ad7c78b59f Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Thu, 24 Feb 2022 08:27:41 -0800
Subject: [PATCH v2 3/3] Move libpq_pipeline test into src/interfaces/libpq.
---
.../libpq/t/002_pipeline.pl} | 2 +-
src/interfaces/libpq/test/.gitignore | 1 +
src/interfaces/libpq/test/Makefile | 2 +-
.../libpq/test}/libpq_pipeline.c | 0
.../test}/traces/disallowed_in_pipeline.trace | 0
.../libpq/test}/traces/multi_pipelines.trace | 0
.../libpq/test}/traces/nosync.trace | 0
.../libpq/test}/traces/pipeline_abort.trace | 0
.../libpq/test}/traces/prepared.trace | 0
.../libpq/test}/traces/simple_pipeline.trace | 0
.../libpq/test}/traces/singlerow.trace | 0
.../libpq/test}/traces/transaction.trace | 0
src/test/modules/libpq_pipeline/.gitignore | 5 ----
src/test/modules/libpq_pipeline/Makefile | 25 -------------------
src/test/modules/libpq_pipeline/README | 1 -
15 files changed, 3 insertions(+), 33 deletions(-)
rename src/{test/modules/libpq_pipeline/t/001_libpq_pipeline.pl => interfaces/libpq/t/002_pipeline.pl} (96%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/libpq_pipeline.c (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/disallowed_in_pipeline.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/multi_pipelines.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/nosync.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/pipeline_abort.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/prepared.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/simple_pipeline.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/singlerow.trace (100%)
rename src/{test/modules/libpq_pipeline => interfaces/libpq/test}/traces/transaction.trace (100%)
delete mode 100644 src/test/modules/libpq_pipeline/.gitignore
delete mode 100644 src/test/modules/libpq_pipeline/Makefile
delete mode 100644 src/test/modules/libpq_pipeline/README
diff --git a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl b/src/interfaces/libpq/t/002_pipeline.pl
similarity index 96%
rename from src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
rename to src/interfaces/libpq/t/002_pipeline.pl
index 0c164dcaba5..2e288d8ba7c 100644
--- a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
+++ b/src/interfaces/libpq/t/002_pipeline.pl
@@ -49,7 +49,7 @@ for my $testname (@tests)
my $expected;
my $result;
- $expected = slurp_file_eval("traces/$testname.trace");
+ $expected = slurp_file_eval("test/traces/$testname.trace");
next unless $expected ne "";
$result = slurp_file_eval($traceout);
next unless $result ne "";
diff --git a/src/interfaces/libpq/test/.gitignore b/src/interfaces/libpq/test/.gitignore
index 5e803d8816a..e24d7f64dc3 100644
--- a/src/interfaces/libpq/test/.gitignore
+++ b/src/interfaces/libpq/test/.gitignore
@@ -1 +1,2 @@
/uri-regress
+/libpq_pipeline
diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile
index 54212159065..9f99309653f 100644
--- a/src/interfaces/libpq/test/Makefile
+++ b/src/interfaces/libpq/test/Makefile
@@ -11,7 +11,7 @@ endif
override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
LDFLAGS_INTERNAL += $(libpq_pgport)
-PROGS = uri-regress
+PROGS = uri-regress libpq_pipeline
all: $(PROGS)
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/interfaces/libpq/test/libpq_pipeline.c
similarity index 100%
rename from src/test/modules/libpq_pipeline/libpq_pipeline.c
rename to src/interfaces/libpq/test/libpq_pipeline.c
diff --git a/src/test/modules/libpq_pipeline/traces/disallowed_in_pipeline.trace b/src/interfaces/libpq/test/traces/disallowed_in_pipeline.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/disallowed_in_pipeline.trace
rename to src/interfaces/libpq/test/traces/disallowed_in_pipeline.trace
diff --git a/src/test/modules/libpq_pipeline/traces/multi_pipelines.trace b/src/interfaces/libpq/test/traces/multi_pipelines.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/multi_pipelines.trace
rename to src/interfaces/libpq/test/traces/multi_pipelines.trace
diff --git a/src/test/modules/libpq_pipeline/traces/nosync.trace b/src/interfaces/libpq/test/traces/nosync.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/nosync.trace
rename to src/interfaces/libpq/test/traces/nosync.trace
diff --git a/src/test/modules/libpq_pipeline/traces/pipeline_abort.trace b/src/interfaces/libpq/test/traces/pipeline_abort.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/pipeline_abort.trace
rename to src/interfaces/libpq/test/traces/pipeline_abort.trace
diff --git a/src/test/modules/libpq_pipeline/traces/prepared.trace b/src/interfaces/libpq/test/traces/prepared.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/prepared.trace
rename to src/interfaces/libpq/test/traces/prepared.trace
diff --git a/src/test/modules/libpq_pipeline/traces/simple_pipeline.trace b/src/interfaces/libpq/test/traces/simple_pipeline.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/simple_pipeline.trace
rename to src/interfaces/libpq/test/traces/simple_pipeline.trace
diff --git a/src/test/modules/libpq_pipeline/traces/singlerow.trace b/src/interfaces/libpq/test/traces/singlerow.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/singlerow.trace
rename to src/interfaces/libpq/test/traces/singlerow.trace
diff --git a/src/test/modules/libpq_pipeline/traces/transaction.trace b/src/interfaces/libpq/test/traces/transaction.trace
similarity index 100%
rename from src/test/modules/libpq_pipeline/traces/transaction.trace
rename to src/interfaces/libpq/test/traces/transaction.trace
diff --git a/src/test/modules/libpq_pipeline/.gitignore b/src/test/modules/libpq_pipeline/.gitignore
deleted file mode 100644
index 3a11e786b83..00000000000
--- a/src/test/modules/libpq_pipeline/.gitignore
+++ /dev/null
@@ -1,5 +0,0 @@
-# Generated subdirectories
-/log/
-/results/
-/tmp_check/
-/libpq_pipeline
diff --git a/src/test/modules/libpq_pipeline/Makefile b/src/test/modules/libpq_pipeline/Makefile
deleted file mode 100644
index 65acc3e997e..00000000000
--- a/src/test/modules/libpq_pipeline/Makefile
+++ /dev/null
@@ -1,25 +0,0 @@
-# src/test/modules/libpq_pipeline/Makefile
-
-PGFILEDESC = "libpq_pipeline - test program for pipeline execution"
-PGAPPICON = win32
-
-PROGRAM = libpq_pipeline
-OBJS = $(WIN32RES) libpq_pipeline.o
-
-NO_INSTALL = 1
-
-PG_CPPFLAGS = -I$(libpq_srcdir)
-PG_LIBS_INTERNAL += $(libpq_pgport)
-
-TAP_TESTS = 1
-
-ifdef USE_PGXS
-PG_CONFIG = pg_config
-PGXS := $(shell $(PG_CONFIG) --pgxs)
-include $(PGXS)
-else
-subdir = src/test/modules/libpq_pipeline
-top_builddir = ../../../..
-include $(top_builddir)/src/Makefile.global
-include $(top_srcdir)/contrib/contrib-global.mk
-endif
diff --git a/src/test/modules/libpq_pipeline/README b/src/test/modules/libpq_pipeline/README
deleted file mode 100644
index d8174dd579a..00000000000
--- a/src/test/modules/libpq_pipeline/README
+++ /dev/null
@@ -1 +0,0 @@
-Test programs and libraries for libpq
--
2.34.0
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: convert libpq uri-regress tests to tap test
@ 2022-02-25 13:32 Peter Eisentraut <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 0 replies; 7+ messages in thread
From: Peter Eisentraut @ 2022-02-25 13:32 UTC (permalink / raw)
To: Tom Lane <[email protected]>; Andres Freund <[email protected]>; +Cc: pgsql-hackers
On 24.02.22 16:17, Tom Lane wrote:
> I think that having t/ directories contain only Perl test scripts
> is a good convention that we should stick to. Peter's proposal
> of a separate test/ subdirectory for C test scaffolding is
> probably fine.
I wonder if there are any conventions in the Perl community about where
to put test support files relative to the "t" directory.
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: convert libpq uri-regress tests to tap test
@ 2022-02-25 17:56 Andres Freund <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Andres Freund @ 2022-02-25 17:56 UTC (permalink / raw)
To: Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers
Hi,
On 2022-02-24 08:46:23 -0800, Andres Freund wrote:
> I'm mildly inclined to only do 0001 and 0002 for now. We'd not loose msvc
> coverage, because it already doesn't build the test. Once we've ironed that
> stuff out, we could do 0003?
From what I can see in the buildfarm client, we'd not loose (nor gain) any
buildfarm coverage either. It doesn't run the test today.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2022-02-25 17:56 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-02-24 17:33 [PATCH v7 2/2] Add a --outdated option to reindexdb Julien Rouhaud <[email protected]>
2022-02-24 12:31 Re: convert libpq uri-regress tests to tap test Peter Eisentraut <[email protected]>
2022-02-24 14:43 ` Re: convert libpq uri-regress tests to tap test Andres Freund <[email protected]>
2022-02-24 15:17 ` Re: convert libpq uri-regress tests to tap test Tom Lane <[email protected]>
2022-02-24 16:46 ` Re: convert libpq uri-regress tests to tap test Andres Freund <[email protected]>
2022-02-25 17:56 ` Re: convert libpq uri-regress tests to tap test Andres Freund <[email protected]>
2022-02-25 13:32 ` Re: convert libpq uri-regress tests to tap test Peter Eisentraut <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox