[tarantool-patches] Re: [PATCH 3/6] vinyl: cache iterator: consolidate curr_stmt updates

[Vladimir Davydov]

On Thu, Mar 28, 2019 at 05:29:48PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev at gmail.com> [19/03/26 18:56]:
> > Currently, vy_cache_iterator->curr_stmt is updated by top-level iterator
> > functions - next, skip, restore - which results in code duplication and
> > spreads core logic among multiple places. To reduce the amount of code
> > and make it generally easier to follow, this patch moves the updates to
> > low level functions - step, seek. It also makes the seek method return
> > the stop flag, which makes it similar to step, thus making the code more
> > consistent.
> 
> OK to push. As a nit, I would pass stop as an out parameter, not
> in return value. We may want to use the return value to indicate
> an error sometime.

Hmm, I just wanted to make it look like vy_cache_iterator_step,
but I guess you're right - we'd better avoid using retval as stop
indicator. I'll either amend this patch or prepare a separate patch
to clean up retvals across all iterators.