pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  

help / color / mirror / Atom feed

From: Tom Lane <tgl@sss.pgh.pa.us>
To: d.kovalenko@postgrespro.ru
Cc: Álvaro Herrera <alvherre@kurilemu.de>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18960: Mistake in test test_simple_pipeline (libpq_pipeline.c)
Date: Sat, 21 Jun 2025 16:01:03 -0400
Message-ID: <2790652.1750536063@sss.pgh.pa.us> (raw)
In-Reply-To: <1181606.1750280943@sss.pgh.pa.us>
References: <18960-09cd4a5100152e58@postgresql.org>
	<1181606.1750280943@sss.pgh.pa.us>

I wrote:
> I agree that's wrong ... but looking around, there's a huge amount
> of random inconsistency in this test script --- this same simple
> task of checking for an expected NULL result is coded several
> different ways with varying amounts of detail provided, and
> some other places share this same outright bug.

> I think it'd be better to make a helper function
> "CheckNoMoreResults(conn)", or something along that line,
> to shorten and standardize these places.

Here's a shot at improving matters.  I also made an effort
at cleaning up memory leaks in libpq_pipeline.c, although
that's surely neatnik-ism not anything meaningful.

			regards, tom lane

Attachments:

  [text/x-diff] v1-clean-up-libpq-pipeline-tests.patch (30.6K, ../2790652.1750536063@sss.pgh.pa.us/2-v1-clean-up-libpq-pipeline-tests.patch)
  download | inline diff:
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
index 9a3c0236325..ebfdb4a8da3 100644
--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c
+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c
@@ -87,6 +87,57 @@ pg_fatal_impl(int line, const char *fmt,...)
 	exit(1);
 }
 
+/*
+ * Check that libpq next returns a PGresult with the specified status,
+ * returning the PGresult so that caller can perform additional checks.
+ */
+#define confirm_result_status_keep(conn, status) confirm_result_status_keep_impl(__LINE__, conn, status)
+static PGresult *
+confirm_result_status_keep_impl(int line, PGconn *conn, ExecStatusType status)
+{
+	PGresult   *res;
+
+	res = PQgetResult(conn);
+	if (res == NULL)
+		pg_fatal_impl(line, "PQgetResult returned null unexpectedly: %s",
+					  PQerrorMessage(conn));
+	if (PQresultStatus(res) != status)
+		pg_fatal_impl(line, "PQgetResult returned status %s, expected %s: %s",
+					  PQresStatus(PQresultStatus(res)),
+					  PQresStatus(status),
+					  PQerrorMessage(conn));
+	return res;
+}
+
+/*
+ * Check that libpq next returns a PGresult with the specified status.
+ */
+#define confirm_result_status(conn, status) confirm_result_status_impl(__LINE__, conn, status)
+static void
+confirm_result_status_impl(int line, PGconn *conn, ExecStatusType status)
+{
+	PGresult   *res;
+
+	res = confirm_result_status_keep_impl(line, conn, status);
+	PQclear(res);
+}
+
+/*
+ * Check that libpq next returns a null PGresult.
+ */
+#define confirm_null_result(conn) confirm_null_result_impl(__LINE__, conn)
+static void
+confirm_null_result_impl(int line, PGconn *conn)
+{
+	PGresult   *res;
+
+	res = PQgetResult(conn);
+	if (res != NULL)
+		pg_fatal_impl(line, "expected NULL PGresult, got %s: %s",
+					  PQresStatus(PQresultStatus(res)),
+					  PQerrorMessage(conn));
+}
+
 /*
  * Check that the query on the given connection got canceled.
  */
@@ -94,14 +145,9 @@ pg_fatal_impl(int line, const char *fmt,...)
 static void
 confirm_query_canceled_impl(int line, PGconn *conn)
 {
-	PGresult   *res = NULL;
+	PGresult   *res;
 
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal_impl(line, "PQgetResult returned null: %s",
-					  PQerrorMessage(conn));
-	if (PQresultStatus(res) != PGRES_FATAL_ERROR)
-		pg_fatal_impl(line, "query did not fail when it was expected");
+	res = confirm_result_status_keep_impl(line, conn, PGRES_FATAL_ERROR);
 	if (strcmp(PQresultErrorField(res, PG_DIAG_SQLSTATE), "57014") != 0)
 		pg_fatal_impl(line, "query failed with a different error than cancellation: %s",
 					  PQerrorMessage(conn));
@@ -234,6 +280,10 @@ copy_connection(PGconn *conn)
 		pg_fatal("Connection to database failed: %s",
 				 PQerrorMessage(copyConn));
 
+	pfree(keywords);
+	pfree(vals);
+	PQconninfoFree(opts);
+
 	return copyConn;
 }
 
@@ -400,6 +450,7 @@ test_cancel(PGconn *conn)
 	confirm_query_canceled(conn);
 
 	PQcancelFinish(cancelConn);
+	PQfinish(monitorConn);
 
 	fprintf(stderr, "ok\n");
 }
