Date: Sat, 11 Jul 2020 18:21:03 +0200
From: Dmitry Dolgov <9erthalion6@gmail.com>
To: Floris Van Nee <florisvannee@Optiver.com>
Cc: Andy Fan <zhihui.fan1213@gmail.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>,
	Jesper Pedersen <jesper.pedersen@redhat.com>,
	David Rowley <dgrowleyml@gmail.com>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	Peter Geoghegan <pg@bowt.ie>, Thomas Munro <thomas.munro@gmail.com>,
	Tomas Vondra <tomas.vondra@2ndquadrant.com>,
	Dilip Kumar <dilipbalaut@gmail.com>
Subject: Re: Index Skip Scan (new UniqueKeys)
Message-ID: <20200711162103.hiygat3k6wu2d25p@localhost>
References: <20200609102247.jdlatmfyeecg52fi@localhost>
 <CAKU4AWrqYz7-+rr=mGe8w6i=NsAZap6cKe=QoQsYGeb4AuiSaA@mail.gmail.com>
 <20200629120709.52w2zi36mtzyliv2@localhost>
 <fb18ee881ea24d31b5272b4e5053a6aa@opammb0561.comp.optiver.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <fb18ee881ea24d31b5272b4e5053a6aa@opammb0561.comp.optiver.com>
Precedence: bulk

> On Fri, Jul 10, 2020 at 05:03:37PM +0000, Floris Van Nee wrote:
>
> Also took another look at the patch now, and found a case of incorrect
> data. It looks related to the new way of creating the paths, as I
> can't recall seeing this in earlier versions.
>
> create table t1 as select a,b,b%5 as c, random() as d from generate_series(1, 10) a, generate_series(1,100) b;
> create index on t1 (a,b,c);
>
> postgres=# explain select distinct on (a) * from t1 order by a,b desc,c;
>                                   QUERY PLAN
> -------------------------------------------------------------------------------
>  Sort  (cost=2.92..2.94 rows=10 width=20)
>    Sort Key: a, b DESC, c
>    ->  Index Scan using t1_a_b_c_idx on t1  (cost=0.28..2.75 rows=10 width=20)
>          Skip scan: true
> (4 rows)

Good point, thanks for looking at this. With the latest planner version
there are indeed more possibilities to use skipping. It never occured to
me that some of those paths will still rely on index scan returning full
data set. I'll look in details and add verification to prevent putting
something like this on top of skip scan in the next version.