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.

diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 38560d11273a6d659242f4c802ea55398ea8c639..801df26945d6185cfdc3dff8497cffdb6589e462 100644 (file)
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -86,7 +86,7 @@ static BufferAccessStrategy vac_strategy;
 
 static void do_analyze_rel(Relation onerel, VacuumStmt *vacstmt,
               AcquireSampleRowsFunc acquirefunc, BlockNumber relpages,
-              bool inh, int elevel);
+              bool inh, bool in_outer_xact, int elevel);
 static void BlockSampler_Init(BlockSampler bs, BlockNumber nblocks,
                  int samplesize);
 static bool BlockSampler_HasMore(BlockSampler bs);
@@ -114,7 +114,8 @@ static Datum ind_fetch_func(VacAttrStatsP stats, int rownum, bool *isNull);
  * analyze_rel() -- analyze one relation
  */
 void
-analyze_rel(Oid relid, VacuumStmt *vacstmt, BufferAccessStrategy bstrategy)
+analyze_rel(Oid relid, VacuumStmt *vacstmt,
+           bool in_outer_xact, BufferAccessStrategy bstrategy)
 {
    Relation    onerel;
    int         elevel;
@@ -264,13 +265,15 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt, BufferAccessStrategy bstrategy)
    /*
     * 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 (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
@@ -300,7 +303,7 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt, BufferAccessStrategy bstrategy)
 static void
 do_analyze_rel(Relation onerel, VacuumStmt *vacstmt,
               AcquireSampleRowsFunc acquirefunc, BlockNumber relpages,
-              bool inh, int elevel)
+              bool inh, bool in_outer_xact, int elevel)
 {
    int         attr_cnt,
                tcnt,
@@ -583,7 +586,8 @@ do_analyze_rel(Relation onerel, VacuumStmt *vacstmt,
                            visibilitymap_count(onerel),
                            hasindex,
                            InvalidTransactionId,
-                           InvalidMultiXactId);
+                           InvalidMultiXactId,
+                           in_outer_xact);
 
    /*
     * Same for indexes. Vacuum always scans all indexes, so if we're part of
@@ -604,7 +608,8 @@ do_analyze_rel(Relation onerel, VacuumStmt *vacstmt,
                                0,
                                false,
                                InvalidTransactionId,
-                               InvalidMultiXactId);
+                               InvalidMultiXactId,
+                               in_outer_xact);
        }
    }
 

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index f9697406a61d73510095947533c4bbb942de97c7..2a0003dd26d3498752a96758b79555d155556035 100644 (file)
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -206,6 +206,8 @@ vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast,
     */
    if (use_own_xacts)
    {
+       Assert(!in_outer_xact);
+
        /* ActiveSnapshot is not set by autovacuum */
        if (ActiveSnapshotSet())
            PopActiveSnapshot();
@@ -251,7 +253,7 @@ vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast,
                    PushActiveSnapshot(GetTransactionSnapshot());
                }
 
-               analyze_rel(relid, vacstmt, vac_strategy);
+               analyze_rel(relid, vacstmt, in_outer_xact, vac_strategy);
 
                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.
  *     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.
- *     However, it's *not* safe to do it in an ANALYZE that's within a
- *     transaction block, because for example the current transaction might
+ *     However, it's *not* safe to do it in an ANALYZE that's within an
+ *     outer transaction, because for example the current transaction might
  *     have dropped the last index; then we'd think relhasindex should be
  *     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
- *     transaction block.  This is OK since postponing the flag maintenance
- *     is always allowable.
+ *     So we refrain from updating the DDL flags if we're inside an outer
+ *     transaction.  This is OK since postponing the flag maintenance is
+ *     always allowable.
  *
  *     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_all_visible_pages,
                    bool hasindex, TransactionId frozenxid,
-                   MultiXactId minmulti)
+                   MultiXactId minmulti,
+                   bool in_outer_xact)
 {
    Oid         relid = RelationGetRelid(relation);
    Relation    rd;
@@ -716,9 +719,9 @@ vac_update_relstats(Relation relation,
        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.

diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index a7853a9f1071c4174d077c27cdd6b24a45403512..93a97a922d0ce99b42206bf6bec9aaa222047ca9 100644 (file)
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -306,7 +306,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
                        new_rel_allvisible,
                        vacrelstats->hasindex,
                        new_frozen_xid,
-                       new_min_multi);
+                       new_min_multi,
+                       false);
 
    /* report results to the stats collector, too */
    pgstat_report_vacuum(RelationGetRelid(onerel),
@@ -1370,7 +1371,8 @@ lazy_cleanup_index(Relation indrel,
                            0,
                            false,
                            InvalidTransactionId,
-                           InvalidMultiXactId);
+                           InvalidMultiXactId,
+                           false);
 
    ereport(elevel,
            (errmsg("index \"%s\" now contains %.0f row versions in %u pages",

diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 0815368902c7af01e1200aa37f475040dfda5809..d6139a6b4ec4132ca0294b21b54f1d66da119ea4 100644 (file)
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -156,7 +156,8 @@ extern void vac_update_relstats(Relation relation,
                    BlockNumber num_all_visible_pages,
                    bool hasindex,
                    TransactionId frozenxid,
-                   MultiXactId minmulti);
+                   MultiXactId minmulti,
+                   bool in_outer_xact);
 extern void vacuum_set_xid_limits(int freeze_min_age, int freeze_table_age,
                      int multixact_freeze_min_age,
                      int multixact_freeze_table_age,
@@ -175,7 +176,7 @@ extern void lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
 
 /* in commands/analyze.c */
 extern void analyze_rel(Oid relid, VacuumStmt *vacstmt,
-           BufferAccessStrategy bstrategy);
+           bool in_outer_xact, BufferAccessStrategy bstrategy);
 extern bool std_typanalyze(VacAttrStats *stats);
 extern double anl_random_fract(void);
 extern double anl_init_selection_state(int n);