Do not use in-place updates for statistics import.

The use of in-place updates was originally there to follow the
precedent of ANALYZE and to reduce the potential for bloat on
pg_class. Per discussion, it's not worth the risks.

Reported-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/cpdanvzykcb5o64rmapkx6n5gjypoce3y52hff7ocxupgpbxu4@53jmlyvukijo

diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c
index 1f6c5c39121c9d761669eb7fd8d53f607ac077ed..66731290a3e6beb226758489a44c4362f048d9be 100644 (file)
--- a/src/backend/statistics/relation_stats.c
+++ b/src/backend/statistics/relation_stats.c
@@ -48,14 +48,13 @@ static struct StatsArgInfo relarginfo[] =
        [NUM_RELATION_STATS_ARGS] = {0}
 };
 
-static bool relation_statistics_update(FunctionCallInfo fcinfo, int elevel,
-                                                                          bool inplace);
+static bool relation_statistics_update(FunctionCallInfo fcinfo, int elevel);
 
 /*
  * Internal function for modifying statistics for a relation.
  */
 static bool
-relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
+relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
 {
        bool            result = true;
        Oid                     reloid;
@@ -66,6 +65,12 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
        bool            update_reltuples = false;
        BlockNumber relallvisible = 0;
        bool            update_relallvisible = false;
+       HeapTuple       ctup;
+       Form_pg_class pgcform;
+       int                     replaces[3] = {0};
+       Datum           values[3] = {0};
+       bool            nulls[3] = {0};
+       int                     nreplaces = 0;
 
        if (!PG_ARGISNULL(RELPAGES_ARG))
        {
@@ -110,101 +115,52 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
         */
        crel = table_open(RelationRelationId, RowExclusiveLock);
 
-       if (inplace)
+       ctup = SearchSysCache1(RELOID, ObjectIdGetDatum(reloid));
+       if (!HeapTupleIsValid(ctup))
        {
-               HeapTuple       ctup = NULL;
-               ScanKeyData key[1];
-               Form_pg_class pgcform;
-               void       *inplace_state = NULL;
-               bool            dirty = false;
-
-               ScanKeyInit(&key[0], Anum_pg_class_oid, BTEqualStrategyNumber, F_OIDEQ,
-                                       ObjectIdGetDatum(reloid));
-               systable_inplace_update_begin(crel, ClassOidIndexId, true, NULL, 1, key,
-                                                                         &ctup, &inplace_state);
-               if (!HeapTupleIsValid(ctup))
-                       elog(ERROR, "pg_class entry for relid %u vanished while updating statistics",
-                                reloid);
-               pgcform = (Form_pg_class) GETSTRUCT(ctup);
-
-               if (update_relpages && pgcform->relpages != relpages)
-               {
-                       pgcform->relpages = relpages;
-                       dirty = true;
-               }
-               if (update_reltuples && pgcform->reltuples != reltuples)
-               {
-                       pgcform->reltuples = reltuples;
-                       dirty = true;
-               }
-               if (update_relallvisible && pgcform->relallvisible != relallvisible)
-               {
-                       pgcform->relallvisible = relallvisible;
-                       dirty = true;
-               }
-
-               if (dirty)
-                       systable_inplace_update_finish(inplace_state, ctup);
-               else
-                       systable_inplace_update_cancel(inplace_state);
-
-               heap_freetuple(ctup);
+               ereport(elevel,
+                               (errcode(ERRCODE_OBJECT_IN_USE),
+                                errmsg("pg_class entry for relid %u not found", reloid)));
+               table_close(crel, RowExclusiveLock);
+               return false;
        }
-       else
-       {
-               TupleDesc       tupdesc = RelationGetDescr(crel);
-               HeapTuple       ctup;
-               Form_pg_class pgcform;
-               int                     replaces[3] = {0};
-               Datum           values[3] = {0};
-               bool            nulls[3] = {0};
-               int                     nreplaces = 0;
-
-               ctup = SearchSysCache1(RELOID, ObjectIdGetDatum(reloid));
-               if (!HeapTupleIsValid(ctup))
-               {
-                       ereport(elevel,
-                                       (errcode(ERRCODE_OBJECT_IN_USE),
-                                        errmsg("pg_class entry for relid %u not found", reloid)));
-                       table_close(crel, RowExclusiveLock);
-                       return false;
-               }
-               pgcform = (Form_pg_class) GETSTRUCT(ctup);
 
-               if (update_relpages && relpages != pgcform->relpages)
-               {
-                       replaces[nreplaces] = Anum_pg_class_relpages;
-                       values[nreplaces] = UInt32GetDatum(relpages);
-                       nreplaces++;
-               }
+       pgcform = (Form_pg_class) GETSTRUCT(ctup);
 
-               if (update_reltuples && reltuples != pgcform->reltuples)
-               {
-                       replaces[nreplaces] = Anum_pg_class_reltuples;
-                       values[nreplaces] = Float4GetDatum(reltuples);
-                       nreplaces++;
-               }
+       if (update_relpages && relpages != pgcform->relpages)
+       {
+               replaces[nreplaces] = Anum_pg_class_relpages;
+               values[nreplaces] = UInt32GetDatum(relpages);
+               nreplaces++;
+       }
 
-               if (update_relallvisible && relallvisible != pgcform->relallvisible)
-               {
-                       replaces[nreplaces] = Anum_pg_class_relallvisible;
-                       values[nreplaces] = UInt32GetDatum(relallvisible);
-                       nreplaces++;
-               }
+       if (update_reltuples && reltuples != pgcform->reltuples)
+       {
+               replaces[nreplaces] = Anum_pg_class_reltuples;
+               values[nreplaces] = Float4GetDatum(reltuples);
+               nreplaces++;
+       }
 
-               if (nreplaces > 0)
-               {
-                       HeapTuple       newtup;
+       if (update_relallvisible && relallvisible != pgcform->relallvisible)
+       {
+               replaces[nreplaces] = Anum_pg_class_relallvisible;
+               values[nreplaces] = UInt32GetDatum(relallvisible);
+               nreplaces++;
+       }
 
-                       newtup = heap_modify_tuple_by_cols(ctup, tupdesc, nreplaces,
-                                                                                          replaces, values, nulls);
-                       CatalogTupleUpdate(crel, &newtup->t_self, newtup);
-                       heap_freetuple(newtup);
-               }
+       if (nreplaces > 0)
+       {
+               TupleDesc       tupdesc = RelationGetDescr(crel);
+               HeapTuple       newtup;
 
-               ReleaseSysCache(ctup);
+               newtup = heap_modify_tuple_by_cols(ctup, tupdesc, nreplaces,
+                                                                                  replaces, values, nulls);
+               CatalogTupleUpdate(crel, &newtup->t_self, newtup);
+               heap_freetuple(newtup);
        }
 
+       ReleaseSysCache(ctup);
+
        /* release the lock, consistent with vac_update_relstats() */
        table_close(crel, RowExclusiveLock);
 
@@ -219,7 +175,7 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
 Datum
 pg_set_relation_stats(PG_FUNCTION_ARGS)
 {
-       relation_statistics_update(fcinfo, ERROR, false);
+       relation_statistics_update(fcinfo, ERROR);
        PG_RETURN_VOID();
 }
 
@@ -243,7 +199,7 @@ pg_clear_relation_stats(PG_FUNCTION_ARGS)
        newfcinfo->args[3].value = UInt32GetDatum(0);
        newfcinfo->args[3].isnull = false;
 
-       relation_statistics_update(newfcinfo, ERROR, false);
+       relation_statistics_update(newfcinfo, ERROR);
        PG_RETURN_VOID();
 }
 
@@ -261,7 +217,7 @@ pg_restore_relation_stats(PG_FUNCTION_ARGS)
                                                                                  relarginfo, WARNING))
                result = false;
 
