Re: Statistics Import and Export - Mailing list pgsql-hackers

On Sat, Aug 24, 2024 at 4:50 AM Jeff Davis <pgsql@j-davis.com> wrote:
>
>
> I have attached version 28j as one giant patch covering what was
> previously 0001-0003. It's a bit rough (tests in particular need some
> work), but it implelements the logic to replace only those values
> specified rather than the whole tuple.
>
hi.
I did some review for v28j

git am shows some whitespace error.


+extern Datum pg_set_relation_stats(PG_FUNCTION_ARGS);
+extern Datum pg_set_attribute_stats(PG_FUNCTION_ARGS);
is unnecessary?


+       <entry role="func_table_entry">
+        <para role="func_signature">
+         <indexterm>
+          <primary>pg_set_relation_stats</primary>
+         </indexterm>
+         <function>pg_set_relation_stats</function> (
+         <parameter>relation</parameter> <type>regclass</type>
+         <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>
+        </para>
+        <para>
+         Updates table-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.
+        </para>
are these <optional> flags wrong? there is only one function currently:
pg_set_relation_stats(relation regclass, relpages integer, reltuples
real, relallvisible integer)
i think you want
pg_set_relation_stats(relation regclass, relpages integer default
null, reltuples real default null, relallvisible integer default null)
we can add following in src/backend/catalog/system_functions.sql:

select * from pg_set_relation_stats('emp'::regclass);
CREATE OR REPLACE FUNCTION
  pg_set_relation_stats(
                        relation regclass,
                        relpages integer default null,
                        reltuples real default null,
                        relallvisible integer default null)
RETURNS bool
LANGUAGE INTERNAL
CALLED ON NULL INPUT VOLATILE
AS 'pg_set_relation_stats';


typedef enum ...
need to add src/tools/pgindent/typedefs.list


+/*
+ * Check that array argument is one dimensional with no NULLs.
+ *
+ * If not, emit at elevel, and set argument to NULL in fcinfo.
+ */
+static void
+check_arg_array(FunctionCallInfo fcinfo, struct arginfo *arginfo,
+ int argnum, int elevel)
+{
+ ArrayType  *arr;
+
+ if (PG_ARGISNULL(argnum))
+ return;
+
+ arr = DatumGetArrayTypeP(PG_GETARG_DATUM(argnum));
+
+ if (ARR_NDIM(arr) != 1)
+ {
+ ereport(elevel,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("\"%s\" cannot be a multidimensional array",
+ arginfo[argnum].argname)));
+ fcinfo->args[argnum].isnull = true;
+ }
+
+ if (array_contains_nulls(arr))
+ {
+ ereport(elevel,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("\"%s\" array cannot contain NULL values",
+ arginfo[argnum].argname)));
+ fcinfo->args[argnum].isnull = true;
+ }
+}
this part elevel should always be ERROR?
if so, we can just
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),




relation_statistics_update and other functions
may need to check relkind?
since relpages, reltuples, relallvisible not meaning to all of relkind?