@@ -428,6 +479,7 @@ test_disallowed_in_pipeline(PGconn *conn)
 			   "synchronous command execution functions are not allowed in pipeline mode\n") != 0)
 		pg_fatal("did not get expected error message; got: \"%s\"",
 				 PQerrorMessage(conn));
+	PQclear(res);
 
 	/* PQsendQuery should fail in pipeline mode */
 	if (PQsendQuery(conn, "SELECT 1") != 0)
@@ -460,6 +512,7 @@ test_disallowed_in_pipeline(PGconn *conn)
 	if (PQresultStatus(res) != PGRES_TUPLES_OK)
 		pg_fatal("PQexec should succeed after exiting pipeline mode but failed with: %s",
 				 PQerrorMessage(conn));
+	PQclear(res);
 
 	fprintf(stderr, "ok\n");
 }
@@ -467,7 +520,6 @@ test_disallowed_in_pipeline(PGconn *conn)
 static void
 test_multi_pipelines(PGconn *conn)
 {
-	PGresult   *res = NULL;
 	const char *dummy_params[1] = {"1"};
 	Oid			dummy_param_oids[1] = {INT4OID};
 
@@ -508,87 +560,31 @@ test_multi_pipelines(PGconn *conn)
 	/* OK, start processing the results */
 
 	/* first pipeline */
+	confirm_result_status(conn, PGRES_TUPLES_OK);
 
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("PQgetResult returned null when there's a pipeline item: %s",
-				 PQerrorMessage(conn));
-
-	if (PQresultStatus(res) != PGRES_TUPLES_OK)
-		pg_fatal("Unexpected result code %s from first pipeline item",
-				 PQresStatus(PQresultStatus(res)));
-	PQclear(res);
-	res = NULL;
-
-	if (PQgetResult(conn) != NULL)
-		pg_fatal("PQgetResult returned something extra after first result");
+	confirm_null_result(conn);
 
 	if (PQexitPipelineMode(conn) != 0)
 		pg_fatal("exiting pipeline mode after query but before sync succeeded incorrectly");
 
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("PQgetResult returned null when sync result expected: %s",
-				 PQerrorMessage(conn));
-
-	if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
-		pg_fatal("Unexpected result code %s instead of sync result, error: %s",
-				 PQresStatus(PQresultStatus(res)), PQerrorMessage(conn));
-	PQclear(res);
+	confirm_result_status(conn, PGRES_PIPELINE_SYNC);
 
 	/* second pipeline */
+	confirm_result_status(conn, PGRES_TUPLES_OK);
 
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("PQgetResult returned null when there's a pipeline item: %s",
-				 PQerrorMessage(conn));
-
-	if (PQresultStatus(res) != PGRES_TUPLES_OK)
-		pg_fatal("Unexpected result code %s from second pipeline item",
-				 PQresStatus(PQresultStatus(res)));
-	PQclear(res);
-	res = NULL;
-
-	if (PQgetResult(conn) != NULL)
-		pg_fatal("PQgetResult returned something extra after first result");
+	confirm_null_result(conn);
 
 	if (PQexitPipelineMode(conn) != 0)
 		pg_fatal("exiting pipeline mode after query but before sync succeeded incorrectly");
 
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("PQgetResult returned null when sync result expected: %s",
-				 PQerrorMessage(conn));
-
-	if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
-		pg_fatal("Unexpected result code %s instead of sync result, error: %s",
-				 PQresStatus(PQresultStatus(res)), PQerrorMessage(conn));
-	PQclear(res);
+	confirm_result_status(conn, PGRES_PIPELINE_SYNC);
 
 	/* third pipeline */
+	confirm_result_status(conn, PGRES_TUPLES_OK);
 
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("PQgetResult returned null when there's a pipeline item: %s",
-				 PQerrorMessage(conn));
+	confirm_null_result(conn);
 
-	if (PQresultStatus(res) != PGRES_TUPLES_OK)
-		pg_fatal("Unexpected result code %s from third pipeline item",
-				 PQresStatus(PQresultStatus(res)));
-
-	res = PQgetResult(conn);
-	if (res != NULL)
-		pg_fatal("Expected null result, got %s",
-				 PQresStatus(PQresultStatus(res)));
-
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("PQgetResult returned null when there's a pipeline item: %s",
-				 PQerrorMessage(conn));
-
-	if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
-		pg_fatal("Unexpected result code %s from second pipeline sync",
-				 PQresStatus(PQresultStatus(res)));
+	confirm_result_status(conn, PGRES_PIPELINE_SYNC);
 
 	/* We're still in pipeline mode ... */
 	if (PQpipelineStatus(conn) == PQ_PIPELINE_OFF)
@@ -657,36 +653,17 @@ test_nosync(PGconn *conn)
 	/* Now read all results */
 	for (;;)
 	{
-		PGresult   *res;
-
-		res = PQgetResult(conn);
-
-		/* NULL results are only expected after TUPLES_OK */
-		if (res == NULL)
-			pg_fatal("got unexpected NULL result after %d results", results);
-
 		/* We expect exactly one TUPLES_OK result for each query we sent */
-		if (PQresultStatus(res) == PGRES_TUPLES_OK)
-		{
-			PGresult   *res2;
+		confirm_result_status(conn, PGRES_TUPLES_OK);
 
-			/* and one NULL result should follow each */
-			res2 = PQgetResult(conn);
-			if (res2 != NULL)
-				pg_fatal("expected NULL, got %s",
-						 PQresStatus(PQresultStatus(res2)));
-			PQclear(res);
-			results++;
-
-			/* if we're done, we're done */
-			if (results == numqueries)
-				break;
+		/* and one NULL result should follow each */
+		confirm_null_result(conn);
 
-			continue;
-		}
+		results++;
 
-		/* anything else is unexpected */
-		pg_fatal("got unexpected %s\n", PQresStatus(PQresultStatus(res)));
+		/* if we're done, we're done */
+		if (results == numqueries)
+			break;
 	}
 
 	fprintf(stderr, "ok\n");
@@ -716,10 +693,12 @@ test_pipeline_abort(PGconn *conn)
 	res = PQexec(conn, drop_table_sql);
 	if (PQresultStatus(res) != PGRES_COMMAND_OK)
 		pg_fatal("dispatching DROP TABLE failed: %s", PQerrorMessage(conn));
+	PQclear(res);
 
 	res = PQexec(conn, create_table_sql);
 	if (PQresultStatus(res) != PGRES_COMMAND_OK)
 		pg_fatal("dispatching CREATE TABLE failed: %s", PQerrorMessage(conn));
+	PQclear(res);
 
 	/*
 	 * Queue up a couple of small pipelines and process each without returning
@@ -763,33 +742,16 @@ test_pipeline_abort(PGconn *conn)
 	 * a pipeline aborted message for the second insert, a pipeline-end, then
 	 * a command-ok and a pipeline-ok for the second pipeline operation.
 	 */
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("Unexpected NULL result: %s", PQerrorMessage(conn));
-	if (PQresultStatus(res) != PGRES_COMMAND_OK)
-		pg_fatal("Unexpected result status %s: %s",
-				 PQresStatus(PQresultStatus(res)),
-				 PQresultErrorMessage(res));
-	PQclear(res);
+	confirm_result_status(conn, PGRES_COMMAND_OK);
 
 	/* NULL result to signal end-of-results for this command */
-	if ((res = PQgetResult(conn)) != NULL)
-		pg_fatal("Expected null result, got %s",
-				 PQresStatus(PQresultStatus(res)));
+	confirm_null_result(conn);
 
 	/* Second query caused error, so we expect an error next */
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("Unexpected NULL result: %s", PQerrorMessage(conn));
-	if (PQresultStatus(res) != PGRES_FATAL_ERROR)
-		pg_fatal("Unexpected result code -- expected PGRES_FATAL_ERROR, got %s",
-				 PQresStatus(PQresultStatus(res)));
-	PQclear(res);
+	confirm_result_status(conn, PGRES_FATAL_ERROR);
 
 	/* NULL result to signal end-of-results for this command */
-	if ((res = PQgetResult(conn)) != NULL)
-		pg_fatal("Expected null result, got %s",
-				 PQresStatus(PQresultStatus(res)));
+	confirm_null_result(conn);
 
 	/*
 	 * pipeline should now be aborted.
@@ -802,17 +764,10 @@ test_pipeline_abort(PGconn *conn)
 		pg_fatal("pipeline should be flagged as aborted but isn't");
 
 	/* third query in pipeline, the second insert */
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("Unexpected NULL result: %s", PQerrorMessage(conn));
-	if (PQresultStatus(res) != PGRES_PIPELINE_ABORTED)
-		pg_fatal("Unexpected result code -- expected PGRES_PIPELINE_ABORTED, got %s",
-				 PQresStatus(PQresultStatus(res)));
-	PQclear(res);
+	confirm_result_status(conn, PGRES_PIPELINE_ABORTED);
 
 	/* NULL result to signal end-of-results for this command */
-	if ((res = PQgetResult(conn)) != NULL)
-		pg_fatal("Expected null result, got %s", PQresStatus(PQresultStatus(res)));
+	confirm_null_result(conn);
 
 	if (PQpipelineStatus(conn) != PQ_PIPELINE_ABORTED)
 		pg_fatal("pipeline should be flagged as aborted but isn't");
@@ -827,14 +782,7 @@ test_pipeline_abort(PGconn *conn)
 	 * (This is so clients know to start processing results normally again and
 	 * can tell the difference between skipped commands and the sync.)
 	 */
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("Unexpected NULL result: %s", PQerrorMessage(conn));
-	if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
-		pg_fatal("Unexpected result code from first pipeline sync\n"
-				 "Expected PGRES_PIPELINE_SYNC, got %s",
-				 PQresStatus(PQresultStatus(res)));
-	PQclear(res);
+	confirm_result_status(conn, PGRES_PIPELINE_SYNC);
 
 	if (PQpipelineStatus(conn) == PQ_PIPELINE_ABORTED)
 		pg_fatal("sync should've cleared the aborted flag but didn't");
@@ -844,30 +792,16 @@ test_pipeline_abort(PGconn *conn)
 		pg_fatal("Fell out of pipeline mode somehow");
 
 	/* the insert from the second pipeline */
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("Unexpected NULL result: %s", PQerrorMessage(conn));
-	if (PQresultStatus(res) != PGRES_COMMAND_OK)
-		pg_fatal("Unexpected result code %s from first item in second pipeline",
-				 PQresStatus(PQresultStatus(res)));
-	PQclear(res);
+	confirm_result_status(conn, PGRES_COMMAND_OK);
 
 	/* Read the NULL result at the end of the command */
-	if ((res = PQgetResult(conn)) != NULL)
-		pg_fatal("Expected null result, got %s", PQresStatus(PQresultStatus(res)));
+	confirm_null_result(conn);
 
 	/* the second pipeline sync */
-	if ((res = PQgetResult(conn)) == NULL)
-		pg_fatal("Unexpected NULL result: %s", PQerrorMessage(conn));
-	if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
-		pg_fatal("Unexpected result code %s from second pipeline sync",
-				 PQresStatus(PQresultStatus(res)));
-	PQclear(res);
+	confirm_result_status(conn, PGRES_PIPELINE_SYNC);
 
-	if ((res = PQgetResult(conn)) != NULL)
-		pg_fatal("Expected null result, got %s: %s",
-				 PQresStatus(PQresultStatus(res)),
-				 PQerrorMessage(conn));
+	/* Read the NULL result at the end of the command */
+	confirm_null_result(conn);
 
 	/* Try to send two queries in one command */
 	if (PQsendQueryParams(conn, "SELECT 1; SELECT 2", 0, NULL, NULL, NULL, NULL, 0) != 1)
@@ -890,15 +824,14 @@ test_pipeline_abort(PGconn *conn)
 				pg_fatal("got unexpected status %s", PQresStatus(PQresultStatus(res)));
 				break;
 		}
