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

Commit6ec1c3e

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 parent8f7bd8e commit6ec1c3e

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

580-
/* Applyrequired updates, if any, to copied tuple */
588+
/* Applystatistical updates, if any, to copied tuple */
581589

582590
dirty= false;
583591
if (pgcform->relpages!= (int32)num_pages)
@@ -590,32 +598,41 @@ vac_update_relstats(Relation relation,
590598
pgcform->reltuples= (float4)num_tuples;
591599
dirty= true;
592600
}
593-
if (pgcform->relhasindex!=hasindex)
594-
{
595-
pgcform->relhasindex=hasindex;
596-
dirty= true;
597-
}
598601

599-
/*
600-
* If we have discovered that there are no indexes, then there's no
601-
* primary key either. This could be done more thoroughly...
602-
*/
603-
if (pgcform->relhaspkey&& !hasindex)
604-
{
605-
pgcform->relhaspkey= false;
606-
dirty= true;
607-
}
602+
/* Apply DDL updates, but not inside a transaction block (see above) */
608603

609-
/* We also clear relhasrules and relhastriggers if needed */
610-
if (pgcform->relhasrules&&relation->rd_rules==NULL)
604+
if (!IsTransactionBlock())
611605
{
612-
pgcform->relhasrules= false;
613-
dirty= true;
614-
}
615-
if (pgcform->relhastriggers&&relation->trigdesc==NULL)
616-
{
617-
pgcform->relhastriggers= false;
618-
dirty= true;
606+
/*
607+
* If we didn't find any indexes, reset relhasindex.
608+
*/
609+
if (pgcform->relhasindex&& !hasindex)
610+
{
611+
pgcform->relhasindex= false;
612+
dirty= true;
613+
}
614+
615+
/*
616+
* If we have discovered that there are no indexes, then there's no
617+
* primary key either. This could be done more thoroughly...
618+
*/
619+
if (pgcform->relhaspkey&& !hasindex)
620+
{
621+
pgcform->relhaspkey= false;
622+
dirty= true;
623+
}
624+
625+
/* We also clear relhasrules and relhastriggers if needed */
626+
if (pgcform->relhasrules&&relation->rd_rules==NULL)
627+
{
628+
pgcform->relhasrules= false;
629+
dirty= true;
630+
}
631+
if (pgcform->relhastriggers&&relation->trigdesc==NULL)
632+
{
633+
pgcform->relhastriggers= false;
634+
dirty= true;
635+
}
619636
}
620637

621638
/*

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

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

1562+
-- check for rollback of ANALYZE corrupting table property flags (bug #11638)
1563+
CREATE TABLE check_fk_presence_1 (id int PRIMARY KEY, t text);
1564+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "check_fk_presence_1_pkey" for table "check_fk_presence_1"
1565+
CREATE TABLE check_fk_presence_2 (id int REFERENCES check_fk_presence_1, t text);
1566+
BEGIN;
1567+
ALTER TABLE check_fk_presence_2 DROP CONSTRAINT check_fk_presence_2_id_fkey;
1568+
ANALYZE check_fk_presence_2;
1569+
ROLLBACK;
1570+
\d check_fk_presence_2
1571+
Table "public.check_fk_presence_2"
1572+
Column | Type | Modifiers
1573+
--------+---------+-----------
1574+
id | integer |
1575+
t | text |
1576+
Foreign-key constraints:
1577+
"check_fk_presence_2_id_fkey" FOREIGN KEY (id) REFERENCES check_fk_presence_1(id)
1578+
1579+
DROP TABLE check_fk_presence_1, check_fk_presence_2;
15621580
--
15631581
-- lock levels
15641582
--

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1151,6 +1151,16 @@ ALTER TABLE test_inh_check ALTER COLUMN a TYPE numeric;
11511151
\d test_inh_check
11521152
\d test_inh_check_child
11531153

1154+
-- check for rollback of ANALYZE corrupting table property flags (bug #11638)
1155+
CREATETABLEcheck_fk_presence_1 (idintPRIMARY KEY, ttext);
1156+
CREATETABLEcheck_fk_presence_2 (idintREFERENCES check_fk_presence_1, ttext);
1157+
BEGIN;
1158+
ALTERTABLE check_fk_presence_2 DROPCONSTRAINT check_fk_presence_2_id_fkey;
1159+
ANALYZE check_fk_presence_2;
1160+
ROLLBACK;
1161+
\d check_fk_presence_2
1162+
DROPTABLE check_fk_presence_1, check_fk_presence_2;
1163+
11541164
--
11551165
-- lock levels
11561166
--

0 commit comments

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