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

Commit9d06da5

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 parent49ef4eb commit9d06da5

File tree

3 files changed

+86
-41
lines changed

3 files changed

+86
-41
lines changed

‎src/backend/commands/vacuum.c

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

582-
/* Applyrequired updates, if any, to copied tuple */
590+
/* Applystatistical updates, if any, to copied tuple */
583591

584592
dirty= false;
585593
if (pgcform->relpages!= (int32)num_pages)
@@ -592,41 +600,50 @@ vac_update_relstats(Relation relation,
592600
pgcform->reltuples= (float4)num_tuples;
593601
dirty= true;
594602
}
595-
if (pgcform->relhasindex!=hasindex)
596-
{
597-
pgcform->relhasindex=hasindex;
598-
dirty= true;
599-
}
600603

601-
/*
602-
* If we have discovered that there are no indexes, then there's no
603-
* primary key either, nor any exclusion constraints. This could be done
604-
* more thoroughly...
605-
*/
606-
if (!hasindex)
604+
/* Apply DDL updates, but not inside a transaction block (see above) */
605+
606+
if (!IsTransactionBlock())
607607
{
608-
if (pgcform->relhaspkey)
608+
/*
609+
* If we didn't find any indexes, reset relhasindex.
610+
*/
611+
if (pgcform->relhasindex&& !hasindex)
609612
{
610-
pgcform->relhaspkey= false;
613+
pgcform->relhasindex= false;
611614
dirty= true;
612615
}
613-
if (pgcform->relhasexclusion&&pgcform->relkind!=RELKIND_INDEX)
616+
617+
/*
618+
* If we have discovered that there are no indexes, then there's no
619+
* primary key either, nor any exclusion constraints. This could be
620+
* done more thoroughly...
621+
*/
622+
if (!hasindex)
614623
{
615-
pgcform->relhasexclusion= false;
616-
dirty= true;
624+
if (pgcform->relhaspkey)
625+
{
626+
pgcform->relhaspkey= false;
627+
dirty= true;
628+
}
629+
if (pgcform->relhasexclusion&&pgcform->relkind!=RELKIND_INDEX)
630+
{
631+
pgcform->relhasexclusion= false;
632+
dirty= true;
633+
}
617634
}
618-
}
619635

620-
/* We also clear relhasrules and relhastriggers if needed */
621-
if (pgcform->relhasrules&&relation->rd_rules==NULL)
622-
{
623-
pgcform->relhasrules= false;
624-
dirty= true;
625-
}
626-
if (pgcform->relhastriggers&&relation->trigdesc==NULL)
627-
{
628-
pgcform->relhastriggers= false;
629-
dirty= true;
636+
/* We also clear relhasrules and relhastriggers if needed */
637+
if (pgcform->relhasrules&&relation->rd_rules==NULL)
638+
{
639+
pgcform->relhasrules= false;
640+
dirty= true;
641+
}
642+
if (pgcform->relhastriggers&&relation->trigdesc==NULL)
643+
{
644+
pgcform->relhastriggers= false;
645+
dirty= true;
646+
}
630647
}
631648

632649
/*

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1544,6 +1544,24 @@ select non_strict(NULL);
15441544

15451545
(1 row)
15461546

1547+
-- check for rollback of ANALYZE corrupting table property flags (bug #11638)
1548+
CREATE TABLE check_fk_presence_1 (id int PRIMARY KEY, t text);
1549+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "check_fk_presence_1_pkey" for table "check_fk_presence_1"
1550+
CREATE TABLE check_fk_presence_2 (id int REFERENCES check_fk_presence_1, t text);
1551+
BEGIN;
1552+
ALTER TABLE check_fk_presence_2 DROP CONSTRAINT check_fk_presence_2_id_fkey;
1553+
ANALYZE check_fk_presence_2;
1554+
ROLLBACK;
1555+
\d check_fk_presence_2
1556+
Table "public.check_fk_presence_2"
1557+
Column | Type | Modifiers
1558+
--------+---------+-----------
1559+
id | integer |
1560+
t | text |
1561+
Foreign-key constraints:
1562+
"check_fk_presence_2_id_fkey" FOREIGN KEY (id) REFERENCES check_fk_presence_1(id)
1563+
1564+
DROP TABLE check_fk_presence_1, check_fk_presence_2;
15471565
--
15481566
-- alter object set schema
15491567
--

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,6 +1124,16 @@ select non_strict(NULL);
11241124
alterfunction non_strict(text) returnsnullonnull input;
11251125
select non_strict(NULL);
11261126

1127+
-- check for rollback of ANALYZE corrupting table property flags (bug #11638)
1128+
CREATETABLEcheck_fk_presence_1 (idintPRIMARY KEY, ttext);
1129+
CREATETABLEcheck_fk_presence_2 (idintREFERENCES check_fk_presence_1, ttext);
1130+
BEGIN;
1131+
ALTERTABLE check_fk_presence_2 DROPCONSTRAINT check_fk_presence_2_id_fkey;
1132+
ANALYZE check_fk_presence_2;
1133+
ROLLBACK;
1134+
\d check_fk_presence_2
1135+
DROPTABLE check_fk_presence_1, check_fk_presence_2;
1136+
11271137
--
11281138
-- alter object set schema
11291139
--

0 commit comments

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