+		PQclear(res);
 	}
 	if (!goterror)
 		pg_fatal("did not get cannot-insert-multiple-commands error");
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("got NULL result");
-	if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
-		pg_fatal("Unexpected result code %s from pipeline sync",
-				 PQresStatus(PQresultStatus(res)));
+
+	/* the second pipeline sync */
+	confirm_result_status(conn, PGRES_PIPELINE_SYNC);
+
 	fprintf(stderr, "ok\n");
 
 	/* Test single-row mode with an error partways */
@@ -935,13 +868,9 @@ test_pipeline_abort(PGconn *conn)
 		pg_fatal("did not get division-by-zero error");
 	if (gotrows != 3)
 		pg_fatal("did not get three rows");
+
 	/* the third pipeline sync */
-	if ((res = PQgetResult(conn)) == NULL)
-		pg_fatal("Unexpected NULL result: %s", PQerrorMessage(conn));
-	if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
-		pg_fatal("Unexpected result code %s from third pipeline sync",
-				 PQresStatus(PQresultStatus(res)));
-	PQclear(res);
+	confirm_result_status(conn, PGRES_PIPELINE_SYNC);
 
 	/* We're still in pipeline mode... */
 	if (PQpipelineStatus(conn) == PQ_PIPELINE_OFF)
