Fix build with Postgres 166 libpq-dev in debian. (#1006)

* Add build support for Postgres 16.

Build passes locally with a released version of Postgres 16.0 and still
builds with previous version. Still missing CI changes to pass all the test
suite with PGVERSION=16.

See #1005.

* Allow src/bin/lib/pg/snprintf.c to build with older Postgres versions.

* Allow code to build with newer Postgres releases.

The internal organisation of the header files in Postgres 16 changed in a
way that we can't poke into libpq-int.h anymore, when building on debian,
because of the way the include/common bits are including in libpq (16)
rather than in the postgresql-server-dev-16 parts.

That said, we can stop looking into the internal details of libpq:

  - connection->last_sqlstate allowed an optimisation to conclude that the
    connection is okay early in the polling.

  - connection->auth_req_received allowed to discard authentication error as
    connectivity problems, but we decided in the past that we need to deploy
	a monitor user on the Postgres nodes anyway, to avoid flooding Postgres
	logs with authentication errors.

* Review Citus/Postgres compat matrix.

Allow building for Postgres 16 and Citus 12.1.0.

* Activate CI tests for Postgres 16.

* The GUC_NO_SHOW_ALL flag now requires GUC_NOT_IN_SAMPLE.

* Fix previous commit.

Author: dimitri

.github/workflows/run-tests.yml

@@ -23,6 +23,7 @@ jobs:
           - 13
           - 14
           - 15
+          - 16
         TEST:
           - multi
           - single

Makefile

@@ -28,8 +28,9 @@ PGVERSION ?= 14
 BUILD_ARGS_PG11 = --build-arg PGVERSION=11 --build-arg CITUSTAG=v9.5.10
 BUILD_ARGS_PG12 = --build-arg PGVERSION=12 --build-arg CITUSTAG=v10.2.9
 BUILD_ARGS_PG13 = --build-arg PGVERSION=13 --build-arg CITUSTAG=v10.2.9
-BUILD_ARGS_PG14 = --build-arg PGVERSION=14 --build-arg CITUSTAG=v11.2.1
-BUILD_ARGS_PG15 = --build-arg PGVERSION=15 --build-arg CITUSTAG=v11.2.1
+BUILD_ARGS_PG14 = --build-arg PGVERSION=14 --build-arg CITUSTAG=v12.1.0
+BUILD_ARGS_PG15 = --build-arg PGVERSION=15 --build-arg CITUSTAG=v12.1.0
+BUILD_ARGS_PG16 = --build-arg PGVERSION=16 --build-arg CITUSTAG=v12.1.0
 
 NOSETESTS = $(shell which nosetests3 || which nosetests)
 
@@ -279,6 +280,9 @@ build-test-pg14: version
 build-test-pg15: version
 	docker build $(BUILD_ARGS_PG15) --target test -t $(TEST_CONTAINER_NAME):pg15 .
 
+build-test-pg16: version
+	docker build $(BUILD_ARGS_PG16) --target test -t $(TEST_CONTAINER_NAME):pg16 .
+
 
 build-test-image: build-test-pg$(PGVERSION) ;
 
@@ -310,10 +314,13 @@ build-pg14: build-test-pg14
 build-pg15: build-test-pg15
 	docker build $(BUILD_ARGS_PG15) -t $(CONTAINER_NAME):pg15 .
 
-build: build-pg11 build-pg12 build-pg13 build-pg14 build-pg15 ;
+build-pg16: build-test-pg16
+	docker build $(BUILD_ARGS_PG16) -t $(CONTAINER_NAME):pg16 .
+
+build: build-pg11 build-pg12 build-pg13 build-pg14 build-pg15 build-pg16 ;
 
 build-check:
-	for v in 11 12 13 14 15; do \
+	for v in 11 12 13 14 15 16; do \
 		docker run --rm -t pg_auto_failover_test:pg$$v pg_autoctl version --json | jq ".pg_version" | xargs echo $$v: ; \
 	done
 

src/bin/lib/pg/snprintf.c

@@ -2,7 +2,7 @@
  * Copyright (c) 1983, 1995, 1996 Eric P. Allman
  * Copyright (c) 1988, 1993
  *	The Regents of the University of California.  All rights reserved.
- * Portions Copyright (c) 1996-2021, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -739,12 +739,8 @@ find_arguments(const char *format, va_list args,
 	int			longflag;
 	int			fmtpos;
 	int			i;
-	int			last_dollar;
-	PrintfArgType argtypes[PG_NL_ARGMAX + 1];
-
-	/* Initialize to "no dollar arguments known" */
-	last_dollar = 0;
-	MemSet(argtypes, 0, sizeof(argtypes));
+	int			last_dollar = 0;	/* Init to "no dollar arguments known" */
+	PrintfArgType argtypes[PG_NL_ARGMAX + 1] = {0};
 
 	/*
 	 * This loop must accept the same format strings as the one in dopr().
@@ -985,8 +981,8 @@ fmtptr(const void *value, PrintfTarget *target)
 	int			vallen;
 	char		convert[64];
 
-	/* we rely on regular C library's sprintf to do the basic conversion */
-	vallen = sprintf(convert, "%p", value);
+	/* we rely on regular C library's snprintf to do the basic conversion */
+	vallen = snprintf(convert, sizeof(convert), "%p", value);
 	if (vallen < 0)
 		target->failed = true;
 	else
@@ -1136,11 +1132,11 @@ fmtfloat(double value, char type, int forcesign, int leftjust,
 	int			padlen;			/* amount to pad with spaces */
 
 	/*
-	 * We rely on the regular C library's sprintf to do the basic conversion,
+	 * We rely on the regular C library's snprintf to do the basic conversion,
 	 * then handle padding considerations here.
 	 *
 	 * The dynamic range of "double" is about 1E+-308 for IEEE math, and not
-	 * too wildly more than that with other hardware.  In "f" format, sprintf
+	 * too wildly more than that with other hardware.  In "f" format, snprintf
 	 * could therefore generate at most 308 characters to the left of the
 	 * decimal point; while we need to allow the precision to get as high as
 	 * 308+17 to ensure that we don't truncate significant digits from very
@@ -1192,14 +1188,14 @@ fmtfloat(double value, char type, int forcesign, int leftjust,
 			fmt[2] = '*';
 			fmt[3] = type;
 			fmt[4] = '\0';
-			vallen = sprintf(convert, fmt, prec, value);
+			vallen = snprintf(convert, sizeof(convert), fmt, prec, value);
 		}
 		else
 		{
 			fmt[0] = '%';
 			fmt[1] = type;
 			fmt[2] = '\0';
-			vallen = sprintf(convert, fmt, value);
+			vallen = snprintf(convert, sizeof(convert), fmt, value);
 		}
 		if (vallen < 0)
 			goto fail;
@@ -1328,7 +1324,7 @@ pg_strfromd(char *str, size_t count, int precision, double value)
 			fmt[2] = '*';
 			fmt[3] = 'g';
 			fmt[4] = '\0';
-			vallen = sprintf(convert, fmt, precision, value);
+			vallen = snprintf(convert, sizeof(convert), fmt, precision, value);
 			if (vallen < 0)
 			{
 				target.failed = true;

src/bin/pg_autoctl/azure.c

@@ -1585,9 +1585,9 @@ azure_fetch_resource_list(const char *group, AzureRegionResources *azRegion)
 			}
 			else if (streq(type, "Microsoft.Compute/virtualMachines"))
 			{
-				int index = azure_node_index_from_name(group, name);
+				int idx = azure_node_index_from_name(group, name);
 
-				strlcpy(azRegion->vmArray[index].name, name, NAMEDATALEN);
+				strlcpy(azRegion->vmArray[idx].name, name, NAMEDATALEN);
 
 				log_info("Found existing VM \"%s\"", name);
 			}

src/bin/pg_autoctl/cli_common.c

@@ -1524,7 +1524,7 @@ keeper_cli_print_version(int argc, char **argv)
 				"pg_autoctl extension version %s\n",
 				PG_AUTOCTL_EXTENSION_VERSION);
 		fformat(stdout, "compiled with %s\n", PG_VERSION_STR);
-		fformat(stdout, "compatible with Postgres 11, 12, 13, 14, and 15\n");
+		fformat(stdout, "compatible with Postgres 11, 12, 13, 14, 15, and 16\n");
 	}
 
 	exit(0);

src/bin/pg_autoctl/pgctl.c

@@ -1837,9 +1837,6 @@ pg_log_startup(const char *pgdata, int logLevel)
 		 */
 		if (pgLogFileMtime >= pgStartupMtime)
 		{
-			char *fileContents;
-			long fileSize;
-
 			log_level(pathLogLevel,
 					  "Postgres logs from \"%s\":", pgLogFilePath);
 

src/bin/pg_autoctl/pgsetup.c

@@ -501,7 +501,7 @@ get_pgpid(PostgresSetup *pgSetup, bool pgIsNotRunningIsOk)
 		}
 		else
 		{
-			int logLevel = pgIsNotRunningIsOk ? LOG_DEBUG : LOG_WARN;
+			logLevel = pgIsNotRunningIsOk ? LOG_DEBUG : LOG_WARN;
 
 			log_level(logLevel,
 					  "Read a stale pid in \"postmaster.pid\": %d", pid);

src/bin/pg_autoctl/primary_standby.c

@@ -1239,7 +1239,7 @@ postgres_maybe_do_crash_recovery(LocalPostgresServer *postgres)
 				 * Now stop Postgres by just killing our child process, and
 				 * wait until the child process has finished with waitpid().
 				 */
-				int wpid, status;
+				int wpid;
 
 				do {
 					if (kill(fpid, SIGTERM) != 0)

src/bin/pg_autoctl/watch.c

@@ -1430,7 +1430,7 @@ print_event(WatchContext *context, EventColPolicy *policy, int index, int r, int
 			case EVENT_COLUMN_TYPE_DESCRIPTION:
 			{
 				char *text = event->description;
-				int len = strlen(text);
+				len = strlen(text);
 
 				/* when KEY_END is used, ensure we see the end of text */
 				if (context->move == WATCH_MOVE_FOCUS_END)

src/monitor/health_check_worker.c

@@ -37,7 +37,6 @@
 #include "fmgr.h"
 #include "lib/stringinfo.h"
 #include "libpq-fe.h"
-#include "libpq-int.h"
 #include "libpq/pqsignal.h"
 #include "poll.h"
 #include "sys/time.h"
@@ -58,8 +57,6 @@
 	"connect_timeout=%u"
 #define MAX_CONN_INFO_SIZE 1024
 
-#define CANNOT_CONNECT_NOW "57P03"
-
 
 typedef enum
 {
@@ -939,20 +936,9 @@ ManageHealthCheck(HealthCheck *healthCheck, struct timeval currentTime)
 				break;
 			}
 
-			/* This logic is taken from libpq's internal_ping (fe-connect.c) */
 			pollingStatus = PQconnectPoll(connection);
-			char *sqlstate = connection->last_sqlstate;
-			bool receivedSqlstate = (sqlstate != NULL && strlen(sqlstate) == 5);
-			bool cannotConnectNowSqlstate = (receivedSqlstate &&
-											 strcmp(sqlstate, CANNOT_CONNECT_NOW) == 0);
-
-			if (pollingStatus == PGRES_POLLING_OK ||
-
-			    /* an auth request means pg is running */
-				connection->auth_req_received ||
 
-			    /* any error but CANNOT_CONNECT means the db is accepting connections */
-				(receivedSqlstate && !cannotConnectNowSqlstate))
+			if (pollingStatus == PGRES_POLLING_OK)
 			{
 				PQfinish(connection);