From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 28 Mar 2019 17:58:19 +0300 From: Vladimir Davydov Subject: Re: [tarantool-patches] Re: [PATCH 5/6] vinyl: run iterator: refactor seek method Message-ID: <20190328145819.rovnk4txkhfziiec@esperanza> References: <59095674ca1bd479afa87f38d4088c4738ee940f.1553613748.git.vdavydov.dev@gmail.com> <20190328143947.GE23295@chai> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190328143947.GE23295@chai> To: Konstantin Osipov Cc: tarantool-patches@freelists.org List-ID: On Thu, Mar 28, 2019 at 05:39:47PM +0300, Konstantin Osipov wrote: > * Vladimir Davydov [19/03/26 18:56]: > > A few changes to make the function more straightforward: > > > > - Move bloom checking and LSN filtering out of 'do_seek' helper. Make > > the helper do just one simple task - lookup the first one in a series > > of statements matching the given search criteria. > > - Fold iterator type and key substitution in 'seek' method, similarly > > to how we did it for other iterators. > > - Cleanup EQ checks. Use the original iterator type and key where > > appropriate to remove extra checks in calling methods. Don't check EQ > > in 'seek' method in case it was checked by 'do_seek'. > > - Add some comments. > > LGTM (same comment re passing EOF in return value applies). > eq_found looks cumbersome, perhaps you could both kill bool > eq_found and not pass eof in the out parameter of do_seek()? eq_found is an optimization that allows us to save a tuple comparison. Not that it really matters for the disk iterator, but still I'd rather keep it. As I said, I agree that retval conventions look ugly in the iterator code. I didn't plan on cleaning it up now, because it doesn't really stand in the way of tuple hints. I'll look into it a bit later.