@@ -1274,21 +1203,11 @@ test_prepared(PGconn *conn)
 	if (PQpipelineSync(conn) != 1)
 		pg_fatal("pipeline sync failed: %s", PQerrorMessage(conn));
 
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("PQgetResult returned null");
-	if (PQresultStatus(res) != PGRES_COMMAND_OK)
-		pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(res)));
-	PQclear(res);
-	res = PQgetResult(conn);
-	if (res != NULL)
-		pg_fatal("expected NULL result");
+	confirm_result_status(conn, PGRES_COMMAND_OK);
 
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("PQgetResult returned NULL");
-	if (PQresultStatus(res) != PGRES_COMMAND_OK)
-		pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(res)));
+	confirm_null_result(conn);
+
+	res = confirm_result_status_keep(conn, PGRES_COMMAND_OK);
 	if (PQnfields(res) != lengthof(expected_oids))
 		pg_fatal("expected %zu columns, got %d",
 				 lengthof(expected_oids), PQnfields(res));
@@ -1300,13 +1219,10 @@ test_prepared(PGconn *conn)
 					 i, expected_oids[i], typ);
 	}
 	PQclear(res);
-	res = PQgetResult(conn);
-	if (res != NULL)
-		pg_fatal("expected NULL result");
 
-	res = PQgetResult(conn);
-	if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
-		pg_fatal("expected PGRES_PIPELINE_SYNC, got %s", PQresStatus(PQresultStatus(res)));
+	confirm_null_result(conn);
+
+	confirm_result_status(conn, PGRES_PIPELINE_SYNC);
 
 	fprintf(stderr, "closing statement..");
 	if (PQsendClosePrepared(conn, "select_one") != 1)