-       if (!relation_statistics_update(positional_fcinfo, WARNING, true))
+       if (!relation_statistics_update(positional_fcinfo, WARNING))
                result = false;
 
        PG_RETURN_BOOL(result);

diff --git a/src/test/regress/expected/stats_import.out b/src/test/regress/expected/stats_import.out
index 0e8491131e3a048c6b0746848bf5cc803f92c5dd..d6713eacc2c61587449cadfdad4315b9f88c3e19 100644 (file)
--- a/src/test/regress/expected/stats_import.out
+++ b/src/test/regress/expected/stats_import.out
@@ -143,39 +143,6 @@ WHERE oid = 'stats_import.test'::regclass;
        18 |       401 |             5
 (1 row)
 
--- test MVCC behavior: changes do not persist after abort (in contrast
--- to pg_restore_relation_stats(), which uses in-place updates).
-BEGIN;
-SELECT
-    pg_catalog.pg_set_relation_stats(
-        relation => 'stats_import.test'::regclass,
-        relpages => NULL::integer,
-        reltuples => 4000.0::real,
-        relallvisible => 4::integer);
- pg_set_relation_stats 
------------------------
- 
-(1 row)
-
-ABORT;
-SELECT relpages, reltuples, relallvisible
-FROM pg_class
-WHERE oid = 'stats_import.test'::regclass;
- relpages | reltuples | relallvisible 
-----------+-----------+---------------
-       18 |       401 |             5
-(1 row)
-
-BEGIN;
-SELECT
-    pg_catalog.pg_clear_relation_stats(
-        'stats_import.test'::regclass);
- pg_clear_relation_stats 
--------------------------
- 
-(1 row)
-
-ABORT;
 SELECT relpages, reltuples, relallvisible
 FROM pg_class
 WHERE oid = 'stats_import.test'::regclass;
