On Tue, Feb 14, 2017 at 12:48 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> That sounds way better.
Here's an updated patch. Please review my changes, which include:
* Various comment updates.
* _bt_parallel_seize now unconditionally sets *pageno to P_NONE at the
beginning, instead of doing it conditionally at the end. This seems
cleaner to me.
* I removed various BTScanPosInvalidate calls from _bt_first in places
where they followed calls to _bt_parallel_done, because I can't see
how the scan position could be valid at that point; note that
_bt_first asserts that it is invalid on entry.
* I added a _bt_parallel_done() call to _bt_first where it apparently
returned without releasing the scan; search for SK_ROW_MEMBER. Maybe
there's something I'm missing that makes this unnecessary, but if so
there should probably be a comment here.
* I wasn't happy with the strange calling convention where
_bt_readnextpage usually gets a valid block number but not for
non-parallel backward scans. I had a stab at fixing that so that the
block number is always valid, but I'm not entirely sure I've got the
logic right. Please see what you think.
* I repositioned the function prototypes you added to nbtree.h to
separate the public and non-public interfaces.
I can't easily test this because your second patch doesn't apply, so
I'd appreciate it if you could have a stab at checking whether I've
broken anything in this revision. Also, it would be good if you could
rebase the second patch.
I think this is pretty close to committable at this point. Whether or
not I broke anything in this revision, I don't think there's too much
left to be done here.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers