|From:||Dmitry Dolgov <9erthalion6(at)gmail(dot)com>|
|To:||Peter Geoghegan <pg(at)bowt(dot)ie>|
|Cc:||Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Floris Van Nee <florisvannee(at)optiver(dot)com>, Jesper Pedersen <jesper(dot)pedersen(at)redhat(dot)com>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, James Coleman <jtc331(at)gmail(dot)com>, Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Bhushan Uparkar <bhushan(dot)uparkar(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>|
|Subject:||Re: Index Skip Scan|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
> On Sun, Nov 03, 2019 at 05:45:47PM +0100, Dmitry Dolgov wrote:
> > * The extra scankeys that you are using in most of the new nbtsearch.c
> > code is an insertion scankey -- not a search style scankey. I think
> > that you should try to be a bit clearer on that distinction in
> > comments. It is already confusing now, but at least there is only zero
> > or one insertion scankeys per scan (for the initial positioning).
> > * There are two _bt_skip() prototypes in nbtree.h (actually, there is
> > a btskip() and a _bt_skip()). I understand that the former is a public
> > wrapper of the latter, but I find the naming a little bit confusing.
> > Maybe rename _bt_skip() to something that is a little bit more
> > suggestive of its purpose.
> > * Suggest running pgindent on the patch.
> Sure, I'll incorporate mentioned improvements into the next patch
> version (hopefully soon).
Here is the new version, that addresses mentioned issues.
> > * Is _bt_scankey_within_page() really doing the right thing within empty pages?
> > It looks like you're accidentally using the high key when the leaf
> > page is empty with forward scans (assuming that the leaf page isn't
> > rightmost). You'll need to think about empty pages for both forward
> > and backward direction scans there.
> Yes, you're right, that's an issue I need to fix.
If I didn't misunderstood something, for the purpose of this function it
makes sense to return false in the case of empty page. That's what I've
added into the patch.
> > Actually, using the high key in some cases may be desirable, once the
> > details are worked out -- the high key is actually very helpful with
> > low cardinality indexes. If you populate an index with retail
> > insertions (i.e. don't just do a CREATE INDEX after the table is
> > populated), and use low cardinality data in the indexed columns then
> > you'll see this effect.
> Can you please elaborate a bit more? I see that using high key will help
> a forward index scans to access the minimum number of leaf pages, but
> I'm not following how is it connected to the _bt_scankey_within_page? Or
> is this commentary related in general to the whole implementation?
This question is still open.
|Next Message||Tom Lane||2019-11-07 16:30:30||Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )|
|Previous Message||Bruce Momjian||2019-11-07 15:28:31||Re: 64 bit transaction id|