Use in-place updates for pg_restore_relation_stats().

This matches the behavior of vac_update_relstats(), which is important
to avoid bloating pg_class.

Author: Corey Huinker
Discussion: https://postgr.es/m/CADkLM=fc3je+ufv3gsHqjjSSf+t8674RXpuXW62EL55MUEQd-g@mail.gmail.com

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 8b81106fa234b68fbff227152010a7e7a1df02ae..2c35252dc06936c05e9c50a2925fb1c424e91a1b 100644 (file)
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -30175,6 +30175,14 @@ postgres=# SELECT '0/0'::pg_lsn + pd.segment_number * ps.setting::int + :offset
          function is to maintain a consistent function signature to avoid
          errors when restoring statistics from previous versions.
         </para>
+        <para>
+         To match the behavior of <xref linkend="sql-vacuum"/> and <xref
+         linkend="sql-analyze"/> when updating relation statistics,
+         <function>pg_restore_relation_stats()</function> does not follow MVCC
+         transactional semantics (see <xref linkend="mvcc"/>). New relation
+         statistics may be durable even if the transaction aborts, and the
+         changes are not isolated from other transactions.
+        </para>
         <para>
          Arguments are passed as pairs of <replaceable>argname</replaceable>
          and <replaceable>argvalue</replaceable>, where

diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c
index e619d5cf5b19aa96e6ddd2cfdeef7d180fe8d217..264a224a4d8861645858e7fcc7f33e0a19090788 100644 (file)
--- a/src/backend/statistics/relation_stats.c
+++ b/src/backend/statistics/relation_stats.c
@@ -20,6 +20,7 @@
 #include "access/heapam.h"
 #include "catalog/indexing.h"
 #include "statistics/stat_utils.h"
+#include "utils/fmgroids.h"
 #include "utils/fmgrprotos.h"
 #include "utils/syscache.h"
 
@@ -50,59 +51,28 @@ static struct StatsArgInfo relarginfo[] =
    [NUM_RELATION_STATS_ARGS] = {0}
 };
 
-static bool relation_statistics_update(FunctionCallInfo fcinfo, int elevel);
+static bool relation_statistics_update(FunctionCallInfo fcinfo, int elevel,
+                                      bool inplace);
 
 /*
  * Internal function for modifying statistics for a relation.
  */
 static bool
-relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
+relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace)
 {
    Oid         reloid;
    Relation    crel;
-   HeapTuple   ctup;
-   Form_pg_class pgcform;
-   int         replaces[3] = {0};
-   Datum       values[3] = {0};
-   bool        nulls[3] = {0};
-   int         ncols = 0;
-   TupleDesc   tupdesc;
+   int32       relpages = DEFAULT_RELPAGES;
+   bool        update_relpages = false;
+   float       reltuples = DEFAULT_RELTUPLES;
+   bool        update_reltuples = false;
+   int32       relallvisible = DEFAULT_RELALLVISIBLE;
+   bool        update_relallvisible = false;
    bool        result = true;
 
-   stats_check_required_arg(fcinfo, relarginfo, RELATION_ARG);
-   reloid = PG_GETARG_OID(RELATION_ARG);
-
-   if (RecoveryInProgress())
-       ereport(ERROR,
-               (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                errmsg("recovery is in progress"),
-                errhint("Statistics cannot be modified during recovery.")));
-
-   stats_lock_check_privileges(reloid);
-
-   /*
-    * Take RowExclusiveLock on pg_class, consistent with
-    * vac_update_relstats().
-    */
-   crel = table_open(RelationRelationId, RowExclusiveLock);
-
-   tupdesc = RelationGetDescr(crel);
-   ctup = SearchSysCacheCopy1(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);
-
-   /* relpages */
    if (!PG_ARGISNULL(RELPAGES_ARG))
    {
-       int32       relpages = PG_GETARG_INT32(RELPAGES_ARG);
+       relpages = PG_GETARG_INT32(RELPAGES_ARG);
 
        /*
         * Partitioned tables may have relpages=-1. Note: for relations with
@@ -116,17 +86,13 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
                     errmsg("relpages cannot be < -1")));
            result = false;
        }
-       else if (relpages != pgcform->relpages)
-       {
-           replaces[ncols] = Anum_pg_class_relpages;
-           values[ncols] = Int32GetDatum(relpages);
-           ncols++;
-       }
+       else
+           update_relpages = true;
    }
 
    if (!PG_ARGISNULL(RELTUPLES_ARG))
    {
-       float       reltuples = PG_GETARG_FLOAT4(RELTUPLES_ARG);
+       reltuples = PG_GETARG_FLOAT4(RELTUPLES_ARG);
 
        if (reltuples < -1.0)
        {
@@ -135,18 +101,13 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
                     errmsg("reltuples cannot be < -1.0")));
            result = false;
        }
-       else if (reltuples != pgcform->reltuples)
-       {
-           replaces[ncols] = Anum_pg_class_reltuples;
-           values[ncols] = Float4GetDatum(reltuples);
-           ncols++;
-       }
-
+       else
+           update_reltuples = true;
    }
 
    if (!PG_ARGISNULL(RELALLVISIBLE_ARG))
    {
-       int32       relallvisible = PG_GETARG_INT32(RELALLVISIBLE_ARG);
+       relallvisible = PG_GETARG_INT32(RELALLVISIBLE_ARG);
 
        if (relallvisible < 0)
        {
@@ -155,23 +116,120 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
                     errmsg("relallvisible cannot be < 0")));
            result = false;
        }
-       else if (relallvisible != pgcform->relallvisible)
+       else
+           update_relallvisible = true;
+   }
+
+   stats_check_required_arg(fcinfo, relarginfo, RELATION_ARG);
+   reloid = PG_GETARG_OID(RELATION_ARG);
+
+   if (RecoveryInProgress())
+       ereport(ERROR,
+               (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                errmsg("recovery is in progress"),
+                errhint("Statistics cannot be modified during recovery.")));
+
+   stats_lock_check_privileges(reloid);
+
+   /*
+    * Take RowExclusiveLock on pg_class, consistent with
+    * vac_update_relstats().
+    */
+   crel = table_open(RelationRelationId, RowExclusiveLock);
+
+   if (inplace)
+   {
+       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)
        {
-           replaces[ncols] = Anum_pg_class_relallvisible;
-           values[ncols] = Int32GetDatum(relallvisible);
-           ncols++;
+           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);
 
-   /* only update pg_class if there is a meaningful change */
-   if (ncols > 0)
+       heap_freetuple(ctup);
+   }
+   else
    {
-       HeapTuple   newtup;
+       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] = Int32GetDatum(relpages);
+           nreplaces++;
+       }
+
+       if (update_reltuples && reltuples != pgcform->reltuples)
+       {
+           replaces[nreplaces] = Anum_pg_class_reltuples;
+           values[nreplaces] = Float4GetDatum(reltuples);
+           nreplaces++;
+       }
+
+       if (update_relallvisible && relallvisible != pgcform->relallvisible)
+       {
+           replaces[nreplaces] = Anum_pg_class_relallvisible;
+           values[nreplaces] = Int32GetDatum(relallvisible);
+           nreplaces++;
+       }
+
+       if (nreplaces > 0)
+       {
+           HeapTuple   newtup;
+
+           newtup = heap_modify_tuple_by_cols(ctup, tupdesc, nreplaces,
+                                              replaces, values, nulls);
+           CatalogTupleUpdate(crel, &newtup->t_self, newtup);
+           heap_freetuple(newtup);
+       }
 
