[PGPRO-5612] Support for checkunique parameter of amcheck.bt_index_check() function (PR #456)

Co-authored-by: Elena Indrupskaya <[email protected]>

Author: kulaginm

doc/pgprobackup.xml

@@ -4176,7 +4176,7 @@ pg_probackup restore -B <replaceable>backup_dir</replaceable> --instance <replac
 pg_probackup checkdb
 [-B <replaceable>backup_dir</replaceable>] [--instance <replaceable>instance_name</replaceable>] [-D <replaceable>data_dir</replaceable>]
 [--help] [-j <replaceable>num_threads</replaceable>] [--progress]
-[--skip-block-validation] [--amcheck] [--heapallindexed]
+[--skip-block-validation] [--amcheck [--checkunique] [--heapallindexed]]
 [<replaceable>connection_options</replaceable>] [<replaceable>logging_options</replaceable>]
 </programlisting>
       <para>
@@ -4195,17 +4195,24 @@ pg_probackup checkdb
         extension or the <application>amcheck_next</application> extension
         installed in the database to check its indexes. For databases
         without <application>amcheck</application>, index verification will be skipped.
+        Additional options <option>--checkunique</option> and <option>--heapallindexed</option>
+        are effective depending on the version of <application>amcheck</application> installed.
       </para>
       </listitem>
       </varlistentry>
 
       <varlistentry>
-<term><option>--skip-block-validation</option></term>
+<term><option>--checkunique</option></term>
       <listitem>
       <para>
-        Skip validation of data files. You can use this flag only
-        together with the <option>--amcheck</option> flag, so that only logical
-        verification of indexes is performed.
+        Verifies unique constraints during logical verification of indexes.
+        You can use this flag only together with the <option>--amcheck</option> flag when
+        the <application>amcheck</application> extension is
+        installed in the database.
+      </para>
+      <para>
+        This verification is only possible if it is supported by the version of the
+        <application>amcheck</application> extension you are using.
       </para>
       </listitem>
       </varlistentry>
@@ -4219,12 +4226,24 @@ pg_probackup checkdb
         <option>--amcheck</option> flag.
       </para>
       <para>
-        This check is only possible if you are using the
-        <application>amcheck</application> extension of version 2.0 or higher, or
-        the <application>amcheck_next</application> extension of any version.
+        This check is only possible if it is supported by the version of the
+        <application>amcheck</application> extension you are using or
+        if the <application>amcheck_next</application> extension is used instead.
+      </para>
+      </listitem>
+      </varlistentry>
+      <varlistentry>
+
+<term><option>--skip-block-validation</option></term>
+      <listitem>
+      <para>
+        Skip validation of data files. You can use this flag only
+        together with the <option>--amcheck</option> flag, so that only logical
+        verification of indexes is performed.
       </para>
       </listitem>
       </varlistentry>
+
     </variablelist>
     </para>
       <para>

src/checkdb.c

@@ -83,6 +83,7 @@ typedef struct pg_indexEntry
 	char *name;
 	char *namespace;
 	bool heapallindexed_is_supported;
+	bool checkunique_is_supported;
 	/* schema where amcheck extension is located */
 	char *amcheck_nspname;
 	/* lock for synchronization of parallel threads  */
@@ -351,10 +352,14 @@ get_index_list(const char *dbname, bool first_db_with_amcheck,
 {
 	PGresult   *res;
 	char *amcheck_nspname = NULL;
+	char *amcheck_extname = NULL;
+	char *amcheck_extversion = NULL;
 	int i;
 	bool heapallindexed_is_supported = false;
+	bool checkunique_is_supported = false;
 	parray *index_list = NULL;
 
+	/* Check amcheck extension version */
 	res = pgut_execute(db_conn, "SELECT "
 								"extname, nspname, extversion "
 								"FROM pg_catalog.pg_namespace n "
@@ -379,24 +384,68 @@ get_index_list(const char *dbname, bool first_db_with_amcheck,
 		return NULL;
 	}
 
+	amcheck_extname = pgut_malloc(strlen(PQgetvalue(res, 0, 0)) + 1);
+	strcpy(amcheck_extname, PQgetvalue(res, 0, 0));
 	amcheck_nspname = pgut_malloc(strlen(PQgetvalue(res, 0, 1)) + 1);
 	strcpy(amcheck_nspname, PQgetvalue(res, 0, 1));
+	amcheck_extversion = pgut_malloc(strlen(PQgetvalue(res, 0, 2)) + 1);
+	strcpy(amcheck_extversion, PQgetvalue(res, 0, 2));
+	PQclear(res);
 
 	/* heapallindexed_is_supported is database specific */
-	if (strcmp(PQgetvalue(res, 0, 2), "1.0") != 0 &&
-		strcmp(PQgetvalue(res, 0, 2), "1") != 0)
+	/* TODO this is wrong check, heapallindexed supported also in 1.1.1, 1.2 and 1.2.1... */
+	if (strcmp(amcheck_extversion, "1.0") != 0 &&
+		strcmp(amcheck_extversion, "1") != 0)
 			heapallindexed_is_supported = true;
 
 	elog(INFO, "Amchecking database '%s' using extension '%s' "
 			   "version %s from schema '%s'",
-				dbname, PQgetvalue(res, 0, 0), 
-				PQgetvalue(res, 0, 2), PQgetvalue(res, 0, 1));
+				dbname, amcheck_extname,
+				amcheck_extversion, amcheck_nspname);
 
 	if (!heapallindexed_is_supported && heapallindexed)
 		elog(WARNING, "Extension '%s' version %s in schema '%s'"
 					  "do not support 'heapallindexed' option",
-					   PQgetvalue(res, 0, 0), PQgetvalue(res, 0, 2),
-					   PQgetvalue(res, 0, 1));
+					   amcheck_extname, amcheck_extversion,
+					   amcheck_nspname);
+
+#ifndef PGPRO_EE
+	/*
+	 * Will support when the vanilla patch will commited https://commitfest.postgresql.org/32/2976/
+	 */
+	checkunique_is_supported = false;
+#else
+	/*
+	 * Check bt_index_check function signature to determine support of checkunique parameter
+	 * This can't be exactly checked by checking extension version,
+	 * For example, 1.1.1 and 1.2.1 supports this parameter, but 1.2 doesn't (PGPROEE-12.4.1)
+	 */
+	res = pgut_execute(db_conn, "SELECT "
+								"    oid "
+								"FROM pg_catalog.pg_proc "
+								"WHERE "
+								"    pronamespace = $1::regnamespace "
+								"AND proname = 'bt_index_check' "
+								"AND 'checkunique' = ANY(proargnames) "
+								"AND (pg_catalog.string_to_array(proargtypes::text, ' ')::regtype[])[pg_catalog.array_position(proargnames, 'checkunique')] = 'bool'::regtype",
+								1, (const char **) &amcheck_nspname);
+
+	if (PQresultStatus(res) != PGRES_TUPLES_OK)
+	{
+		PQclear(res);
+		elog(ERROR, "Cannot check 'checkunique' option is supported in bt_index_check function %s: %s",
+			dbname, PQerrorMessage(db_conn));
+	}
+
+	checkunique_is_supported = PQntuples(res) >= 1;
+	PQclear(res);
+#endif
+
+	if (!checkunique_is_supported && checkunique)
+		elog(WARNING, "Extension '%s' version %s in schema '%s' "
+					  "do not support 'checkunique' parameter",
+					   amcheck_extname, amcheck_extversion,
+					   amcheck_nspname);
 
 	/*
 	 * In order to avoid duplicates, select global indexes
@@ -453,6 +502,7 @@ get_index_list(const char *dbname, bool first_db_with_amcheck,
 		strcpy(ind->namespace, namespace);	/* enough buffer size guaranteed */
 
 		ind->heapallindexed_is_supported = heapallindexed_is_supported;
+		ind->checkunique_is_supported = checkunique_is_supported;
 		ind->amcheck_nspname = pgut_malloc(strlen(amcheck_nspname) + 1);
 		strcpy(ind->amcheck_nspname, amcheck_nspname);
 		pg_atomic_clear_flag(&ind->lock);
@@ -464,6 +514,9 @@ get_index_list(const char *dbname, bool first_db_with_amcheck,
 	}
 
 	PQclear(res);
+	free(amcheck_extversion);
+	free(amcheck_nspname);
+	free(amcheck_extname);
 
 	return index_list;
 }
@@ -473,46 +526,54 @@ static bool
 amcheck_one_index(check_indexes_arg *arguments,
 				 pg_indexEntry *ind)
 {
-	PGresult   *res;
-	char		*params[2];
+	PGresult	*res;
+	char		*params[3];
+	static const char	*queries[] = {
+			"SELECT %s.bt_index_check(index => $1)",
+			"SELECT %s.bt_index_check(index => $1, heapallindexed => $2)",
+			"SELECT %s.bt_index_check(index => $1, heapallindexed => $2, checkunique => $3)",
+	};
+	int			params_count;
 	char		*query = NULL;
 
-	params[0] = palloc(64);
+	if (interrupted)
+		elog(ERROR, "Interrupted");
 
+#define INDEXRELID 0
+#define HEAPALLINDEXED 1
+#define CHECKUNIQUE 2
 	/* first argument is index oid */
-	sprintf(params[0], "%u", ind->indexrelid);
+	params[INDEXRELID] = palloc(64);
+	sprintf(params[INDEXRELID], "%u", ind->indexrelid);
 	/* second argument is heapallindexed */
-	params[1] = heapallindexed ? "true" : "false";
+	params[HEAPALLINDEXED] = heapallindexed ? "true" : "false";
+	/* third optional argument is checkunique */
+	params[CHECKUNIQUE] = checkunique ? "true" : "false";
+#undef CHECKUNIQUE
+#undef HEAPALLINDEXED
 
-	if (interrupted)
-		elog(ERROR, "Interrupted");
-
-	if (ind->heapallindexed_is_supported)
-	{
-		query = palloc(strlen(ind->amcheck_nspname)+strlen("SELECT .bt_index_check($1, $2)")+1);
-		sprintf(query, "SELECT %s.bt_index_check($1, $2)", ind->amcheck_nspname);
+	params_count = ind->checkunique_is_supported ?
+			3 :
+			( ind->heapallindexed_is_supported ? 2 : 1 );
 
-		res = pgut_execute_parallel(arguments->conn_arg.conn,
-								arguments->conn_arg.cancel_conn,
-								query, 2, (const char **)params, true, true, true);
-	}
-	else
-	{
-		query = palloc(strlen(ind->amcheck_nspname)+strlen("SELECT .bt_index_check($1)")+1);
-		sprintf(query, "SELECT %s.bt_index_check($1)", ind->amcheck_nspname);
+	/*
+	 * Prepare query text with schema name
+	 * +1 for \0 and -2 for %s
+	 */
+	query = palloc(strlen(ind->amcheck_nspname) + strlen(queries[params_count - 1]) + 1 - 2);
+	sprintf(query, queries[params_count - 1], ind->amcheck_nspname);
 
-		res = pgut_execute_parallel(arguments->conn_arg.conn,
+	res = pgut_execute_parallel(arguments->conn_arg.conn,
 								arguments->conn_arg.cancel_conn,
-								query, 1, (const char **)params, true, true, true);
-	}
+								query, params_count, (const char **)params, true, true, true);
 
 	if (PQresultStatus(res) != PGRES_TUPLES_OK)
 	{
 		elog(WARNING, "Thread [%d]. Amcheck failed in database '%s' for index: '%s.%s': %s",
 					   arguments->thread_num, arguments->conn_opt.pgdatabase,
 					   ind->namespace, ind->name, PQresultErrorMessage(res));
 
-		pfree(params[0]);
+		pfree(params[INDEXRELID]);
 		pfree(query);
 		PQclear(res);
 		return false;
@@ -522,7 +583,8 @@ amcheck_one_index(check_indexes_arg *arguments,
 				arguments->thread_num,
 				arguments->conn_opt.pgdatabase, ind->namespace, ind->name);
 
-	pfree(params[0]);
+	pfree(params[INDEXRELID]);
+#undef INDEXRELID
 	pfree(query);
 	PQclear(res);
 	return true;

src/help.c

@@ -190,7 +190,7 @@ help_pg_probackup(void)
 	printf(_("\n  %s checkdb [-B backup-path] [--instance=instance_name]\n"), PROGRAM_NAME);
 	printf(_("                 [-D pgdata-path] [--progress] [-j num-threads]\n"));
 	printf(_("                 [--amcheck] [--skip-block-validation]\n"));
-	printf(_("                 [--heapallindexed]\n"));
+	printf(_("                 [--heapallindexed] [--checkunique]\n"));
 	printf(_("                 [--help]\n"));
 
 	printf(_("\n  %s show -B backup-path\n"), PROGRAM_NAME);
@@ -601,7 +601,7 @@ help_checkdb(void)
 	printf(_("\n%s checkdb [-B backup-path] [--instance=instance_name]\n"), PROGRAM_NAME);
 	printf(_("                 [-D pgdata-path] [-j num-threads] [--progress]\n"));
 	printf(_("                 [--amcheck] [--skip-block-validation]\n"));
-	printf(_("                 [--heapallindexed]\n\n"));
+	printf(_("                 [--heapallindexed] [--checkunique]\n\n"));
 
 	printf(_("  -B, --backup-path=backup-path    location of the backup storage area\n"));
 	printf(_("      --instance=instance_name     name of the instance\n"));
@@ -616,6 +616,8 @@ help_checkdb(void)
 	printf(_("                                   using 'amcheck' or 'amcheck_next' extensions\n"));
 	printf(_("      --heapallindexed             also check that heap is indexed\n"));
 	printf(_("                                   can be used only with '--amcheck' option\n"));
+	printf(_("      --checkunique                also check unique constraints\n"));
+	printf(_("                                   can be used only with '--amcheck' option\n"));
 
 	printf(_("\n  Logging options:\n"));
 	printf(_("      --log-level-console=log-level-console\n"));

src/pg_probackup.c

@@ -126,6 +126,7 @@ static parray *exclude_relative_paths_list = NULL;
 /* checkdb options */
 bool need_amcheck = false;
 bool heapallindexed = false;
+bool checkunique = false;
 bool amcheck_parent = false;
 
 /* delete options */
@@ -240,6 +241,7 @@ static ConfigOption cmd_options[] =
 	/* checkdb options */
 	{ 'b', 195, "amcheck",			&need_amcheck,		SOURCE_CMD_STRICT },
 	{ 'b', 196, "heapallindexed",	&heapallindexed,	SOURCE_CMD_STRICT },
+	{ 'b', 198, "checkunique",		&checkunique,		SOURCE_CMD_STRICT },
 	{ 'b', 197, "parent",			&amcheck_parent,	SOURCE_CMD_STRICT },
 	/* delete options */
 	{ 'b', 145, "wal",				&delete_wal,		SOURCE_CMD_STRICT },
@@ -596,6 +598,16 @@ main(int argc, char *argv[])
 		instance_config.pgdata == NULL)
 			elog(ERROR, "required parameter not specified: --instance");
 
+	/* Check checkdb command options consistency */
+	if (backup_subcmd == CHECKDB_CMD &&
+		!need_amcheck)
+	{
+		if (heapallindexed)
+			elog(ERROR, "--heapallindexed can only be used with --amcheck option");
+		if (checkunique)
+			elog(ERROR, "--checkunique can only be used with --amcheck option");
+	}
+
 	/* Usually checkdb for file logging requires log_directory
 	 * to be specified explicitly, but if backup_dir and instance name are provided,
 	 * checkdb can use the usual default values or values from config

src/pg_probackup.h

@@ -829,6 +829,7 @@ extern ShowFormat show_format;
 
 /* checkdb options */
 extern bool heapallindexed;
+extern bool checkunique;
 extern bool skip_block_validation;
 
 /* current settings */