Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commite0722d9

Browse files
committed
Avoid corrupting tables when ANALYZE inside a transaction is rolled back.
VACUUM and ANALYZE update the target table's pg_class row in-place, that isnontransactionally. This is OK, more or less, for the statistical columns,which are mostly nontransactional anyhow. It's not so OK for the DDL hintflags (relhasindex etc), which might get changed in response totransactional changes that could still be rolled back. This isn't aproblem for VACUUM, since it can't be run inside a transaction block norin parallel with DDL on the table. However, we allow ANALYZE inside atransaction block, so if the transaction had earlier removed the lastindex, rule, or trigger from the table, and then we roll back thetransaction after ANALYZE, the table would be left in a corrupted statewith the hint flags not set though they should be.To fix, suppress the hint-flag updates if we are InTransactionBlock().This is safe enough because it's always OK to postpone hint maintenancesome more; the worst-case consequence is a few extra searches of pg_indexet al. There was discussion of instead using a transactional update,but that would change the behavior in ways that are not all desirable:in most scenarios we're better off keeping ANALYZE's statistical valueseven if the ANALYZE itself rolls back. In any case we probably don't wantto change this behavior in back branches.Per bug #11638 from Casey Shobe. This has been broken for a good longtime, so back-patch to all supported branches.Tom Lane and Michael Paquier, initial diagnosis by Andres Freund
1 parent6cb4aff commite0722d9

File tree

3 files changed

+81
-37
lines changed

3 files changed

+81
-37
lines changed

