From 33dde1068992cc5f0f869efa9bec3e5fa7e912e9 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 15 Oct 2014 17:09:53 +0900 Subject: [PATCH] Fix non-transactional ANALYZE causing relation inconsistencies When ANALYZE is used within a transaction that changes a given table schema by some other DDL queries by (manipulating for example indexes, triggers, etc.), the pg_class entry of this relation was getting updated in a non-transactional way causing relation definition inconsistencies. This commit changes ANALYZE to always have it perform transactional updates of the relation definitions it manipulates in pg_class, to make it transaction-safe. --- src/backend/commands/analyze.c | 6 ++++-- src/backend/commands/vacuum.c | 18 +++++++++++++++--- src/backend/commands/vacuumlazy.c | 6 ++++-- src/include/commands/vacuum.h | 3 ++- src/test/regress/expected/alter_table.out | 20 ++++++++++++++++++++ src/test/regress/sql/alter_table.sql | 13 +++++++++++++ 6 files changed, 58 insertions(+), 8 deletions(-) diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index c09ca7e..f1e447d 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -584,7 +584,8 @@ do_analyze_rel(Relation onerel, VacuumStmt *vacstmt, visibilitymap_count(onerel), hasindex, InvalidTransactionId, - InvalidMultiXactId); + InvalidMultiXactId, + !(vacstmt->options & VACOPT_VACUUM)); /* * Same for indexes. Vacuum always scans all indexes, so if we're part of @@ -605,7 +606,8 @@ do_analyze_rel(Relation onerel, VacuumStmt *vacstmt, 0, false, InvalidTransactionId, - InvalidMultiXactId); + InvalidMultiXactId, + !(vacstmt->options & VACOPT_VACUUM)); } } diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index e5fefa3..6f05a74 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -672,7 +672,7 @@ vac_update_relstats(Relation relation, BlockNumber num_pages, double num_tuples, BlockNumber num_all_visible_pages, bool hasindex, TransactionId frozenxid, - MultiXactId minmulti) + MultiXactId minmulti, bool is_vacuum) { Oid relid = RelationGetRelid(relation); Relation rd; @@ -768,9 +768,21 @@ vac_update_relstats(Relation relation, dirty = true; } - /* If anything changed, write out the tuple. */ + /* + * If anything changed, write out the tuple. For ANALYZE, this change + * needs to be transactional as this query can be run in a transaction + * block. + */ if (dirty) - heap_inplace_update(rd, ctup); + { + if (is_vacuum) + { + simple_heap_update(rd, &ctup->t_self, ctup); + CatalogUpdateIndexes(rd, ctup); + } + else + heap_inplace_update(rd, ctup); + } heap_close(rd, RowExclusiveLock); } diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c index 5d6d031..3778d9d 100644 --- a/src/backend/commands/vacuumlazy.c +++ b/src/backend/commands/vacuumlazy.c @@ -309,7 +309,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt, new_rel_allvisible, vacrelstats->hasindex, new_frozen_xid, - new_min_multi); + new_min_multi, + false); /* report results to the stats collector, too */ new_live_tuples = new_rel_tuples - vacrelstats->new_dead_tuples; @@ -1377,7 +1378,8 @@ lazy_cleanup_index(Relation indrel, 0, false, InvalidTransactionId, - InvalidMultiXactId); + InvalidMultiXactId, + false); ereport(elevel, (errmsg("index \"%s\" now contains %.0f row versions in %u pages", diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index d33552a..9781926 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -156,7 +156,8 @@ extern void vac_update_relstats(Relation relation, BlockNumber num_all_visible_pages, bool hasindex, TransactionId frozenxid, - MultiXactId minmulti); + MultiXactId minmulti, + bool is_vacuum); extern void vacuum_set_xid_limits(Relation rel, int freeze_min_age, int freeze_table_age, int multixact_freeze_min_age, diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 10f45f2..f5ae511 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -2517,3 +2517,23 @@ ALTER TABLE logged1 SET UNLOGGED; -- silently do nothing DROP TABLE logged3; DROP TABLE logged2; DROP TABLE logged1; +-- check presence of foreign key with transactional ANALYZE +CREATE TABLE check_fk_presence_1 (id int PRIMARY KEY, t text); +CREATE TABLE check_fk_presence_2 (id int REFERENCES check_fk_presence_1, t text); +BEGIN; +ALTER TABLE check_fk_presence_2 DROP CONSTRAINT check_fk_presence_2_id_fkey; +COPY check_fk_presence_2 FROM stdin; +ANALYZE check_fk_presence_2; +ALTER TABLE check_fk_presence_2 ADD FOREIGN KEY (table1_fk) REFERENCES check_fk_presence_1 (id); +ERROR: column "table1_fk" referenced in foreign key constraint does not exist +ROLLBACK; +\d check_fk_presence_2 +Table "public.check_fk_presence_2" + Column | Type | Modifiers +--------+---------+----------- + id | integer | + t | text | +Foreign-key constraints: + "check_fk_presence_2_id_fkey" FOREIGN KEY (id) REFERENCES check_fk_presence_1(id) + +DROP TABLE check_fk_presence_1, check_fk_presence_2; diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 12fd7c2..05d8226 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1676,3 +1676,16 @@ ALTER TABLE logged1 SET UNLOGGED; -- silently do nothing DROP TABLE logged3; DROP TABLE logged2; DROP TABLE logged1; +-- check presence of foreign key with transactional ANALYZE +CREATE TABLE check_fk_presence_1 (id int PRIMARY KEY, t text); +CREATE TABLE check_fk_presence_2 (id int REFERENCES check_fk_presence_1, t text); +BEGIN; +ALTER TABLE check_fk_presence_2 DROP CONSTRAINT check_fk_presence_2_id_fkey; +COPY check_fk_presence_2 FROM stdin; +1 test +\. +ANALYZE check_fk_presence_2; +ALTER TABLE check_fk_presence_2 ADD FOREIGN KEY (table1_fk) REFERENCES check_fk_presence_1 (id); +ROLLBACK; +\d check_fk_presence_2 +DROP TABLE check_fk_presence_1, check_fk_presence_2; -- 2.1.2