> On Thu, Sep 5, 2019 at 9:41 PM Alvaro Herrera from 2ndQuadrant <alvherre@alvh.no-ip.org> wrote:
>
> On 2019-Sep-05, Dmitry Dolgov wrote:
>
> > Here is the version in which stepping between the pages works better. It seems
> > sufficient to fix the case you've mentioned before, but for that we need to
> > propagate keepPrev logic through `_bt_steppage` & `_bt_readnextpage`, and I
> > can't say I like this solution. I have an idea that maybe it would be simpler
> > to teach the code after index_skip to not do `_bt_next` right after one skip
> > happened before. It should immediately elliminate several hacks from index skip
> > itself, so I'll try to pursue this idea.
>
> Cool.
Here it is. Since now the code after index_skip knows whether to do
index_getnext or not, it's possible to use unmodified `_bt_readpage` /
`_bt_steppage`. To achieve that there is a flag that indicated whether or not
we were skipping to the current item (I guess it's possible to implement it
without such a flag, but the at the end result looked more ugly as for me). On
the way I've simplified few things, and all the tests we accumulated before are
still passing. I'm almost sure it's possible to implement some parts of the
code more elegant, but don't see yet how.
> I think multiplying two ScanDirections to watch for a negative result is
> pretty ugly:
Probably, but the only alternative I see to check if directions are opposite is
to check that directions come in pairs (back, forth), (forth, back). Is there
an easier way?
> I think "scanstart" needs more documentation, both in the SGML docs as
> well as the code comments surrounding it.
I was able to remove it after another round of simplification.