>
>
>
> Patch that allows relation_statistics_update to continue after one failed
> stat (0001) attached, along with bool->void change (0002).
>

Once more, with feeling:
From 0cf8a90c37ec95bbbefb274bc3d48a077b4868b8 Mon Sep 17 00:00:00 2001
From: Corey Huinker <corey.huin...@gmail.com>
Date: Wed, 16 Oct 2024 19:13:01 -0400
Subject: [PATCH v3 1/2] Allow relation_statistics_update to continue after
 non-ERROR validation.

Some validation checks would, when elevel was set to WARNING or lower,
immediately return false after the failed check, rather than let the
update continue on to other attributes.

Currently the only caller is pg_set_relation_stats() which sets the
elevel to ERROR, so this incorrect behavior would never surface, but
later callers will user WARNING elevel.
---
 src/backend/statistics/relation_stats.c | 31 +++++++++----------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c
index 1a6d1640c3..126eab49d8 100644
--- a/src/backend/statistics/relation_stats.c
+++ b/src/backend/statistics/relation_stats.c
@@ -67,7 +67,6 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
 	bool		nulls[3] = {0};
 	int			ncols = 0;
 	TupleDesc	tupdesc;
-	HeapTuple	newtup;
 
 
 	stats_check_required_arg(fcinfo, relarginfo, RELATION_ARG);
@@ -110,10 +109,8 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 					 errmsg("relpages cannot be < -1")));
 			table_close(crel, RowExclusiveLock);
-			return false;
 		}
-
-		if (relpages != pgcform->relpages)
+		else if (relpages != pgcform->relpages)
 		{
 			replaces[ncols] = Anum_pg_class_relpages;
 			values[ncols] = Int32GetDatum(relpages);
@@ -131,10 +128,8 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 					 errmsg("reltuples cannot be < -1.0")));
 			table_close(crel, RowExclusiveLock);
-			return false;
 		}
-
-		if (reltuples != pgcform->reltuples)
+		else if (reltuples != pgcform->reltuples)
 		{
 			replaces[ncols] = Anum_pg_class_reltuples;
 			values[ncols] = Float4GetDatum(reltuples);
@@ -152,10 +147,8 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 					 errmsg("relallvisible cannot be < 0")));
 			table_close(crel, RowExclusiveLock);
-			return false;
 		}
-
-		if (relallvisible != pgcform->relallvisible)
+		else if (relallvisible != pgcform->relallvisible)
 		{
 			replaces[ncols] = Anum_pg_class_relallvisible;
 			values[ncols] = Int32GetDatum(relallvisible);
@@ -164,22 +157,20 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
 	}
 
 	/* only update pg_class if there is a meaningful change */
