> Ideally, we would want the OID and attnum to be first columns to match
> the pg_stat_activity/pg_stat_progress style, but that will be too invasive.
>
I don't think there is an order that will make anyone happy, let alone
everyone.
The existing columns (schemaname, tablename, attname, inherited) constitute
a grain, so its hard to separate them.
Once we add attnum, that can replace attname in the above grain. Once we
add starelid/tableid, then that can be combined win inherited and either
attnum/attname for a grain. The ordering of these columns will largely fall
down to the perspective of the person seeking to use it.
>
> - This <type> should be int2
> - Maybe for the describtion, it should be something like:
>
+1
>
> "The number of the column, as in
> <structname>pg_attribute</structname>.<structfield>attnum</structfield>"
>
We don't do that for attname, why would attnum be different?
>
>
> + <row>
> + <entry role="catalog_table_entry"><para role="column_definition">
> + <structfield>starelid</structfield> <type>oid</type>
> + </para>
> + <para>
> + ID of the relation
> + </para></entry>
> + </row>
>
> This should indicate it is a reference to pg_class.oid, like so:
>
I went with pg_attribute, but the reference is there now.
>
> Maybe "ID of the table or index" is better, since this can only be a
> table or index
> for pg_stats.
>
I went with table, because it matches the tablename definition (i.e. it is
equally inaccurate).
>
> I dislike the existing "pg_stats.tablename", since this can also be an
> expression index.
> "pg_stats.relation" with a description of "Name of table or index" is
> more appropriate.
> It is a change that we can possibly make in a major version. Looked
> through the archives,
> and did not see this being reported/discussed.
>
I don't see it changing in any version, minor or major.
>
> v1-0002:
>
> I examined the version variants of getAttributeStats and all looks good,
> and also ran a test on a 18 and 19 server version with the patched pg_dump
> client and all looks good.
>
> One minor comment is:
>
>
> + pg_fatal("statistics table oid information missing");
>
>
> I noticed in other pg_fatal messages, we include OIDs
>
> pg_fatal("could not find function definition for function
> with OID %u",
> cast->castfunc);
>
> Should we do the same here?
>
If I had the oid, I wouldn't have the error. :)
I've modified the error message to give the namespace/tag of the entry.
Will post revised patch later.