Re: [HACKERS] multivariate statistics (v19)

On 01/30/2017 05:12 PM, Alvaro Herrera wrote:>
> Hmm.  So we have a catalog pg_mv_statistics which stores two things:
> 1. the configuration regarding mvstats that have been requested by user
>    via CREATE/ALTER STATISTICS
> 2. the actual values captured from the above, via ANALYZE
>
> I think this conflates two things that really are separate, given their
> different timings and usage patterns.  This decision is causing the
> catalog to have columns enabled/built flags for each set of stats
> requested, which looks a bit odd.  In particular, the fact that you have
> to heap_update the catalog in order to add more stuff as it's built
> looks inconvenient.
>
> Have you thought about having the "requested" bits be separate from the
> actual computed values?  Something like
>
> pg_mv_statistics
>   starelid
>   staname
>   stanamespace
>   staowner     -- all the above as currently
>   staenabled    array of "char" {d,f,s}
>   stakeys
> // no CATALOG_VARLEN here
>
> where each char in the staenabled array has a #define and indicates one
> type, "ndistinct", "functional dep", "selectivity" etc.
>
> The actual values computed by ANALYZE would live in a catalog like:
>
> pg_mv_statistics_values
>   stvstaid    -- OID of the corresponding pg_mv_statistics row.  Needed?

Definitely needed. How else would you know which MCV list and histogram 
belong together? This works just like in pg_statistic - when both MCV 
and histograms are enabled for the statistic, we first build MCV list, 
then histogram on remaining rows. So we need to pair them.

>   stvrelid    -- same as starelid
>   stvkeys    -- same as stakeys
> #ifdef CATALOG_VARLEN
>   stvkind    'd' or 'f' or 's', etc
>   stvvalue    the bytea blob
> #endif
>
> I think that would be simpler, both conceptually and in terms of code.

I think the main issue here is that it throws away the special data 
types (pg_histogram, pg_mcv, pg_ndistinct, pg_dependencies), which I 
think is a neat idea and would like to keep it. This would throw that 
away, making everything bytea again. I don't like that.

>
> The other angle to consider is planner-side: how does the planner gets
> to the values?  I think as far as the planner goes, the first catalog
> doesn't matter at all, because a statistics type that has been enabled
> but not computed is not interesting at all; planner only cares about the
> values in the second catalog (this is why I added stvkeys).  Currently
> you're just caching a single pg_mv_statistics row in get_relation_info
> (and only if any of the "built" flags is set), which is simple.  With my
> proposed change, you'd need to keep multiple pg_mv_statistics_values
> rows.
>
> But maybe you already tried something like what I propose and there's a
> reason not to do it?
>

Honestly, I don't see how this improves the situation. We still need to 
cache data for exactly one catalog, so how is that simpler?

The way I see it, it actually makes things more complicated, because now 
we have two catalogs to manage instead of one (e.g. when doing DROP 
STATISTICS, or after ALTER TABLE ... DROP COLUMN).

The 'built' flags may be easily replaced with a check if the bytea-like 
columns are NULL, and the 'enabled' columns may be replaced by the array 
of char, just like you proposed.

That'd give us a single catalog looking like this:

pg_mv_statistics  starelid  staname  stanamespace  staowner      -- all the above as currently  staenabled    array of
"char"{d,f,s}  stakeys  stadeps  (dependencies)  standist (ndistinct coefficients)  stamcv   (MCV list)  stahist
(histogram)

Which is probably a better / simpler structure than the current one.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services