Re: Page Scan Mode in Hash Index

From: Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Jesper Pedersen <jesper(dot)pedersen(at)redhat(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Page Scan Mode in Hash Index
Date: 2017-09-20 15:43:42
Message-ID: CAE9k0PktGvfidvzoTJNRD8WXY0hUAo3wktYWy4tFN0oY6xWM9Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Wed, Sep 20, 2017 at 8:05 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Wed, Sep 20, 2017 at 5:37 AM, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
>> Thanks for all your review comments. Please find my comments in-line.
>
> + if (!BlockNumberIsValid(opaque->hasho_nextblkno))
> + {
> + if (so->currPos.buf == so->hashso_bucket_buf ||
> + so->currPos.buf == so->hashso_split_bucket_buf)
> + prev_blkno = InvalidBlockNumber;
> + else
> + prev_blkno = opaque->hasho_prevblkno;
> + }
>
> 1. Why not remove the outer "if" statement?
>

Yes, the outer if statement is not required. I just missed to remove
that in my earlier patch.

> 2. How about adding a comment, like /* If this is a primary bucket
> page, hasho_prevblkno is not a real block number. */
>

Added.

>> When _hash_readpage() doesn't find any qualifying tuples i.e. when
>> _hash_readnext() returns Invalid buffer, we just update prevPage,
>> nextPage and buf in
>> currPos (not currPage or lsn) as currPage and lsn should point to last
>> page in the hash bucket so that we can mark the killed items as dead
>> at the end of scan (with the help of _hash_kill_items). Hence, we keep
>> the currpage and lsn as it is if no more valid hash pages are found.
>
> How about adding a comment about this, by extending this comment:
>
> + * Remember next and previous block numbers for scrollable
> + * cursors to know the start position and return FALSE
> + * indicating that no more matching tuples were found.
>
> e.g. (Don't reset currPage or lsn, because we expect _hash_kill_items
> to be called for the old page after this function returns.)
>
>

Added.

Attached are the patches with above changes. Thanks.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

Attachment Content-Type Size
0001-Rewrite-hash-index-scan-to-work-page-at-a-time_v16.patch text/x-patch 32.1 KB
0002-Remove-redundant-hash-function-_hash_step-and-do-som.patch text/x-patch 8.5 KB
0003-Improve-locking-startegy-during-VACUUM-in-Hash-Index_v7.patch text/x-patch 7.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Khandekar 2017-09-20 15:57:10 Re: UPDATE of partition key
Previous Message Peter Geoghegan 2017-09-20 15:40:02 Re: Varying results when using merge joins over postgres_fdw vs hash joins