> > > > 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