I wrote:
> "Bossart, Nathan" <bossartn@amazon.com> writes:
>> I run 'make check-world' a lot, and I typically use parallelism and
>> redirect stdout to /dev/null as suggested in the docs [0]. This seems
>> to eliminate all of the test chatter except for this one message:
>> NOTICE: database "regression" does not exist, skipping
> Yeah, that's bugged me too ever since we got to the point where that
> was the only output ...
Actually ... why shouldn't we suppress that by running the command
with client_min_messages = warning? This would have to be a change
to pg_regress, but I'm having a hard time thinking of cases where
quieting that message would be a problem.
I tried doing this as a one-liner change in pg_regress's
drop_database_if_exists(), but the idea fell over pretty
quickly, because what underlies that is a "psql -c" call:
$ psql -c 'set client_min_messages = warning; drop database if exists foo'
ERROR: DROP DATABASE cannot run inside a transaction block
We could dodge that, with modern versions of psql, by issuing
two -c switches. So after a bit of hacking I have the attached
POC patch. It's incomplete because now that we have this
infrastructure we should change other parts of pg_regress
to not launch psql N times where one would do. But it's enough
to get through check-world without any chatter.
Any objections to polishing this up and pushing it?
regards, tom lane
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 05296f7ee1..cfadc0cd70 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -122,7 +122,9 @@ static void make_directory(const char *dir);
static void header(const char *fmt,...) pg_attribute_printf(1, 2);
static void status(const char *fmt,...) pg_attribute_printf(1, 2);
-static void psql_command(const char *database, const char *query,...) pg_attribute_printf(2, 3);
+static StringInfo psql_start_command(void);
+static void psql_add_command(StringInfo buf, const char *query,...) pg_attribute_printf(2, 3);
+static void psql_end_command(StringInfo buf, const char *database);
/*
* allow core files if possible.
@@ -1136,51 +1138,94 @@ config_sspi_auth(const char *pgdata, const char *superuser_name)
#endif /* ENABLE_SSPI */
/*
- * Issue a command via psql, connecting to the specified database
+ * psql_start_command, psql_add_command, psql_end_command
+ *
+ * Issue one or more commands within one psql call.
+ * Set up with psql_start_command, then add commands one at a time
+ * with psql_add_command, and finally execute with psql_end_command.
*
* Since we use system(), this doesn't return until the operation finishes
*/
+static StringInfo
+psql_start_command(void)
+{
+ StringInfo buf = makeStringInfo();
+
+ appendStringInfo(buf,
+ "\"%s%spsql\" -X",
+ bindir ? bindir : "",
+ bindir ? "/" : "");
+ return buf;
+}
+
static void
-psql_command(const char *database, const char *query,...)
+psql_add_command(StringInfo buf, const char *query,...)
{
- char query_formatted[1024];
- char query_escaped[2048];
- char psql_cmd[MAXPGPATH + 2048];
- va_list args;
- char *s;
- char *d;
+ StringInfoData cmdbuf;
+ const char *cmdptr;
+
+ /* Add each command as a -c argument in the psql call */
+ appendStringInfoString(buf, " -c \"");
/* Generate the query with insertion of sprintf arguments */
- va_start(args, query);
- vsnprintf(query_formatted, sizeof(query_formatted), query, args);
- va_end(args);
+ initStringInfo(&cmdbuf);
+ for (;;)
+ {
+ va_list args;
+ int needed;
+
+ va_start(args, query);
+ needed = appendStringInfoVA(&cmdbuf, query, args);
+ va_end(args);
+ if (needed == 0)
+ break; /* success */
+ enlargeStringInfo(&cmdbuf, needed);
+ }
/* Now escape any shell double-quote metacharacters */
- d = query_escaped;
- for (s = query_formatted; *s; s++)
+ for (cmdptr = cmdbuf.data; *cmdptr; cmdptr++)
{
- if (strchr("\\\"$`", *s))
- *d++ = '\\';
- *d++ = *s;
+ if (strchr("\\\"$`", *cmdptr))
+ appendStringInfoChar(buf, '\\');
+ appendStringInfoChar(buf, *cmdptr);
}
- *d = '\0';
- /* And now we can build and execute the shell command */
- snprintf(psql_cmd, sizeof(psql_cmd),
- "\"%s%spsql\" -X -c \"%s\" \"%s\"",
- bindir ? bindir : "",
- bindir ? "/" : "",
- query_escaped,
- database);
+ appendStringInfoChar(buf, '"');
+
+ pfree(cmdbuf.data);
+}
- if (system(psql_cmd) != 0)
+static void
+psql_end_command(StringInfo buf, const char *database)
+{
+ /* Add the database name --- assume it needs no extra escaping */
+ appendStringInfo(buf,
+ " \"%s\"",
+ database);
+
+ /* And now we can execute the shell command */
+ if (system(buf->data) != 0)
{
/* psql probably already reported the error */
- fprintf(stderr, _("command failed: %s\n"), psql_cmd);
+ fprintf(stderr, _("command failed: %s\n"), buf->data);
exit(2);
}
+
+ /* Clean up */
+ pfree(buf->data);
+ pfree(buf);
}
+/*
+ * Shorthand macro for the common case of a single command
+ */
+#define psql_command(database, ...) \
+ do { \
+ StringInfo buf = psql_start_command(); \
+ psql_add_command(buf, __VA_ARGS__); \
+ psql_end_command(buf, database); \
+ } while (0)
+
/*
* Spawn a process to execute the given shell command; don't wait for it
*
@@ -2012,8 +2057,13 @@ open_result_files(void)
static void
drop_database_if_exists(const char *dbname)
{
+ StringInfo buf = psql_start_command();
+
header(_("dropping database \"%s\""), dbname);
- psql_command("postgres", "DROP DATABASE IF EXISTS \"%s\"", dbname);
+ /* Set warning level so we don't see chatter about nonexistent DB */
+ psql_add_command(buf, "SET client_min_messages = warning");
+ psql_add_command(buf, "DROP DATABASE IF EXISTS \"%s\"", dbname);
+ psql_end_command(buf, "postgres");
}
static void
@@ -2055,8 +2105,13 @@ create_database(const char *dbname)
static void
drop_role_if_exists(const char *rolename)
{
+ StringInfo buf = psql_start_command();
+
header(_("dropping role \"%s\""), rolename);
- psql_command("postgres", "DROP ROLE IF EXISTS \"%s\"", rolename);
+ /* Set warning level so we don't see chatter about nonexistent role */
+ psql_add_command(buf, "SET client_min_messages = warning");
+ psql_add_command(buf, "DROP ROLE IF EXISTS \"%s\"", rolename);
+ psql_end_command(buf, "postgres");
}
static void