@@ -1314,18 +1230,11 @@ test_prepared(PGconn *conn)
 	if (PQpipelineSync(conn) != 1)
 		pg_fatal("pipeline sync failed: %s", PQerrorMessage(conn));
 
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("expected non-NULL result");
-	if (PQresultStatus(res) != PGRES_COMMAND_OK)
-		pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(res)));
-	PQclear(res);
-	res = PQgetResult(conn);
-	if (res != NULL)
-		pg_fatal("expected NULL result");
-	res = PQgetResult(conn);
-	if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
-		pg_fatal("expected PGRES_PIPELINE_SYNC, got %s", PQresStatus(PQresultStatus(res)));
+	confirm_result_status(conn, PGRES_COMMAND_OK);
+
+	confirm_null_result(conn);
+
+	confirm_result_status(conn, PGRES_PIPELINE_SYNC);
 
 	if (PQexitPipelineMode(conn) != 1)
 		pg_fatal("could not exit pipeline mode: %s", PQerrorMessage(conn));
@@ -1334,6 +1243,7 @@ test_prepared(PGconn *conn)
 	res = PQdescribePrepared(conn, "select_one");
 	if (PQresultStatus(res) != PGRES_FATAL_ERROR)
 		pg_fatal("expected FATAL_ERROR, got %s", PQresStatus(PQresultStatus(res)));
+	PQclear(res);
 
 	/*
 	 * Also test the blocking close, this should not fail since closing a
@@ -1342,32 +1252,36 @@ test_prepared(PGconn *conn)
 	res = PQclosePrepared(conn, "select_one");
 	if (PQresultStatus(res) != PGRES_COMMAND_OK)
 		pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(res)));
+	PQclear(res);
 
 	fprintf(stderr, "creating portal... ");
-	PQexec(conn, "BEGIN");
-	PQexec(conn, "DECLARE cursor_one CURSOR FOR SELECT 1");
+
+	res = PQexec(conn, "BEGIN");
+	if (PQresultStatus(res) != PGRES_COMMAND_OK)
+		pg_fatal("BEGIN failed: %s", PQerrorMessage(conn));
+	PQclear(res);
+
+	res = PQexec(conn, "DECLARE cursor_one CURSOR FOR SELECT 1");
+	if (PQresultStatus(res) != PGRES_COMMAND_OK)
+		pg_fatal("DECLARE CURSOR failed: %s", PQerrorMessage(conn));
+	PQclear(res);
+
 	PQenterPipelineMode(conn);
 	if (PQsendDescribePortal(conn, "cursor_one") != 1)
 		pg_fatal("PQsendDescribePortal failed: %s", PQerrorMessage(conn));
 	if (PQpipelineSync(conn) != 1)
 		pg_fatal("pipeline sync failed: %s", PQerrorMessage(conn));
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("PQgetResult returned null");
-	if (PQresultStatus(res) != PGRES_COMMAND_OK)
-		pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(res)));
 
+	res = confirm_result_status_keep(conn, PGRES_COMMAND_OK);
 	typ = PQftype(res, 0);
 	if (typ != INT4OID)
 		pg_fatal("portal: expected type %u, got %u",
 				 INT4OID, typ);
 	PQclear(res);
-	res = PQgetResult(conn);
-	if (res != NULL)
-		pg_fatal("expected NULL result");
-	res = PQgetResult(conn);
-	if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
-		pg_fatal("expected PGRES_PIPELINE_SYNC, got %s", PQresStatus(PQresultStatus(res)));
+
+	confirm_null_result(conn);
+
+	confirm_result_status(conn, PGRES_PIPELINE_SYNC);
 
 	fprintf(stderr, "closing portal... ");
 	if (PQsendClosePortal(conn, "cursor_one") != 1)
@@ -1375,18 +1289,11 @@ test_prepared(PGconn *conn)
 	if (PQpipelineSync(conn) != 1)
 		pg_fatal("pipeline sync failed: %s", PQerrorMessage(conn));
 
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("expected non-NULL result");
-	if (PQresultStatus(res) != PGRES_COMMAND_OK)
-		pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(res)));
-	PQclear(res);
-	res = PQgetResult(conn);
-	if (res != NULL)
-		pg_fatal("expected NULL result");
-	res = PQgetResult(conn);
-	if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
-		pg_fatal("expected PGRES_PIPELINE_SYNC, got %s", PQresStatus(PQresultStatus(res)));
+	confirm_result_status(conn, PGRES_COMMAND_OK);
+
+	confirm_null_result(conn);
+
+	confirm_result_status(conn, PGRES_PIPELINE_SYNC);
 
 	if (PQexitPipelineMode(conn) != 1)
 		pg_fatal("could not exit pipeline mode: %s", PQerrorMessage(conn));
@@ -1395,6 +1302,7 @@ test_prepared(PGconn *conn)
 	res = PQdescribePortal(conn, "cursor_one");
 	if (PQresultStatus(res) != PGRES_FATAL_ERROR)
 		pg_fatal("expected FATAL_ERROR, got %s", PQresStatus(PQresultStatus(res)));
+	PQclear(res);
 
 	/*
 	 * Also test the blocking close, this should not fail since closing a
@@ -1403,6 +1311,7 @@ test_prepared(PGconn *conn)
 	res = PQclosePortal(conn, "cursor_one");
 	if (PQresultStatus(res) != PGRES_COMMAND_OK)
 		pg_fatal("expected COMMAND_OK, got %s", PQresStatus(PQresultStatus(res)));
+	PQclear(res);
 
 	fprintf(stderr, "ok\n");
 }
@@ -1509,6 +1418,10 @@ test_protocol_version(PGconn *conn)
 		pg_fatal("expected 30002, got %d", protocol_version);
 
 	PQfinish(conn);
+
+	pfree(keywords);
+	pfree(vals);
+	PQconninfoFree(opts);
 }
 
 /* Notice processor: print notices, and count how many we got */
