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

Commit40058fb

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 parent1a27fbd commit40058fb

File tree

3 files changed

+82
-37
lines changed

3 files changed

+82
-37
lines changed

‎src/backend/commands/vacuum.c‎

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -550,23 +550,31 @@ vac_estimate_reltuples(Relation relation, bool is_analyze,
550550
*
551551
*We violate transaction semantics here by overwriting the rel's
552552
*existing pg_class tuple with the new values. This is reasonably
553-
*safe since the new values are correct whether or not this transaction
554-
*commits. The reason for this is that if we updated these tuples in
555-
*the usual way, vacuuming pg_class itself wouldn't work very well ---
556-
*by the time we got done with a vacuum cycle, most of the tuples in
557-
*pg_class would've been obsoleted. Of course, this only works for
558-
*fixed-size never-null columns, but these are.
559-
*
560-
*Note another assumption: that two VACUUMs/ANALYZEs on a table can't
561-
*run in parallel, nor can VACUUM/ANALYZE run in parallel with a
562-
*schema alteration such as adding an index, rule, or trigger. Otherwise
563-
*our updates of relhasindex etc might overwrite uncommitted updates.
553+
*safe as long as we're sure that the new values are correct whether or
554+
*not this transaction commits. The reason for doing this is that if
555+
*we updated these tuples in the usual way, vacuuming pg_class itself
556+
*wouldn't work very well --- by the time we got done with a vacuum
557+
*cycle, most of the tuples in pg_class would've been obsoleted. Of
558+
*course, this only works for fixed-size not-null columns, but these are.
564559
*
565560
*Another reason for doing it this way is that when we are in a lazy
566-
*VACUUM and have PROC_IN_VACUUM set, we mustn't do anyupdates ---
567-
*somebody vacuuming pg_class might think they could delete a tuple
561+
*VACUUM and have PROC_IN_VACUUM set, we mustn't do anyregular updates.
562+
*Somebody vacuuming pg_class might think they could delete a tuple
568563
*marked with xmin = our xid.
569564
*
565+
*In addition to fundamentally nontransactional statistics such as
566+
*relpages and relallvisible, we try to maintain certain lazily-updated
567+
*DDL flags such as relhasindex, by clearing them if no longer correct.
568+
*It's safe to do this in VACUUM, which can't run in parallel with
569+
*CREATE INDEX/RULE/TRIGGER and can't be part of a transaction block.
570+
*However, it's *not* safe to do it in an ANALYZE that's within a
571+
*transaction block, because for example the current transaction might
572+
*have dropped the last index; then we'd think relhasindex should be
573+
*cleared, but if the transaction later rolls back this would be wrong.
574+
*So we refrain from updating the DDL flags if we're inside a
575+
*transaction block. This is OK since postponing the flag maintenance
576+
*is always allowable.
577+
*
570578
*This routine is shared by VACUUM and ANALYZE.
571579
*/
572580
void
@@ -590,7 +598,7 @@ vac_update_relstats(Relation relation,
590598
relid);
591599
pgcform= (Form_pg_class)GETSTRUCT(ctup);
592600

593-
/* Applyrequired updates, if any, to copied tuple */
601+
/* Applystatistical updates, if any, to copied tuple */
594602

595603
dirty= false;
596604
if (pgcform->relpages!= (int32)num_pages)
@@ -608,32 +616,41 @@ vac_update_relstats(Relation relation,
608616
pgcform->relallvisible= (int32)num_all_visible_pages;
609617
dirty= true;
610618
}
611-
if (pgcform->relhasindex!=hasindex)
612-
{
613-
pgcform->relhasindex=hasindex;
614-
dirty= true;
615-
}
616619

617-
/*
618-
* If we have discovered that there are no indexes, then there's no
619-
* primary key either. This could be done more thoroughly...
620-
*/
621-
if (pgcform->relhaspkey&& !hasindex)
622-
{
623-
pgcform->relhaspkey= false;
624-
dirty= true;
625-
}
620+
/* Apply DDL updates, but not inside a transaction block (see above) */
626621

627-
/* We also clear relhasrules and relhastriggers if needed */
628-
if (pgcform->relhasrules&&relation->rd_rules==NULL)
622+
if (!IsTransactionBlock())
629623
{
630-
pgcform->relhasrules= false;
631-
dirty= true;
632-
}
633-
if (pgcform->relhastriggers&&relation->trigdesc==NULL)
634-
{
635-
pgcform->relhastriggers= false;
636-
dirty= true;
624+
/*
625+
* If we didn't find any indexes, reset relhasindex.
626+
*/
627+
if (pgcform->relhasindex&& !hasindex)
628+
{
629+
pgcform->relhasindex= false;
630+
dirty= true;
631+
}
632+
633+
/*
634+
* If we have discovered that there are no indexes, then there's no
635+
* primary key either. This could be done more thoroughly...
636+
*/
637+
if (pgcform->relhaspkey&& !hasindex)
638+
{
639+
pgcform->relhaspkey= false;
640+
dirty= true;
641+
}
642+
643+
/* We also clear relhasrules and relhastriggers if needed */
644+
if (pgcform->relhasrules&&relation->rd_rules==NULL)
645+
{
646+
pgcform->relhasrules= false;
647+
dirty= true;
648+
}
649+
if (pgcform->relhastriggers&&relation->trigdesc==NULL)
650+
{
651+
pgcform->relhastriggers= false;
652+
dirty= true;
653+
}
637654
}
638655

639656
/*

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1841,6 +1841,24 @@ Check constraints:
18411841
"test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
18421842
Inherits: test_inh_check
18431843

1844+
-- check for rollback of ANALYZE corrupting table property flags (bug #11638)
1845+
CREATE TABLE check_fk_presence_1 (id int PRIMARY KEY, t text);
1846+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "check_fk_presence_1_pkey" for table "check_fk_presence_1"
1847+
CREATE TABLE check_fk_presence_2 (id int REFERENCES check_fk_presence_1, t text);
1848+
BEGIN;
1849+
ALTER TABLE check_fk_presence_2 DROP CONSTRAINT check_fk_presence_2_id_fkey;
1850+
ANALYZE check_fk_presence_2;
1851+
ROLLBACK;
1852+
\d check_fk_presence_2
1853+
Table "public.check_fk_presence_2"
1854+
Column | Type | Modifiers
1855+
--------+---------+-----------
1856+
id | integer |
1857+
t | text |
1858+
Foreign-key constraints:
1859+
"check_fk_presence_2_id_fkey" FOREIGN KEY (id) REFERENCES check_fk_presence_1(id)
1860+
1861+
DROP TABLE check_fk_presence_1, check_fk_presence_2;
18441862
--
18451863
-- lock levels
18461864
--

‎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