-       newtup = heap_modify_tuple_by_cols(ctup, tupdesc, ncols, replaces, values,
-                                          nulls);
-       CatalogTupleUpdate(crel, &newtup->t_self, newtup);
-       heap_freetuple(newtup);
+       ReleaseSysCache(ctup);
    }
 
    /* release the lock, consistent with vac_update_relstats() */
@@ -188,7 +246,7 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
 Datum
 pg_set_relation_stats(PG_FUNCTION_ARGS)
 {
-   relation_statistics_update(fcinfo, ERROR);
+   relation_statistics_update(fcinfo, ERROR, false);
    PG_RETURN_VOID();
 }
 
@@ -212,7 +270,7 @@ pg_clear_relation_stats(PG_FUNCTION_ARGS)
    newfcinfo->args[3].value = DEFAULT_RELALLVISIBLE;
    newfcinfo->args[3].isnull = false;
 
-   relation_statistics_update(newfcinfo, ERROR);
+   relation_statistics_update(newfcinfo, ERROR, false);
    PG_RETURN_VOID();
 }
 
@@ -230,7 +288,7 @@ pg_restore_relation_stats(PG_FUNCTION_ARGS)
                                          relarginfo, WARNING))
        result = false;
 
-   if (!relation_statistics_update(positional_fcinfo, WARNING))
+   if (!relation_statistics_update(positional_fcinfo, WARNING, true))
        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 aab862c97c717ebbeaf10e68ad8891082ec51680..fb50da1cd839688f1f5fceb871c0930d3c987da3 100644 (file)
--- a/src/test/regress/expected/stats_import.out
+++ b/src/test/regress/expected/stats_import.out
@@ -105,6 +105,47 @@ 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;
+ relpages | reltuples | relallvisible 
+----------+-----------+---------------
+       18 |       401 |             5
+(1 row)
+
 -- clear
 SELECT
     pg_catalog.pg_clear_relation_stats(
@@ -705,6 +746,25 @@ 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,
@@ -714,6 +774,7 @@ 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 31455b58c1dac7f3b254ec2e87b2e73af6d00c43..d3058bf8f6b40ba0d88779aa2e8f131ad95ed1a0 100644 (file)
--- a/src/test/regress/sql/stats_import.sql
+++ b/src/test/regress/sql/stats_import.sql
@@ -76,6 +76,31 @@ 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;
+
 -- clear
 SELECT
     pg_catalog.pg_clear_relation_stats(
@@ -565,10 +590,22 @@ 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