‎src/backend/commands/vacuum.c

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -648,23 +648,31 @@ vac_estimate_reltuples(Relation relation, bool is_analyze,
648648
*
649649
*We violate transaction semantics here by overwriting the rel's
650650
*existing pg_class tuple with the new values. This is reasonably
651-
*safe since the new values are correct whether or not this transaction
652-
*commits. The reason for this is that if we updated these tuples in
653-
*the usual way, vacuuming pg_class itself wouldn't work very well ---
654-
*by the time we got done with a vacuum cycle, most of the tuples in
655-
*pg_class would've been obsoleted. Of course, this only works for
656-
*fixed-size never-null columns, but these are.
657-
*
658-
*Note another assumption: that two VACUUMs/ANALYZEs on a table can't
659-
*run in parallel, nor can VACUUM/ANALYZE run in parallel with a
660-
*schema alteration such as adding an index, rule, or trigger. Otherwise
661-
*our updates of relhasindex etc might overwrite uncommitted updates.
651+
*safe as long as we're sure that the new values are correct whether or
652+
*not this transaction commits. The reason for doing this is that if
653+
*we updated these tuples in the usual way, vacuuming pg_class itself
654+
*wouldn't work very well --- by the time we got done with a vacuum
655+
*cycle, most of the tuples in pg_class would've been obsoleted. Of
656+
*course, this only works for fixed-size not-null columns, but these are.
662657
*
663658
*Another reason for doing it this way is that when we are in a lazy
664-
*VACUUM and have PROC_IN_VACUUM set, we mustn't do anyupdates ---
665-
*somebody vacuuming pg_class might think they could delete a tuple
659+
*VACUUM and have PROC_IN_VACUUM set, we mustn't do anyregular updates.
660+
*Somebody vacuuming pg_class might think they could delete a tuple
666661
*marked with xmin = our xid.
667662
*
663+
*In addition to fundamentally nontransactional statistics such as
664+
*relpages and relallvisible, we try to maintain certain lazily-updated
665+
*DDL flags such as relhasindex, by clearing them if no longer correct.
666+
*It's safe to do this in VACUUM, which can't run in parallel with
667+
*CREATE INDEX/RULE/TRIGGER and can't be part of a transaction block.
668+
*However, it's *not* safe to do it in an ANALYZE that's within a
669+
*transaction block, because for example the current transaction might
670+
*have dropped the last index; then we'd think relhasindex should be
671+
*cleared, but if the transaction later rolls back this would be wrong.
672+
*So we refrain from updating the DDL flags if we're inside a
673+
*transaction block. This is OK since postponing the flag maintenance
674+
*is always allowable.
675+
*
668676
*This routine is shared by VACUUM and ANALYZE.
669677
*/
670678
void
@@ -689,7 +697,7 @@ vac_update_relstats(Relation relation,
689697
relid);
690698
pgcform= (Form_pg_class)GETSTRUCT(ctup);
691699

692-
/* Applyrequired updates, if any, to copied tuple */
700+
/* Applystatistical updates, if any, to copied tuple */
693701

694702
dirty= false;
695703
if (pgcform->relpages!= (int32)num_pages)
@@ -707,32 +715,41 @@ vac_update_relstats(Relation relation,
707715
pgcform->relallvisible= (int32)num_all_visible_pages;
708716
dirty= true;
709717
}
710-
if (pgcform->relhasindex!=hasindex)
711-
{
712-
pgcform->relhasindex=hasindex;
713-
dirty= true;
714-
}
715718

716-
/*
717-
* If we have discovered that there are no indexes, then there's no
718-
* primary key either. This could be done more thoroughly...
719-
*/
720-
if (pgcform->relhaspkey&& !hasindex)
721-
{
722-
pgcform->relhaspkey= false;
723-
dirty= true;
724-
}
719+
/* Apply DDL updates, but not inside a transaction block (see above) */
725720

726-
/* We also clear relhasrules and relhastriggers if needed */
727-
if (pgcform->relhasrules&&relation->rd_rules==NULL)
721+
if (!IsTransactionBlock())
728722
{
729-
pgcform->relhasrules= false;
730-
dirty= true;
731-
}
732-
if (pgcform->relhastriggers&&relation->trigdesc==NULL)
733-
{
734-
pgcform->relhastriggers= false;
735-
dirty= true;
723+
/*
724+
* If we didn't find any indexes, reset relhasindex.
725+
*/
726+
if (pgcform->relhasindex&& !hasindex)
727+
{
728+
pgcform->relhasindex= false;
729+
dirty= true;
730+
}
731+
732+
/*
733+
* If we have discovered that there are no indexes, then there's no
734+
* primary key either. This could be done more thoroughly...
735+
*/
736+
if (pgcform->relhaspkey&& !hasindex)
737+
{
738+
pgcform->relhaspkey= false;
739+
dirty= true;
740+
}
741+
742+
/* We also clear relhasrules and relhastriggers if needed */
743+
if (pgcform->relhasrules&&relation->rd_rules==NULL)
744+
{
745+
pgcform->relhasrules= false;
746+
dirty= true;
747+
}
748+
if (pgcform->relhastriggers&&relation->trigdesc==NULL)
749+
{
750+
pgcform->relhastriggers= false;
751+
dirty= true;
752+
}
736753
}
737754

738755
/*

‎src/test/regress/expected/alter_table.out

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1811,6 +1811,23 @@ Check constraints:
18111811
"test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
18121812
Inherits: test_inh_check
18131813

1814+
-- check for rollback of ANALYZE corrupting table property flags (bug #11638)
1815+
CREATE TABLE check_fk_presence_1 (id int PRIMARY KEY, t text);
1816+
CREATE TABLE check_fk_presence_2 (id int REFERENCES check_fk_presence_1, t text);
1817+
BEGIN;
1818+
ALTER TABLE check_fk_presence_2 DROP CONSTRAINT check_fk_presence_2_id_fkey;
1819+
ANALYZE check_fk_presence_2;
1820+
ROLLBACK;
1821+
\d check_fk_presence_2
1822+
Table "public.check_fk_presence_2"
1823+
Column | Type | Modifiers
1824+
--------+---------+-----------
1825+
id | integer |
1826+
t | text |
1827+
Foreign-key constraints:
1828+
"check_fk_presence_2_id_fkey" FOREIGN KEY (id) REFERENCES check_fk_presence_1(id)
1829+
1830+
DROP TABLE check_fk_presence_1, check_fk_presence_2;
18141831
--
18151832
-- lock levels
18161833
--

‎src/test/regress/sql/alter_table.sql

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1254,6 +1254,16 @@ ALTER TABLE test_inh_check ALTER COLUMN a TYPE numeric;
12541254
\d test_inh_check
12551255
\d test_inh_check_child
12561256

1257+
-- check for rollback of ANALYZE corrupting table property flags (bug #11638)
1258+
CREATETABLEcheck_fk_presence_1 (idintPRIMARY KEY, ttext);
1259+
CREATETABLEcheck_fk_presence_2 (idintREFERENCES check_fk_presence_1, ttext);
1260+
BEGIN;
1261+
ALTERTABLE check_fk_presence_2 DROPCONSTRAINT check_fk_presence_2_id_fkey;
1262+
ANALYZE check_fk_presence_2;
1263+
ROLLBACK;
1264+
\d check_fk_presence_2
1265+
DROPTABLE check_fk_presence_1, check_fk_presence_2;
1266+
12571267
--
12581268
-- lock levels
12591269
--

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp