On Wed, Apr 25, 2018 at 8:06 AM, Teodor Sigaev <teo...@sigaev.ru> wrote:
> Leave it in both places. In ideal world, in amcheck test we need to create a
> broken index, but I don't know a correct way to do it :)
There are many ways to create a broken index, but they're hard to make
work with pg_regress. :-)
It looks like you pushed v1, which didn't have the tests and other
changes you asked for. Attached patch adds those back.
Thanks
--
Peter Geoghegan
From fc2d158d3d1c52e94a6d3f66c65027bf09b64f7f Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <p...@bowt.ie>
Date: Thu, 19 Apr 2018 17:45:26 -0700
Subject: [PATCH] Add amcheck missing downlink tests.
Also use palloc0() for main amcheck state, and adjust a few comments.
Author: Peter Geoghegan
---
contrib/amcheck/expected/check_btree.out | 23 ++++++++++++++++++++++-
contrib/amcheck/sql/check_btree.sql | 20 +++++++++++++++++++-
contrib/amcheck/verify_nbtree.c | 14 ++++++++------
3 files changed, 49 insertions(+), 8 deletions(-)
diff --git a/contrib/amcheck/expected/check_btree.out b/contrib/amcheck/expected/check_btree.out
index ed80ac4..e864579 100644
--- a/contrib/amcheck/expected/check_btree.out
+++ b/contrib/amcheck/expected/check_btree.out
@@ -1,7 +1,12 @@
--- minimal test, basically just verifying that amcheck
CREATE TABLE bttest_a(id int8);
CREATE TABLE bttest_b(id int8);
CREATE TABLE bttest_multi(id int8, data int8);
+CREATE TABLE delete_test_table (a bigint, b bigint, c bigint, d bigint);
+-- Stabalize tests
+ALTER TABLE bttest_a SET (autovacuum_enabled = false);
+ALTER TABLE bttest_b SET (autovacuum_enabled = false);
+ALTER TABLE bttest_multi SET (autovacuum_enabled = false);
+ALTER TABLE delete_test_table SET (autovacuum_enabled = false);
INSERT INTO bttest_a SELECT * FROM generate_series(1, 100000);
INSERT INTO bttest_b SELECT * FROM generate_series(100000, 1, -1);
INSERT INTO bttest_multi SELECT i, i%2 FROM generate_series(1, 100000) as i;
@@ -120,9 +125,25 @@ SELECT bt_index_parent_check('bttest_multi_idx', true);
(1 row)
+--
+-- Test for multilevel page deletion/downlink present checks
+--
+INSERT INTO delete_test_table SELECT i, 1, 2, 3 FROM generate_series(1,80000) i;
+ALTER TABLE delete_test_table ADD PRIMARY KEY (a,b,c,d);
+DELETE FROM delete_test_table WHERE a > 40000;
+VACUUM delete_test_table;
+DELETE FROM delete_test_table WHERE a > 10;
+VACUUM delete_test_table;
+SELECT bt_index_parent_check('delete_test_table_pkey', true);
+ bt_index_parent_check
+-----------------------
+
+(1 row)
+
-- cleanup
DROP TABLE bttest_a;
DROP TABLE bttest_b;
DROP TABLE bttest_multi;
+DROP TABLE delete_test_table;
DROP OWNED BY bttest_role; -- permissions
DROP ROLE bttest_role;
diff --git a/contrib/amcheck/sql/check_btree.sql b/contrib/amcheck/sql/check_btree.sql
index 4ca9d2d..7b1ab4f 100644
--- a/contrib/amcheck/sql/check_btree.sql
+++ b/contrib/amcheck/sql/check_btree.sql
@@ -1,7 +1,13 @@
--- minimal test, basically just verifying that amcheck
CREATE TABLE bttest_a(id int8);
CREATE TABLE bttest_b(id int8);
CREATE TABLE bttest_multi(id int8, data int8);
+CREATE TABLE delete_test_table (a bigint, b bigint, c bigint, d bigint);
+
+-- Stabalize tests
+ALTER TABLE bttest_a SET (autovacuum_enabled = false);
+ALTER TABLE bttest_b SET (autovacuum_enabled = false);
+ALTER TABLE bttest_multi SET (autovacuum_enabled = false);
+ALTER TABLE delete_test_table SET (autovacuum_enabled = false);
INSERT INTO bttest_a SELECT * FROM generate_series(1, 100000);
INSERT INTO bttest_b SELECT * FROM generate_series(100000, 1, -1);
@@ -71,9 +77,21 @@ TRUNCATE bttest_multi;
INSERT INTO bttest_multi SELECT i, i%2 FROM generate_series(1, 100000) as i;
SELECT bt_index_parent_check('bttest_multi_idx', true);
+--
+-- Test for multilevel page deletion/downlink present checks
+--
+INSERT INTO delete_test_table SELECT i, 1, 2, 3 FROM generate_series(1,80000) i;
+ALTER TABLE delete_test_table ADD PRIMARY KEY (a,b,c,d);
+DELETE FROM delete_test_table WHERE a > 40000;
+VACUUM delete_test_table;
+DELETE FROM delete_test_table WHERE a > 10;
+VACUUM delete_test_table;
+SELECT bt_index_parent_check('delete_test_table_pkey', true);
+
-- cleanup
DROP TABLE bttest_a;
DROP TABLE bttest_b;
DROP TABLE bttest_multi;
+DROP TABLE delete_test_table;
DROP OWNED BY bttest_role; -- permissions
DROP ROLE bttest_role;
diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
index 9449529..a3a5287 100644
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -340,7 +340,7 @@ bt_check_every_level(Relation rel, Relation heaprel, bool readonly,
/*
* Initialize state for entire verification operation
*/
- state = palloc(sizeof(BtreeCheckState));
+ state = palloc0(sizeof(BtreeCheckState));
state->rel = rel;
state->heaprel = heaprel;
state->readonly = readonly;
@@ -772,13 +772,15 @@ nextpage:
* - That tuples report that they have the expected number of attributes.
* INCLUDE index pivot tuples should not contain non-key attributes.
*
- * Furthermore, when state passed shows ShareLock held, and target page is
- * internal page, function also checks:
+ * Furthermore, when state passed shows ShareLock held, function also checks:
*
* - That all child pages respect downlinks lower bound.
*
+ * - That downlink to block was encountered in parent where that's expected.
+ * (Limited to heapallindexed readonly callers.)
+ *
* This is also where heapallindexed callers use their Bloom filter to
- * fingerprint IndexTuples.
+ * fingerprint IndexTuples for later IndexBuildHeapScan() verification.
*
* Note: Memory allocated in this routine is expected to be released by caller
* resetting state->targetcontext.
@@ -1074,7 +1076,7 @@ bt_target_page_check(BtreeCheckState *state)
/*
* * Check if page has a downlink in parent *
*
- * This can only be checked in readonly + heapallindexed case.
+ * This can only be checked in heapallindexed + readonly case.
*/
if (state->heapallindexed && state->readonly)
bt_downlink_missing_check(state);
@@ -1561,7 +1563,7 @@ bt_downlink_missing_check(BtreeCheckState *state)
* infinity items. Besides, bt_downlink_check() is unwilling to descend
* multiple levels. (The similar bt_downlink_check() P_ISDELETED() check
* within bt_check_level_from_leftmost() won't reach the page either, since
- * the leaf's live siblings should have their sibling links updating to
+ * the leaf's live siblings should have their sibling links updated to
* bypass the deletion target page when it is marked fully dead.)
*
* If this error is raised, it might be due to a previous multi-level page
--
2.7.4
