Test IsInTransactionChain, not IsTransactionBlock, in vac_update_relstats.

As noted by Noah Misch, my initial cut at fixing bug #11638 didn't cover
all cases where ANALYZE might be invoked in an unsafe context.  We need to
test the result of IsInTransactionChain not IsTransactionBlock; which is
notationally a pain because IsInTransactionChain requires an isTopLevel
flag, which would have to be passed down through several levels of callers.
I chose to pass in_outer_xact (ie, the result of IsInTransactionChain)
rather than isTopLevel per se, as that seemed marginally more apropos
for the intermediate functions to know about.
This commit is contained in:
Tom Lane 2014-10-30 13:03:22 -04:00
parent 6057c212f3
commit fd0f651a86
4 changed files with 31 additions and 20 deletions

View File

@ -87,7 +87,7 @@ static BufferAccessStrategy vac_strategy;
static void do_analyze_rel(Relation onerel, VacuumStmt *vacstmt, static void do_analyze_rel(Relation onerel, VacuumStmt *vacstmt,
AcquireSampleRowsFunc acquirefunc, BlockNumber relpages, AcquireSampleRowsFunc acquirefunc, BlockNumber relpages,
bool inh, int elevel); bool inh, bool in_outer_xact, int elevel);
static void BlockSampler_Init(BlockSampler bs, BlockNumber nblocks, static void BlockSampler_Init(BlockSampler bs, BlockNumber nblocks,
int samplesize); int samplesize);
static bool BlockSampler_HasMore(BlockSampler bs); static bool BlockSampler_HasMore(BlockSampler bs);
@ -115,7 +115,8 @@ static Datum ind_fetch_func(VacAttrStatsP stats, int rownum, bool *isNull);
* analyze_rel() -- analyze one relation * analyze_rel() -- analyze one relation
*/ */
void void
analyze_rel(Oid relid, VacuumStmt *vacstmt, BufferAccessStrategy bstrategy) analyze_rel(Oid relid, VacuumStmt *vacstmt,
bool in_outer_xact, BufferAccessStrategy bstrategy)
{ {
Relation onerel; Relation onerel;
int elevel; int elevel;
@ -265,13 +266,15 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt, BufferAccessStrategy bstrategy)
/* /*
* Do the normal non-recursive ANALYZE. * Do the normal non-recursive ANALYZE.
*/ */
do_analyze_rel(onerel, vacstmt, acquirefunc, relpages, false, elevel); do_analyze_rel(onerel, vacstmt, acquirefunc, relpages,
false, in_outer_xact, elevel);
/* /*
* If there are child tables, do recursive ANALYZE. * If there are child tables, do recursive ANALYZE.
*/ */
if (onerel->rd_rel->relhassubclass) if (onerel->rd_rel->relhassubclass)
do_analyze_rel(onerel, vacstmt, acquirefunc, relpages, true, elevel); do_analyze_rel(onerel, vacstmt, acquirefunc, relpages,
true, in_outer_xact, elevel);
/* /*
* Close source relation now, but keep lock so that no one deletes it * Close source relation now, but keep lock so that no one deletes it
@ -301,7 +304,7 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt, BufferAccessStrategy bstrategy)
static void static void
do_analyze_rel(Relation onerel, VacuumStmt *vacstmt, do_analyze_rel(Relation onerel, VacuumStmt *vacstmt,
AcquireSampleRowsFunc acquirefunc, BlockNumber relpages, AcquireSampleRowsFunc acquirefunc, BlockNumber relpages,
bool inh, int elevel) bool inh, bool in_outer_xact, int elevel)
{ {
int attr_cnt, int attr_cnt,
tcnt, tcnt,
@ -584,7 +587,8 @@ do_analyze_rel(Relation onerel, VacuumStmt *vacstmt,
visibilitymap_count(onerel), visibilitymap_count(onerel),
hasindex, hasindex,
InvalidTransactionId, InvalidTransactionId,
InvalidMultiXactId); InvalidMultiXactId,
in_outer_xact);
/* /*
* Same for indexes. Vacuum always scans all indexes, so if we're part of * Same for indexes. Vacuum always scans all indexes, so if we're part of
@ -605,7 +609,8 @@ do_analyze_rel(Relation onerel, VacuumStmt *vacstmt,
0, 0,
false, false,
InvalidTransactionId, InvalidTransactionId,
InvalidMultiXactId); InvalidMultiXactId,
in_outer_xact);
} }
} }

View File

@ -206,6 +206,8 @@ vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast,
*/ */
if (use_own_xacts) if (use_own_xacts)
{ {
Assert(!in_outer_xact);
/* ActiveSnapshot is not set by autovacuum */ /* ActiveSnapshot is not set by autovacuum */
if (ActiveSnapshotSet()) if (ActiveSnapshotSet())
PopActiveSnapshot(); PopActiveSnapshot();
@ -251,7 +253,7 @@ vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast,
PushActiveSnapshot(GetTransactionSnapshot()); PushActiveSnapshot(GetTransactionSnapshot());
} }
analyze_rel(relid, vacstmt, vac_strategy); analyze_rel(relid, vacstmt, in_outer_xact, vac_strategy);
if (use_own_xacts) if (use_own_xacts)
{ {
@ -665,13 +667,13 @@ vac_estimate_reltuples(Relation relation, bool is_analyze,
* DDL flags such as relhasindex, by clearing them if no longer correct. * DDL flags such as relhasindex, by clearing them if no longer correct.
* It's safe to do this in VACUUM, which can't run in parallel with * It's safe to do this in VACUUM, which can't run in parallel with
* CREATE INDEX/RULE/TRIGGER and can't be part of a transaction block. * CREATE INDEX/RULE/TRIGGER and can't be part of a transaction block.
* However, it's *not* safe to do it in an ANALYZE that's within a * However, it's *not* safe to do it in an ANALYZE that's within an
* transaction block, because for example the current transaction might * outer transaction, because for example the current transaction might
* have dropped the last index; then we'd think relhasindex should be * have dropped the last index; then we'd think relhasindex should be
* cleared, but if the transaction later rolls back this would be wrong. * cleared, but if the transaction later rolls back this would be wrong.
* So we refrain from updating the DDL flags if we're inside a * So we refrain from updating the DDL flags if we're inside an outer
* transaction block. This is OK since postponing the flag maintenance * transaction. This is OK since postponing the flag maintenance is
* is always allowable. * always allowable.
* *
* This routine is shared by VACUUM and ANALYZE. * This routine is shared by VACUUM and ANALYZE.
*/ */
@ -680,7 +682,8 @@ vac_update_relstats(Relation relation,
BlockNumber num_pages, double num_tuples, BlockNumber num_pages, double num_tuples,
BlockNumber num_all_visible_pages, BlockNumber num_all_visible_pages,
bool hasindex, TransactionId frozenxid, bool hasindex, TransactionId frozenxid,
MultiXactId minmulti) MultiXactId minmulti,
bool in_outer_xact)
{ {
Oid relid = RelationGetRelid(relation); Oid relid = RelationGetRelid(relation);
Relation rd; Relation rd;
@ -716,9 +719,9 @@ vac_update_relstats(Relation relation,
dirty = true; dirty = true;
} }
/* Apply DDL updates, but not inside a transaction block (see above) */ /* Apply DDL updates, but not inside an outer transaction (see above) */
if (!IsTransactionBlock()) if (!in_outer_xact)
{ {
/* /*
* If we didn't find any indexes, reset relhasindex. * If we didn't find any indexes, reset relhasindex.

View File

@ -309,7 +309,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
new_rel_allvisible, new_rel_allvisible,
vacrelstats->hasindex, vacrelstats->hasindex,
new_frozen_xid, new_frozen_xid,
new_min_multi); new_min_multi,
false);
/* report results to the stats collector, too */ /* report results to the stats collector, too */
new_live_tuples = new_rel_tuples - vacrelstats->new_dead_tuples; new_live_tuples = new_rel_tuples - vacrelstats->new_dead_tuples;
@ -1377,7 +1378,8 @@ lazy_cleanup_index(Relation indrel,
0, 0,
false, false,
InvalidTransactionId, InvalidTransactionId,
InvalidMultiXactId); InvalidMultiXactId,
false);
ereport(elevel, ereport(elevel,
(errmsg("index \"%s\" now contains %.0f row versions in %u pages", (errmsg("index \"%s\" now contains %.0f row versions in %u pages",

View File

@ -156,7 +156,8 @@ extern void vac_update_relstats(Relation relation,
BlockNumber num_all_visible_pages, BlockNumber num_all_visible_pages,
bool hasindex, bool hasindex,
TransactionId frozenxid, TransactionId frozenxid,
MultiXactId minmulti); MultiXactId minmulti,
bool in_outer_xact);
extern void vacuum_set_xid_limits(Relation rel, extern void vacuum_set_xid_limits(Relation rel,
int freeze_min_age, int freeze_table_age, int freeze_min_age, int freeze_table_age,
int multixact_freeze_min_age, int multixact_freeze_min_age,
@ -175,7 +176,7 @@ extern void lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
/* in commands/analyze.c */ /* in commands/analyze.c */
extern void analyze_rel(Oid relid, VacuumStmt *vacstmt, extern void analyze_rel(Oid relid, VacuumStmt *vacstmt,
BufferAccessStrategy bstrategy); bool in_outer_xact, BufferAccessStrategy bstrategy);
extern bool std_typanalyze(VacAttrStats *stats); extern bool std_typanalyze(VacAttrStats *stats);
extern double anl_random_fract(void); extern double anl_random_fract(void);
extern double anl_init_selection_state(int n); extern double anl_init_selection_state(int n);