-	if (ncols == 0)
+	if (ncols > 0)
 	{
-		table_close(crel, RowExclusiveLock);
-		return false;
+		HeapTuple	newtup;
+
+		newtup = heap_modify_tuple_by_cols(ctup, tupdesc, ncols, 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);
-
 	/* release the lock, consistent with vac_update_relstats() */
 	table_close(crel, RowExclusiveLock);
 
-	return true;
+	return (ncols > 0);
 }
 
 /*
-- 
2.47.0

From b36976de8d2e5a797eb48859ef597d8c157e8a3e Mon Sep 17 00:00:00 2001
From: Corey Huinker <corey.huin...@gmail.com>
Date: Fri, 18 Oct 2024 19:06:13 -0400
Subject: [PATCH v3 2/2] Change pg_set_relation_stats() return type to void.

This function will either raise an ERROR or run to normal completion.
The boolean return value of relation_statistics_update() is preserved,
but that is useful for more nuanced future uses than
pg_set_relation_stats().
---
 src/include/catalog/pg_proc.dat            |  4 ++--
 src/backend/catalog/system_functions.sql   |  2 +-
 src/backend/statistics/relation_stats.c    |  6 ++++--
 src/test/regress/expected/stats_import.out | 16 ++++++++--------
 doc/src/sgml/func.sgml                     | 11 ++++-------
 5 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 7c0b74fe05..b4430e7c77 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -12345,14 +12345,14 @@
 { oid => '9944',
   descr => 'set statistics on relation',
   proname => 'pg_set_relation_stats', provolatile => 'v', proisstrict => 'f',
-  proparallel => 'u', prorettype => 'bool',
+  proparallel => 'u', prorettype => 'void',
   proargtypes => 'regclass int4 float4 int4',
   proargnames => '{relation,relpages,reltuples,relallvisible}',
   prosrc => 'pg_set_relation_stats' },
 { oid => '9945',
   descr => 'clear statistics on relation',
   proname => 'pg_clear_relation_stats', provolatile => 'v', proisstrict => 'f',
-  proparallel => 'u', prorettype => 'bool',
+  proparallel => 'u', prorettype => 'void',
   proargtypes => 'regclass',
   proargnames => '{relation}',
   prosrc => 'pg_clear_relation_stats' },
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index 92b6aefe7a..b7e2906f11 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -644,7 +644,7 @@ CREATE OR REPLACE FUNCTION
                         relpages integer DEFAULT NULL,
                         reltuples real DEFAULT NULL,
                         relallvisible integer DEFAULT NULL)
-RETURNS bool
+RETURNS void
 LANGUAGE INTERNAL
 CALLED ON NULL INPUT VOLATILE
 AS 'pg_set_relation_stats';
diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c
index 126eab49d8..7c6ae4ecd8 100644
--- a/src/backend/statistics/relation_stats.c
+++ b/src/backend/statistics/relation_stats.c
@@ -179,7 +179,8 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
 Datum
 pg_set_relation_stats(PG_FUNCTION_ARGS)
 {
-	PG_RETURN_BOOL(relation_statistics_update(fcinfo, ERROR));
+	relation_statistics_update(fcinfo, ERROR);
+	PG_RETURN_VOID();
 }
 
 /*
@@ -202,5 +203,6 @@ pg_clear_relation_stats(PG_FUNCTION_ARGS)
 	newfcinfo->args[3].value = DEFAULT_RELALLVISIBLE;
 	newfcinfo->args[3].isnull = false;
 
-	PG_RETURN_BOOL(relation_statistics_update(newfcinfo, ERROR));
+	relation_statistics_update(newfcinfo, ERROR);
+	PG_RETURN_VOID();
 }
diff --git a/src/test/regress/expected/stats_import.out b/src/test/regress/expected/stats_import.out
index a4e3a0f3c4..b50af75bb9 100644
--- a/src/test/regress/expected/stats_import.out
+++ b/src/test/regress/expected/stats_import.out
@@ -38,7 +38,7 @@ SELECT
         relallvisible => 4::integer);
  pg_set_relation_stats 
 -----------------------
- t
+ 
 (1 row)
 
 -- reltuples default
@@ -50,7 +50,7 @@ SELECT
         relallvisible => 4::integer);
  pg_set_relation_stats 
 -----------------------
- t
+ 
 (1 row)
 
 -- relallvisible default
@@ -62,7 +62,7 @@ SELECT
         relallvisible => NULL::integer);
  pg_set_relation_stats 
 -----------------------
- f
+ 
 (1 row)
 
 -- named arguments
@@ -74,7 +74,7 @@ SELECT
         relallvisible => 4::integer);
  pg_set_relation_stats 
 -----------------------
- f
+ 
 (1 row)
 
 SELECT relpages, reltuples, relallvisible
@@ -94,7 +94,7 @@ SELECT
         5::integer);
  pg_set_relation_stats 
 -----------------------
- t
+ 
 (1 row)
 
 SELECT relpages, reltuples, relallvisible
@@ -111,7 +111,7 @@ SELECT
         'stats_import.test'::regclass);
  pg_clear_relation_stats 
 -------------------------
- t
+ 
 (1 row)
 
 SELECT relpages, reltuples, relallvisible
@@ -158,7 +158,7 @@ SELECT
         relpages => 2::integer);
  pg_set_relation_stats 
 -----------------------
- t
+ 
 (1 row)
 
 -- nothing stops us from setting it to -1
@@ -168,7 +168,7 @@ SELECT
         relpages => -1::integer);
  pg_set_relation_stats 
 -----------------------
- t
+ 
 (1 row)
 
 DROP SCHEMA stats_import CASCADE;
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index ad663c94d7..14dd035134 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -30174,15 +30174,13 @@ DETAIL:  Make sure pg_wal_replay_wait() isn't called within a transaction with a
          <optional>, <parameter>relpages</parameter> <type>integer</type></optional>
          <optional>, <parameter>reltuples</parameter> <type>real</type></optional>
          <optional>, <parameter>relallvisible</parameter> <type>integer</type></optional> )
-         <returnvalue>boolean</returnvalue>
+         <returnvalue>void</returnvalue>
         </para>
         <para>
          Updates relation-level statistics for the given relation to the
          specified values. The parameters correspond to columns in <link
          linkend="catalog-pg-class"><structname>pg_class</structname></link>. Unspecified
-         or <literal>NULL</literal> values leave the setting
-         unchanged. Returns <literal>true</literal> if a change was made;
-         <literal>false</literal> otherwise.
+         or <literal>NULL</literal> values leave the setting unchanged.
         </para>
         <para>
          Ordinarily, these statistics are collected automatically or updated
@@ -30212,12 +30210,11 @@ DETAIL:  Make sure pg_wal_replay_wait() isn't called within a transaction with a
           <primary>pg_clear_relation_stats</primary>
          </indexterm>
          <function>pg_clear_relation_stats</function> ( <parameter>relation</parameter> <type>regclass</type> )
-         <returnvalue>boolean</returnvalue>
+         <returnvalue>void</returnvalue>
         </para>
         <para>
          Clears table-level statistics for the given relation, as though the
-         table was newly created. Returns <literal>true</literal> if a change
-         was made; <literal>false</literal> otherwise.
+         table was newly created.
         </para>
         <para>
          The caller must have the <literal>MAINTAIN</literal> privilege on
-- 
2.47.0

Reply via email to