Re: PoC/WIP: Extended statistics on expressions

Hi,

Attached is a patch fixing most of the issues. There are a couple
exceptions:

> * Looking at the pg_stats_ext view, I think perhaps expressions stats
> should be omitted entirely from that view, since it doesn't show any
> useful information about them. So it could remove "e" from the "kinds"
> array, and exclude rows whose only kind is "e", since such rows have
> no interesting data in them. Essentially, the new view
> pg_stats_ext_exprs makes having any expression stats in pg_stats_ext
> redundant. Hiding this data in pg_stats_ext would also be consistent
> with making the "expressions" stats kind hidden from the user.

I haven't removed the expressions stats from pg_stats_ext view yet. I'm
not 100% sure about it yet.

> * Why does the new code in tcop/utility.c not use
> RangeVarGetRelidExtended together with RangeVarCallbackOwnsRelation?
> That seems preferable to doing the ACL check in CreateStatistics().
> For one thing, as it stands, it allows the lock to be taken even if
> the user doesn't own the table. Is it intentional that the current
> code allows extended stats to be created on system catalogs? That
> would be one thing that using RangeVarCallbackOwnsRelation would
> change, but I can't see a reason to allow it.

I haven't switched utility.c to RangeVarGetRelidExtended together with
RangeVarCallbackOwnsRelation, because the current code allows checking
for object type first. I don't recall why exactly was it done this way,
but I didn't feel like changing that in this patch.

You're however right it should not be possible to create statistics on
system catalogs. For regular users that should be rejected thanks to the
ownership check, but superuser may create it. I've added proper check to
CreateStatistics() - this is probably worth backpatching.

> * In src/bin/psql/describe.c, I think the \d output should also
> exclude the "expressions" stats kind and just list the other kinds (or
> have no kinds list at all, if there are no other kinds), to make it
> consistent with the CREATE STATISTICS syntax.

I've done this, but I went one step further - we hide the list of kinds
using the same rules as pg_dump, i.e. we don't list the kinds if all of
them are selected. Not sure if that's the right thing, though.

The rest of the issues should be fixed, I think.

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company