@@ -1525,7 +1438,6 @@ notice_processor(void *arg, const char *message)
 static void
 test_pipeline_idle(PGconn *conn)
 {
-	PGresult   *res;
 	int			n_notices = 0;
 
 	fprintf(stderr, "\npipeline idle...\n");
@@ -1538,17 +1450,11 @@ test_pipeline_idle(PGconn *conn)
 	if (PQsendQueryParams(conn, "SELECT 1", 0, NULL, NULL, NULL, NULL, 0) != 1)
 		pg_fatal("failed to send query: %s", PQerrorMessage(conn));
 	PQsendFlushRequest(conn);
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("PQgetResult returned null when there's a pipeline item: %s",
-				 PQerrorMessage(conn));
-	if (PQresultStatus(res) != PGRES_TUPLES_OK)
-		pg_fatal("unexpected result code %s from first pipeline item",
-				 PQresStatus(PQresultStatus(res)));
-	PQclear(res);
-	res = PQgetResult(conn);
-	if (res != NULL)
-		pg_fatal("did not receive terminating NULL");
+
+	confirm_result_status(conn, PGRES_TUPLES_OK);
+
+	confirm_null_result(conn);
+
 	if (PQsendQueryParams(conn, "SELECT 2", 0, NULL, NULL, NULL, NULL, 0) != 1)
 		pg_fatal("failed to send query: %s", PQerrorMessage(conn));
 	if (PQexitPipelineMode(conn) == 1)
@@ -1558,14 +1464,11 @@ test_pipeline_idle(PGconn *conn)
 		pg_fatal("did not get expected error; got: %s",
 				 PQerrorMessage(conn));
 	PQsendFlushRequest(conn);
-	res = PQgetResult(conn);
-	if (PQresultStatus(res) != PGRES_TUPLES_OK)
-		pg_fatal("unexpected result code %s from second pipeline item",
-				 PQresStatus(PQresultStatus(res)));
-	PQclear(res);
-	res = PQgetResult(conn);
-	if (res != NULL)
-		pg_fatal("did not receive terminating NULL");
+
+	confirm_result_status(conn, PGRES_TUPLES_OK);
+
+	confirm_null_result(conn);
+
 	if (PQexitPipelineMode(conn) != 1)
 		pg_fatal("exiting pipeline failed: %s", PQerrorMessage(conn));
 
@@ -1579,11 +1482,9 @@ test_pipeline_idle(PGconn *conn)
 	if (PQsendQueryParams(conn, "SELECT pg_catalog.pg_advisory_unlock(1,1)", 0, NULL, NULL, NULL, NULL, 0) != 1)
 		pg_fatal("failed to send query: %s", PQerrorMessage(conn));
 	PQsendFlushRequest(conn);
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("unexpected NULL result received");
-	if (PQresultStatus(res) != PGRES_TUPLES_OK)
-		pg_fatal("unexpected result code %s", PQresStatus(PQresultStatus(res)));
+
+	confirm_result_status(conn, PGRES_TUPLES_OK);
+
 	if (PQexitPipelineMode(conn) != 1)
 		pg_fatal("failed to exit pipeline mode: %s", PQerrorMessage(conn));
 	fprintf(stderr, "ok - 2\n");
@@ -1592,7 +1493,6 @@ test_pipeline_idle(PGconn *conn)
 static void
 test_simple_pipeline(PGconn *conn)
 {
-	PGresult   *res = NULL;
 	const char *dummy_params[1] = {"1"};
 	Oid			dummy_param_oids[1] = {INT4OID};
 
@@ -1623,20 +1523,9 @@ test_simple_pipeline(PGconn *conn)
 	if (PQpipelineSync(conn) != 1)
 		pg_fatal("pipeline sync failed: %s", PQerrorMessage(conn));
 
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("PQgetResult returned null when there's a pipeline item: %s",
-				 PQerrorMessage(conn));
-
-	if (PQresultStatus(res) != PGRES_TUPLES_OK)
-		pg_fatal("Unexpected result code %s from first pipeline item",
-				 PQresStatus(PQresultStatus(res)));
-
-	PQclear(res);
-	res = NULL;
+	confirm_result_status(conn, PGRES_TUPLES_OK);
 
-	if (PQgetResult(conn) != NULL)
-		pg_fatal("PQgetResult returned something extra after first query result.");
+	confirm_null_result(conn);
 
 	/*
 	 * Even though we've processed the result there's still a sync to come and
@@ -1645,21 +1534,9 @@ test_simple_pipeline(PGconn *conn)
 	if (PQexitPipelineMode(conn) != 0)
 		pg_fatal("exiting pipeline mode after query but before sync succeeded incorrectly");
 
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("PQgetResult returned null when sync result PGRES_PIPELINE_SYNC expected: %s",
-				 PQerrorMessage(conn));
-
-	if (PQresultStatus(res) != PGRES_PIPELINE_SYNC)
-		pg_fatal("Unexpected result code %s instead of PGRES_PIPELINE_SYNC, error: %s",
-				 PQresStatus(PQresultStatus(res)), PQerrorMessage(conn));
-
-	PQclear(res);
-	res = NULL;
+	confirm_result_status(conn, PGRES_PIPELINE_SYNC);
 
-	if (PQgetResult(conn) != NULL)
-		pg_fatal("PQgetResult returned something extra after pipeline end: %s",
-				 PQresStatus(PQresultStatus(res)));
+	confirm_null_result(conn);
 
 	/* We're still in pipeline mode... */
 	if (PQpipelineStatus(conn) == PQ_PIPELINE_OFF)
@@ -1792,20 +1669,12 @@ test_singlerowmode(PGconn *conn)
 		pg_fatal("failed to send flush request");
 	if (PQsetSingleRowMode(conn) != 1)
 		pg_fatal("PQsetSingleRowMode() failed");
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("unexpected NULL");
-	if (PQresultStatus(res) != PGRES_SINGLE_TUPLE)
-		pg_fatal("Expected PGRES_SINGLE_TUPLE, got %s",
-				 PQresStatus(PQresultStatus(res)));
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("unexpected NULL");
-	if (PQresultStatus(res) != PGRES_TUPLES_OK)
-		pg_fatal("Expected PGRES_TUPLES_OK, got %s",
-				 PQresStatus(PQresultStatus(res)));
-	if (PQgetResult(conn) != NULL)
-		pg_fatal("expected NULL result");
+
+	confirm_result_status(conn, PGRES_SINGLE_TUPLE);
+
+	confirm_result_status(conn, PGRES_TUPLES_OK);
+
+	confirm_null_result(conn);
 
 	if (PQsendQueryParams(conn, "SELECT 1",
 						  0, NULL, NULL, NULL, NULL, 0) != 1)
@@ -1813,14 +1682,10 @@ test_singlerowmode(PGconn *conn)
 				 PQerrorMessage(conn));
 	if (PQsendFlushRequest(conn) != 1)
 		pg_fatal("failed to send flush request");
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("unexpected NULL");
-	if (PQresultStatus(res) != PGRES_TUPLES_OK)
-		pg_fatal("Expected PGRES_TUPLES_OK, got %s",
-				 PQresStatus(PQresultStatus(res)));
-	if (PQgetResult(conn) != NULL)
-		pg_fatal("expected NULL result");
+
+	confirm_result_status(conn, PGRES_TUPLES_OK);
+
+	confirm_null_result(conn);
 
 	/*
 	 * Try chunked mode as well; make sure that it correctly delivers a
@@ -1834,33 +1699,23 @@ test_singlerowmode(PGconn *conn)
 		pg_fatal("failed to send flush request");
 	if (PQsetChunkedRowsMode(conn, 3) != 1)
 		pg_fatal("PQsetChunkedRowsMode() failed");
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("unexpected NULL");
-	if (PQresultStatus(res) != PGRES_TUPLES_CHUNK)
-		pg_fatal("Expected PGRES_TUPLES_CHUNK, got %s: %s",
-				 PQresStatus(PQresultStatus(res)),
-				 PQerrorMessage(conn));
+
+	res = confirm_result_status_keep(conn, PGRES_TUPLES_CHUNK);
 	if (PQntuples(res) != 3)
 		pg_fatal("Expected 3 rows, got %d", PQntuples(res));
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("unexpected NULL");
-	if (PQresultStatus(res) != PGRES_TUPLES_CHUNK)
-		pg_fatal("Expected PGRES_TUPLES_CHUNK, got %s",
-				 PQresStatus(PQresultStatus(res)));
+	PQclear(res);
+
+	res = confirm_result_status_keep(conn, PGRES_TUPLES_CHUNK);
 	if (PQntuples(res) != 2)
 		pg_fatal("Expected 2 rows, got %d", PQntuples(res));
-	res = PQgetResult(conn);
-	if (res == NULL)
-		pg_fatal("unexpected NULL");
-	if (PQresultStatus(res) != PGRES_TUPLES_OK)
-		pg_fatal("Expected PGRES_TUPLES_OK, got %s",
-				 PQresStatus(PQresultStatus(res)));
+	PQclear(res);
+
+	res = confirm_result_status_keep(conn, PGRES_TUPLES_OK);
 	if (PQntuples(res) != 0)
 		pg_fatal("Expected 0 rows, got %d", PQntuples(res));
-	if (PQgetResult(conn) != NULL)
-		pg_fatal("expected NULL result");
+	PQclear(res);
+
+	confirm_null_result(conn);
 
 	if (PQexitPipelineMode(conn) != 1)
 		pg_fatal("failed to end pipeline mode: %s", PQerrorMessage(conn));
@@ -1995,9 +1850,8 @@ test_transaction(PGconn *conn)
 		if (num_syncs <= 0)
 			break;
 	}
-	if (PQgetResult(conn) != NULL)
-		pg_fatal("returned something extra after all the syncs: %s",
-				 PQresStatus(PQresultStatus(res)));
+
+	confirm_null_result(conn);
 
 	if (PQexitPipelineMode(conn) != 1)
 		pg_fatal("failed to end pipeline mode: %s", PQerrorMessage(conn));
@@ -2053,16 +1907,19 @@ test_uniqviol(PGconn *conn)
 				 "create table ppln_uniqviol(id bigint primary key, idata bigint)");
 	if (PQresultStatus(res) != PGRES_COMMAND_OK)
 		pg_fatal("failed to create table: %s", PQerrorMessage(conn));
+	PQclear(res);
 
 	res = PQexec(conn, "begin");
 	if (PQresultStatus(res) != PGRES_COMMAND_OK)
 		pg_fatal("failed to begin transaction: %s", PQerrorMessage(conn));
+	PQclear(res);
 
 	res = PQprepare(conn, "insertion",
 					"insert into ppln_uniqviol values ($1, $2) returning id",
 					2, paramTypes);
-	if (res == NULL || PQresultStatus(res) != PGRES_COMMAND_OK)
+	if (PQresultStatus(res) != PGRES_COMMAND_OK)
 		pg_fatal("failed to prepare query: %s", PQerrorMessage(conn));
+	PQclear(res);
 
 	if (PQenterPipelineMode(conn) != 1)
 		pg_fatal("failed to enter pipeline mode");
@@ -2191,7 +2048,6 @@ test_uniqviol(PGconn *conn)
 static bool
 process_result(PGconn *conn, PGresult *res, int results, int numsent)
 {
-	PGresult   *res2;
 	bool		got_error = false;
 
 	if (res == NULL)
@@ -2203,29 +2059,19 @@ process_result(PGconn *conn, PGresult *res, int results, int numsent)
 			got_error = true;
 			fprintf(stderr, "result %d/%d (error): %s\n", results, numsent, PQerrorMessage(conn));
 			PQclear(res);
-
-			res2 = PQgetResult(conn);
-			if (res2 != NULL)
-				pg_fatal("expected NULL, got %s",
-						 PQresStatus(PQresultStatus(res2)));
+			confirm_null_result(conn);
 			break;
 
 		case PGRES_TUPLES_OK:
 			fprintf(stderr, "result %d/%d: %s\n", results, numsent, PQgetvalue(res, 0, 0));
 			PQclear(res);
-
-			res2 = PQgetResult(conn);
-			if (res2 != NULL)
-				pg_fatal("expected NULL, got %s",
-						 PQresStatus(PQresultStatus(res2)));
+			confirm_null_result(conn);
 			break;
 
 		case PGRES_PIPELINE_ABORTED:
 			fprintf(stderr, "result %d/%d: pipeline aborted\n", results, numsent);
-			res2 = PQgetResult(conn);
-			if (res2 != NULL)
-				pg_fatal("expected NULL, got %s",
-						 PQresStatus(PQresultStatus(res2)));
+			PQclear(res);
+			confirm_null_result(conn);
 			break;
 
 		default:
@@ -2271,7 +2117,7 @@ main(int argc, char **argv)
 {
 	const char *conninfo = "";
 	PGconn	   *conn;
-	FILE	   *trace;
+	FILE	   *trace = NULL;
 	char	   *testname;
 	int			numrows = 10000;
 	PGresult   *res;
@@ -2332,9 +2178,11 @@ main(int argc, char **argv)
 	res = PQexec(conn, "SET lc_messages TO \"C\"");
 	if (PQresultStatus(res) != PGRES_COMMAND_OK)
 		pg_fatal("failed to set \"lc_messages\": %s", PQerrorMessage(conn));
+	PQclear(res);
 	res = PQexec(conn, "SET debug_parallel_query = off");
 	if (PQresultStatus(res) != PGRES_COMMAND_OK)
 		pg_fatal("failed to set \"debug_parallel_query\": %s", PQerrorMessage(conn));
+	PQclear(res);
 
 	/* Set the trace file, if requested */
 	if (tracefile != NULL)
@@ -2388,5 +2236,9 @@ main(int argc, char **argv)
 
 	/* close the connection to the database and cleanup */
 	PQfinish(conn);
+
+	if (trace && trace != stdout)
+		fclose(trace);
+
 	return 0;
 }

view thread (9+ messages)  latest in thread

Message-ID: <2790652.1750536063@sss.pgh.pa.us>
Permalink:  ../2790652.1750536063@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/2790652.1750536063@sss.pgh.pa.us

copy link · copy postgr.es · copy git am

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: tgl@sss.pgh.pa.us, d.kovalenko@postgrespro.ru, alvherre@kurilemu.de, pgsql-bugs@lists.postgresql.org
  Subject: Re: BUG #18960: Mistake in test test_simple_pipeline (libpq_pipeline.c)
  In-Reply-To: <2790652.1750536063@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox