Re: Is there value in having optimizer stats for joins/foreignkeys?

>
> Overall, I think 0001 goes in the right direction, and maybe we should
> even apply it early, without waiting for the rest of the patch series.
> It seems like an improvement.
>

+1

> The main question I have is whether maybe we should get rid of the
> columns vs. expressions in more places. Currently the patch removes that
> from the catalog, but then it still "restores" this difference when
> reading the statistics information, so that both StatExtEntry and
> StatisticExtInfo still store this separately. But do we need to?
>

I'd be in favor of this. It was a pain interleaving the expressions in
every time we found a negative attnum, and it also prevented skippping
ahead to to desired attribute because we had to know that we had consumed
all "previous" expressions. Good riddance to that.

> It means we still need two loops in a lot of places, first over columns
> and then over expressions. If we treated everything as expressions,
> wouldn't it be simpler?
>

That might be the best way.

> I noticed a couple more details for 0002:
>
> - I didn't realize we require the user to pick the "anchor" table and
> put it first in the CREATE STATISTICS command:
>
> CREATE INDEX ON t1 (a);
> CREATE STATISTICS (mcv) on t1.b, t1.c, t2.e, t2.f from t1 join t2
> ON (t1.a = t2.d);
> ERROR: no suitable index on "t2" column "d" for join statistics
> HINT: Create an index on the join column to enable index-based join
> sampling.
>
> In this example both t1 and t2 could have be an anchor table. So maybe
> we could try determining the anchor table automatically? But maybe there
> are issues, e.g. what if there are multiple candidates?
>

It's true there could be multiple candidates, but I think we want a clearly
defined anchor table because that is the table that controls when these
stats are refreshed, as it is the one that has the real rowsample, and all
the others are joins off of that.

That could mean that we have essentially the same extended stats object on
two different tables, differing only in the syntax required to specify the
different anchor table, but such situations should be rare.

> - It's a bit inconvenient the statistics is listed in \d only for the
> anchor table. It'd be good to list it for all tables, but maybe add an
> info whether it's the anchor or not?
>

I'm iffy on this.

On the one hand, it would be helpful information for the fact-like tables
in the join, and even if we adopt the "view can have stats" version that
I'm advocating we could still have \d discover that through a pg_depend.

On the other hand, extended stats declared on a big fact-dimension join
would result in \d on a dimension table showing a bunch of extended stats
objects which would be peripheral information at best.

>
>
> > A few questions on where to take this next:
> >
> > 1. N-way joins. Tomas, you made the case that 2-way isn't enough (a
> > fact table joining two correlated dimensions). The catalog is already
> > prepared for n-way, but ANALYZE skips collection with a warning.
> > Should extending collection to n-way be the next piece, or would you
> > rather see the 2-way version land first?
> >
>
> I don't think n-way joins have to be supported from the beginning, but
> it would be good to have an experimental patch doing that in the patch
> series. In my experience it helps with getting the infrastructure ready
> for supporting it later.
>

While they don't have to be there from the very beginning, I fear that not
including them will cause us to paint ourselves into a corner somewhere.

>
> > 2. Other statistics kinds such as ndistinct and functional dependencies.
> > Are those other stats kinds must haves in the initial commit?
> >
>
> I think this is similar. I think it's fine to not support all these
> statistical kinds in the initial commit, but it'd help to have some sort
> of experimental patch.
>
> > 3. Index-requirement semantics (Chengpeng, Tom, Tomas). Currently a
> > suitable index on the probed join column is required: CREATE
> > STATISTICS errors if none exists, and a NORMAL dependency on the
> > chosen index makes DROP INDEX require CASCADE (so the stats can't
> > silently stop building). ANALYZE re-selects the best available index
> > on each run. Is this what we want? The alternatives I considered:
> > a. Drop the index requirement and add a non-index (sequential-scan)
> > sampling fallback. This is the most user-friendly, and not necessarily
> > slower when the joined tables are small.
> > b. Pin the specific index by storing its OID in the catalog. This
> > makes the dependency exact, but I'm unsure how it should interact with
> > REINDEX: REINDEX CONCURRENTLY swaps the index to a new OID — the
> > NORMAL dependency follows automatically, but a stored OID would need
> > to be updated explicitly.
> > c. Block DROP INDEX only when it would remove the last suitable
> > index (allowing it while an equivalent remains). Also user-friendly,
> > but I haven't found a clean way to implement it.
> >
> > I'm currently inclined either to keep the present behavior for v1, or
> > to go with (a) and add the seq-scan fallback. Would appreciate your
> > thoughts.
> >
>
> Not sure. I think both (a) and (c) would be OK. I'd probably go with
> some version of (a), i.e. pick a suitable index at ANALYZE time, and
> either "fail" when there's no index or use a seqscan sampling.
>

I'm a big proponent of "a" here. The negative consequence of a missing
index is a slow ANALYZE, remedied by creating the index.

"b" creates a maintenance headache. If we lock ourselves into index x1, and
x1 gets bloated, how do we switch to a compacted x2 index? If x1 is dropped
and rebuilt, must we rebuild the stats object? If x1 is dropped by
(x1,y1,z1) exists, couldn't we live with that
sub-optimal-but-better-than-seqscan index?

"c" creates a situation where the user experiences no negative consequences
for dropping 1..N-1 indexes, but the last index fails for a reason that
didn't previously prevent the others from being dropped.