Re: Statistics Import and Export

On Sat, Aug 24, 2024 at 4:50 AM Jeff Davis <pgsql(at)j-davis(dot)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?