From 1f679759a5711b4fa55cd7e021d92872def1646e Mon Sep 17 00:00:00 2001 From: Mark Dilger Date: Thu, 26 Aug 2021 11:52:50 -0700 Subject: [PATCH v2] Support amcheck of sequences. 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. --- contrib/amcheck/expected/check_heap.out | 6 ++- contrib/amcheck/t/001_verify_heapam.pl | 68 ++++++++++++++++++++++++- contrib/amcheck/verify_heapam.c | 10 +++- doc/src/sgml/amcheck.sgml | 8 +-- doc/src/sgml/ref/pg_amcheck.sgml | 15 +++--- src/bin/pg_amcheck/pg_amcheck.c | 6 +-- 6 files changed, 95 insertions(+), 18 deletions(-) diff --git a/contrib/amcheck/expected/check_heap.out b/contrib/amcheck/expected/check_heap.out index ad3086d2aa..c010361025 100644 --- 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 index 4f720a7ed0..ba40f64b58 100644 --- a/contrib/amcheck/t/001_verify_heapam.pl +++ b/contrib/amcheck/t/001_verify_heapam.pl @@ -8,7 +8,7 @@ use PostgresNode; use TestLib; use Fcntl qw(:seek); -use Test::More tests => 80; +use Test::More tests => 272; my ($node, $result); @@ -60,6 +60,22 @@ detects_no_corruption( "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 index 226271923a..85832183e6 100644 --- 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 index c570690b59..11d1eb5af2 100644 --- a/doc/src/sgml/amcheck.sgml +++ b/doc/src/sgml/amcheck.sgml @@ -220,10 +220,10 @@ SET client_min_messages = DEBUG1; - 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. The following optional arguments are recognized: diff --git a/doc/src/sgml/ref/pg_amcheck.sgml b/doc/src/sgml/ref/pg_amcheck.sgml index d00c48d0e7..1fd0ecd911 100644 --- a/doc/src/sgml/ref/pg_amcheck.sgml +++ b/doc/src/sgml/ref/pg_amcheck.sgml @@ -41,8 +41,9 @@ PostgreSQL documentation - 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. @@ -124,7 +125,7 @@ PostgreSQL documentation This is similar to the option, except that - it applies only to indexes, not tables. + it applies only to indexes, not to other relation types. @@ -140,7 +141,7 @@ PostgreSQL documentation This is similar to the option, - except that it applies only to indexes, not tables. + except that it applies only to indexes, not other relation types. @@ -236,7 +237,8 @@ PostgreSQL documentation This is similar to the option, except that - it applies only to tables, not indexes. + it applies only to tables, materialized views, and sequences, not to + indexes. @@ -252,7 +254,8 @@ PostgreSQL documentation This is similar to the option, - except that it applies only to tables, not indexes. + except that it applies only to tables, materialized views, and + sequences, not to indexes. diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c index f4b1ef95e9..9d9c3392d9 100644 --- a/src/bin/pg_amcheck/pg_amcheck.c +++ b/src/bin/pg_amcheck/pg_amcheck.c @@ -1915,14 +1915,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); -- 2.21.1 (Apple Git-122.3)