Date: Mon, 9 Mar 2020 20:57:14 +0100
From: Dmitry Dolgov <9erthalion6@gmail.com>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	Tomas Vondra <tomas.vondra@2ndquadrant.com>,
	florisvannee@optiver.com, pg@bowt.ie, jesper.pedersen@redhat.com,
	Alvaro Herrera <alvherre@2ndquadrant.com>, thomas.munro@gmail.com,
	jtc331@gmail.com, rafia.pghackers@gmail.com,
	Jeff Janes <jeff.janes@gmail.com>, bhushan.uparkar@gmail.com,
	PostgreSQL-development <pgsql-hackers@postgresql.org>,
	Alexander Korotkov <a.korotkov@postgrespro.ru>
Subject: Re: Index Skip Scan
Message-ID: <20200309195714.eum3b6e5mogqj3gm@localhost>
References: <20200206.212220.642002310227620136.horikyota.ntt@gmail.com>
 <20200207162543.zlj2tvcwy7jku6vu@localhost>
 <20200208131159.5sjcbcjnwdj4q7fa@development>
 <20200214.172313.259958375378013155.horikyota.ntt@gmail.com>
 <20200214121820.xvvrkdbykkygfs65@localhost>
 <20200217162451.nrpy5k63f5y2mwm6@localhost>
 <CAApHDvo2abVQOiZ6wf9ExhobvvhLGWZQha0uWRAMby1NOD1ccw@mail.gmail.com>
 <20200308142228.3zb237cd3oedspl2@localhost>
 <CAApHDvrMRHm3BoW0ZHHAmZY91HVSSkhv1zKSPfQF0QfkN7NdpQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAApHDvrMRHm3BoW0ZHHAmZY91HVSSkhv1zKSPfQF0QfkN7NdpQ@mail.gmail.com>
Precedence: bulk

> On Mon, Mar 09, 2020 at 10:27:26AM +1300, David Rowley wrote:
>
> > > I think the changes in create_distinct_paths() need more work.  The
> > > way I think this should work is that create_distinct_paths() gets to
> > > know exactly nothing about what path types support the elimination of
> > > duplicate values.  The Path should carry the UniqueKeys so that can be
> > > determined. In create_distinct_paths() you should just be able to make
> > > use of those paths, which should already have been created when
> > > creating index paths for the rel due to PlannerInfo's query_uniquekeys
> > > having been set.
> >
> > Just for me to clarify. The idea is to "move" information about what
> > path types support skipping into UniqueKeys (derived from PlannerInfo's
> > query_uniquekeys), but other checks (e.g. if index am supports that)
> > still perform in create_distinct_paths?
>
> create_distinct_paths() shouldn't know any details specific to the
> pathtype that it's using or considering using.  All the details should
> just be in Path. e.g. uniquekeys, pathkeys, costs etc. There should be
> no IsA(path, ...).  Please have a look over the details in my reply to
> Tomas. I hope that reply has enough information in it, but please
> reply there if I've missed something.

Yes, I've read this reply, just wanted to ask here, since I had other
questions as well. Speaking of which:

> > > On Wed, Mar 04, 2020 at 11:32:00AM +1300, David Rowley wrote:
> > > There's also some weird looking assumptions that an EquivalenceMember
> > > can only be a Var in create_distinct_paths(). I think you're only
> > > saved from crashing there because a ProjectionPath will be created
> > > atop of the IndexPath to evaluate expressions, in which case you're
> > > not seeing the IndexPath.

I'm probably missing something, so to eliminate any misunderstanding
from my side:

> > > This results in the optimisation not working in cases like:
> > >
> > > postgres=# create table t (a int); create index on t ((a+1)); explain
> > > select distinct a+1 from t;
> > > CREATE TABLE
> > > CREATE INDEX
> > >                         QUERY PLAN
> > > -----------------------------------------------------------
> > >  HashAggregate  (cost=48.25..50.75 rows=200 width=4)
> > >    Group Key: (a + 1)
> > >    ->  Seq Scan on t  (cost=0.00..41.88 rows=2550 width=4)

In this particular example skipping is not applied because, as you've
mentioned, we're dealing with ProjectionPath (not IndexScan /
IndexOnlyScan). Which means we're not even reaching the code with
EquivalenceMember, so I'm still not sure how do they connected?

Assuming we'll implement it in a way that we do not know about what kind
of path type is that in create_distinct_path, then it can also work for
ProjectionPath or anything else (if UniqueKeys are present). But then
still EquivalenceMember are used only to figure out correct
distinctPrefixKeys and do not affect whether or not skipping is applied.
What do I miss?