Support amcheck of sequences

petere · petere · commit c3b011d99181 · 2021-09-28T15:26:25.000+02:00
Sequences were left out of the list of relation kinds that verify_heapam knew how to check, though it is fairly trivial to allow them. Doing that, and while at it, updating pg_amcheck to include sequences in relations matched by table and relation patterns. Author: Mark Dilger <mark.dilger@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/81ad4757-92c1-4aa3-7bee-f609544837e3%40enterprisedb.com
diff --git a/contrib/amcheck/expected/check_heap.out b/contrib/amcheck/expected/check_heap.out
@@ -180,8 +180,10 @@ CREATE SEQUENCE test_sequence;
 SELECT * FROM verify_heapam('test_sequence',
 							startblock := NULL,
 							endblock := NULL);
-ERROR:  cannot check relation "test_sequence"
-DETAIL:  This operation is not supported for sequences.
+ blkno | offnum | attnum | msg 
+-------+--------+--------+-----
+(0 rows)
+
 -- Check that foreign tables are rejected
 CREATE FOREIGN DATA WRAPPER dummy;
 CREATE SERVER dummy_server FOREIGN DATA WRAPPER dummy;
diff --git a/contrib/amcheck/t/001_verify_heapam.pl b/contrib/amcheck/t/001_verify_heapam.pl
@@ -8,7 +8,7 @@
 use TestLib;
 
 use Fcntl qw(:seek);
-use Test::More tests => 80;
+use Test::More tests => 272;
 
 my ($node, $result);
 
@@ -60,6 +60,22 @@
 	"verify_heapam('test', skip := 'all-frozen')",
 	"all-frozen corrupted table skipping all-frozen");
 
+#
+# Check a sequence with no corruption.  The current implementation of sequences
+# doesn't require its own test setup, since sequences are really just heap
+# tables under-the-hood.  To guard against future implementation changes made
+# without remembering to update verify_heapam, we create and exercise a
+# sequence, checking along the way that it passes corruption checks.
+#
+fresh_test_sequence('test_seq');
+check_all_options_uncorrupted('test_seq', 'plain');
+advance_test_sequence('test_seq');
+check_all_options_uncorrupted('test_seq', 'plain');
+set_test_sequence('test_seq');
+check_all_options_uncorrupted('test_seq', 'plain');
+reset_test_sequence('test_seq');
+check_all_options_uncorrupted('test_seq', 'plain');
+
 # Returns the filesystem path for the named relation.
 sub relation_filepath
 {
@@ -110,6 +126,56 @@ sub fresh_test_table
 	));
 }
 
+# Create a test sequence of the given name.
+sub fresh_test_sequence
+{
+	my ($seqname) = @_;
+
+	return $node->safe_psql(
+		'postgres', qq(
+		DROP SEQUENCE IF EXISTS $seqname CASCADE;
+		CREATE SEQUENCE $seqname
+			INCREMENT BY 13
+			MINVALUE 17
+			START WITH 23;
+		SELECT nextval('$seqname');
+		SELECT setval('$seqname', currval('$seqname') + nextval('$seqname'));
+	));
+}
+
+# Call SQL functions to increment the sequence
+sub advance_test_sequence
+{
+	my ($seqname) = @_;
+
+	return $node->safe_psql(
+		'postgres', qq(
+		SELECT nextval('$seqname');
+	));
+}
+
+# Call SQL functions to set the sequence
+sub set_test_sequence
+{
+	my ($seqname) = @_;
+
+	return $node->safe_psql(
+		'postgres', qq(
+		SELECT setval('$seqname', 102);
+	));
+}
+
+# Call SQL functions to reset the sequence
+sub reset_test_sequence
+{
+	my ($seqname) = @_;
+
+	return $node->safe_psql(
+		'postgres', qq(
+		ALTER SEQUENCE $seqname RESTART WITH 51
+	));
+}
+
 # Stops the test node, corrupts the first page of the named relation, and
 # restarts the node.
 sub corrupt_first_page
diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
@@ -305,14 +305,20 @@ verify_heapam(PG_FUNCTION_ARGS)
 	 */
 	if (ctx.rel->rd_rel->relkind != RELKIND_RELATION &&
 		ctx.rel->rd_rel->relkind != RELKIND_MATVIEW &&
-		ctx.rel->rd_rel->relkind != RELKIND_TOASTVALUE)
+		ctx.rel->rd_rel->relkind != RELKIND_TOASTVALUE &&
+		ctx.rel->rd_rel->relkind != RELKIND_SEQUENCE)
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("cannot check relation \"%s\"",
 						RelationGetRelationName(ctx.rel)),
 				 errdetail_relkind_not_supported(ctx.rel->rd_rel->relkind)));
 