@@ -836,25 +803,6 @@ WHERE oid = 'stats_import.test'::regclass;
 (1 row)
 
 -- ok: just relpages
-SELECT pg_restore_relation_stats(
-        'relation', 'stats_import.test'::regclass,
-        'version', 150000::integer,
-        'relpages', '15'::integer);
- pg_restore_relation_stats 
----------------------------
- t
-(1 row)
-
-SELECT relpages, reltuples, relallvisible
-FROM pg_class
-WHERE oid = 'stats_import.test'::regclass;
- relpages | reltuples | relallvisible 
-----------+-----------+---------------
-       15 |       400 |             4
-(1 row)
-
--- test non-MVCC behavior: new value should persist after abort
-BEGIN;
 SELECT pg_restore_relation_stats(
         'relation', 'stats_import.test'::regclass,
         'version', 150000::integer,
@@ -864,7 +812,6 @@ SELECT pg_restore_relation_stats(
  t
 (1 row)
 
-ABORT;
 SELECT relpages, reltuples, relallvisible
 FROM pg_class
 WHERE oid = 'stats_import.test'::regclass;

diff --git a/src/test/regress/sql/stats_import.sql b/src/test/regress/sql/stats_import.sql
index 5e24c779d80071c1f4efc2ae2d0b18e0f16500f5..9740ab3ff0254fc1833def732474611335c3f1ca 100644 (file)
--- a/src/test/regress/sql/stats_import.sql
+++ b/src/test/regress/sql/stats_import.sql
@@ -100,27 +100,6 @@ SELECT relpages, reltuples, relallvisible
 FROM pg_class
 WHERE oid = 'stats_import.test'::regclass;
 
--- test MVCC behavior: changes do not persist after abort (in contrast
--- to pg_restore_relation_stats(), which uses in-place updates).
-BEGIN;
-SELECT
-    pg_catalog.pg_set_relation_stats(
-        relation => 'stats_import.test'::regclass,
-        relpages => NULL::integer,
-        reltuples => 4000.0::real,
-        relallvisible => 4::integer);
-ABORT;
-
-SELECT relpages, reltuples, relallvisible
-FROM pg_class
-WHERE oid = 'stats_import.test'::regclass;
-
-BEGIN;
-SELECT
-    pg_catalog.pg_clear_relation_stats(
-        'stats_import.test'::regclass);
-ABORT;
-
 SELECT relpages, reltuples, relallvisible
 FROM pg_class
 WHERE oid = 'stats_import.test'::regclass;
@@ -649,22 +628,10 @@ FROM pg_class
 WHERE oid = 'stats_import.test'::regclass;
 
 -- ok: just relpages
-SELECT pg_restore_relation_stats(
-        'relation', 'stats_import.test'::regclass,
-        'version', 150000::integer,
-        'relpages', '15'::integer);
-
-SELECT relpages, reltuples, relallvisible
-FROM pg_class
-WHERE oid = 'stats_import.test'::regclass;
-
--- test non-MVCC behavior: new value should persist after abort
-BEGIN;
 SELECT pg_restore_relation_stats(
         'relation', 'stats_import.test'::regclass,
         'version', 150000::integer,
         'relpages', '16'::integer);
-ABORT;
 
 SELECT relpages, reltuples, relallvisible
 FROM pg_class