mirror of
https://github.com/postgres/postgres.git
synced 2025-07-18 17:42:25 +03:00
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:
@ -82,7 +82,8 @@ static MemoryContext anl_context = NULL;
|
|||||||
static BufferAccessStrategy vac_strategy;
|
static BufferAccessStrategy vac_strategy;
|
||||||
|
|
||||||
|
|
||||||
static void do_analyze_rel(Relation onerel, VacuumStmt *vacstmt, bool inh);
|
static void do_analyze_rel(Relation onerel, VacuumStmt *vacstmt,
|
||||||
|
bool inh, bool in_outer_xact);
|
||||||
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);
|
||||||
@ -114,7 +115,8 @@ static bool std_typanalyze(VacAttrStats *stats);
|
|||||||
* 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;
|
||||||
|
|
||||||
@ -214,13 +216,13 @@ 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, false);
|
do_analyze_rel(onerel, vacstmt, false, in_outer_xact);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* 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, true);
|
do_analyze_rel(onerel, vacstmt, true, in_outer_xact);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* 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
|
||||||
@ -243,7 +245,8 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt, BufferAccessStrategy bstrategy)
|
|||||||
* do_analyze_rel() -- analyze one relation, recursively or not
|
* do_analyze_rel() -- analyze one relation, recursively or not
|
||||||
*/
|
*/
|
||||||
static void
|
static void
|
||||||
do_analyze_rel(Relation onerel, VacuumStmt *vacstmt, bool inh)
|
do_analyze_rel(Relation onerel, VacuumStmt *vacstmt,
|
||||||
|
bool inh, bool in_outer_xact)
|
||||||
{
|
{
|
||||||
int attr_cnt,
|
int attr_cnt,
|
||||||
tcnt,
|
tcnt,
|
||||||
@ -520,7 +523,10 @@ do_analyze_rel(Relation onerel, VacuumStmt *vacstmt, bool inh)
|
|||||||
if (!inh)
|
if (!inh)
|
||||||
vac_update_relstats(onerel,
|
vac_update_relstats(onerel,
|
||||||
RelationGetNumberOfBlocks(onerel),
|
RelationGetNumberOfBlocks(onerel),
|
||||||
totalrows, hasindex, InvalidTransactionId);
|
totalrows,
|
||||||
|
hasindex,
|
||||||
|
InvalidTransactionId,
|
||||||
|
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
|
||||||
@ -537,7 +543,10 @@ do_analyze_rel(Relation onerel, VacuumStmt *vacstmt, bool inh)
|
|||||||
totalindexrows = ceil(thisdata->tupleFract * totalrows);
|
totalindexrows = ceil(thisdata->tupleFract * totalrows);
|
||||||
vac_update_relstats(Irel[ind],
|
vac_update_relstats(Irel[ind],
|
||||||
RelationGetNumberOfBlocks(Irel[ind]),
|
RelationGetNumberOfBlocks(Irel[ind]),
|
||||||
totalindexrows, false, InvalidTransactionId);
|
totalindexrows,
|
||||||
|
false,
|
||||||
|
InvalidTransactionId,
|
||||||
|
in_outer_xact);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -204,6 +204,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();
|
||||||
@ -243,7 +245,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)
|
||||||
{
|
{
|
||||||
@ -557,20 +559,21 @@ 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.
|
||||||
*/
|
*/
|
||||||
void
|
void
|
||||||
vac_update_relstats(Relation relation,
|
vac_update_relstats(Relation relation,
|
||||||
BlockNumber num_pages, double num_tuples,
|
BlockNumber num_pages, double num_tuples,
|
||||||
bool hasindex, TransactionId frozenxid)
|
bool hasindex, TransactionId frozenxid,
|
||||||
|
bool in_outer_xact)
|
||||||
{
|
{
|
||||||
Oid relid = RelationGetRelid(relation);
|
Oid relid = RelationGetRelid(relation);
|
||||||
Relation rd;
|
Relation rd;
|
||||||
@ -601,9 +604,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.
|
||||||
|
@ -266,7 +266,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
|
|||||||
vac_update_relstats(onerel,
|
vac_update_relstats(onerel,
|
||||||
new_rel_pages, new_rel_tuples,
|
new_rel_pages, new_rel_tuples,
|
||||||
vacrelstats->hasindex,
|
vacrelstats->hasindex,
|
||||||
new_frozen_xid);
|
new_frozen_xid,
|
||||||
|
false);
|
||||||
|
|
||||||
/* report results to the stats collector, too */
|
/* report results to the stats collector, too */
|
||||||
pgstat_report_vacuum(RelationGetRelid(onerel),
|
pgstat_report_vacuum(RelationGetRelid(onerel),
|
||||||
@ -1091,7 +1092,9 @@ lazy_cleanup_index(Relation indrel,
|
|||||||
if (!stats->estimated_count)
|
if (!stats->estimated_count)
|
||||||
vac_update_relstats(indrel,
|
vac_update_relstats(indrel,
|
||||||
stats->num_pages, stats->num_index_tuples,
|
stats->num_pages, stats->num_index_tuples,
|
||||||
false, InvalidTransactionId);
|
false,
|
||||||
|
InvalidTransactionId,
|
||||||
|
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",
|
||||||
|
@ -146,7 +146,8 @@ extern void vac_update_relstats(Relation relation,
|
|||||||
BlockNumber num_pages,
|
BlockNumber num_pages,
|
||||||
double num_tuples,
|
double num_tuples,
|
||||||
bool hasindex,
|
bool hasindex,
|
||||||
TransactionId frozenxid);
|
TransactionId frozenxid,
|
||||||
|
bool in_outer_xact);
|
||||||
extern void vacuum_set_xid_limits(int freeze_min_age, int freeze_table_age,
|
extern void vacuum_set_xid_limits(int freeze_min_age, int freeze_table_age,
|
||||||
bool sharedRel,
|
bool sharedRel,
|
||||||
TransactionId *oldestXmin,
|
TransactionId *oldestXmin,
|
||||||
@ -161,6 +162,6 @@ 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);
|
||||||
|
|
||||||
#endif /* VACUUM_H */
|
#endif /* VACUUM_H */
|
||||||
|
Reference in New Issue
Block a user