-	if (ctx.rel->rd_rel->relam != HEAP_TABLE_AM_OID)
+	/*
+	 * Sequences always use heap AM, but they don't show that in the catalogs.
+	 * Other relkinds might be using a different AM, so check.
+	 */
+	if (ctx.rel->rd_rel->relkind != RELKIND_SEQUENCE &&
+		ctx.rel->rd_rel->relam != HEAP_TABLE_AM_OID)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("only heap AM is supported")));
diff --git a/doc/src/sgml/amcheck.sgml b/doc/src/sgml/amcheck.sgml
@@ -220,10 +220,10 @@ SET client_min_messages = DEBUG1;
     </term>
     <listitem>
      <para>
-      Checks a table for structural corruption, where pages in the relation
-      contain data that is invalidly formatted, and for logical corruption,
-      where pages are structurally valid but inconsistent with the rest of the
-      database cluster.
+      Checks a table, sequence, or materialized view for structural corruption,
+      where pages in the relation contain data that is invalidly formatted, and
+      for logical corruption, where pages are structurally valid but
+      inconsistent with the rest of the database cluster.
      </para>
      <para>
       The following optional arguments are recognized:
diff --git a/doc/src/sgml/ref/pg_amcheck.sgml b/doc/src/sgml/ref/pg_amcheck.sgml
@@ -41,8 +41,9 @@ PostgreSQL documentation
   </para>
 
   <para>
-   Only table relations and btree indexes are currently supported.  Other
-   relation types are silently skipped.
+   Only ordinary and toast table relations, materialized views, sequences, and
+   btree indexes are currently supported.  Other relation types are silently
+   skipped.
   </para>
 
   <para>
@@ -124,7 +125,7 @@ PostgreSQL documentation
       </para>
       <para>
        This is similar to the <option>--relation</option> option, except that
-       it applies only to indexes, not tables.
+       it applies only to indexes, not to other relation types.
       </para>
      </listitem>
     </varlistentry>
@@ -140,7 +141,7 @@ PostgreSQL documentation
       </para>
       <para>
        This is similar to the <option>--exclude-relation</option> option,
-       except that it applies only to indexes, not tables.
+       except that it applies only to indexes, not other relation types.
       </para>
      </listitem>
     </varlistentry>
@@ -236,7 +237,8 @@ PostgreSQL documentation
       </para>
       <para>
        This is similar to the <option>--relation</option> option, except that
-       it applies only to tables, not indexes.
+       it applies only to tables, materialized views, and sequences, not to
+       indexes.
       </para>
      </listitem>
     </varlistentry>
@@ -252,7 +254,8 @@ PostgreSQL documentation
       </para>
       <para>
        This is similar to the <option>--exclude-relation</option> option,
-       except that it applies only to tables, not indexes.
+       except that it applies only to tables, materialized views, and
+       sequences, not to indexes.
       </para>
      </listitem>
     </varlistentry>
diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c
@@ -1910,14 +1910,14 @@ compile_relation_list_one_db(PGconn *conn, SimplePtrList *relations,
 	if (opts.allrel)
 		appendPQExpBuffer(&sql,
 						  " AND c.relam = %u "
-						  "AND c.relkind IN ('r', 'm', 't') "
+						  "AND c.relkind IN ('r', 'S', 'm', 't') "
 						  "AND c.relnamespace != %u",
 						  HEAP_TABLE_AM_OID, PG_TOAST_NAMESPACE);
 	else
 		appendPQExpBuffer(&sql,
 						  " AND c.relam IN (%u, %u)"
-						  "AND c.relkind IN ('r', 'm', 't', 'i') "
-						  "AND ((c.relam = %u AND c.relkind IN ('r', 'm', 't')) OR "
+						  "AND c.relkind IN ('r', 'S', 'm', 't', 'i') "
+						  "AND ((c.relam = %u AND c.relkind IN ('r', 'S', 'm', 't')) OR "
 						  "(c.relam = %u AND c.relkind = 'i'))",
 						  HEAP_TABLE_AM_OID, BTREE_AM_OID,
 						  HEAP_TABLE_AM_OID, BTREE_AM_OID);
