Re: [PATCH] nodeindexscan with reorder memory leak

Hi!

On Mon, Feb 7, 2022 at 11:42 AM Aliaksandr Kalenik <akalenik(at)kontur(dot)io> wrote:
> Thanks for your review!
>
> On Sun, Jan 30, 2022 at 7:24 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Actually, that code has got worse problems than that. I tried to improve
> > our regression tests to exercise that code path, as attached. What I got
> > was
> >
> > +SELECT point(x,x), (SELECT circle_center(f1) FROM gcircle_tbl ORDER BY f1 <-> p
> > oint(x,x) LIMIT 1) as c FROM generate_series(0,1000,1) x;
> > +ERROR: index returned tuples in wrong order
>
> I tried to figure out what is the problem with this query. This error
> happens when actual distance is less than estimated distance.
> For this specific query it happened while comparing these values:
> 50.263279680219099532223481219262 (actual distance returned by
> dist_cpoint in geo_ops.c) and 50.263279680219113743078196421266
> (bounding box distance returned by computeDistance in gistproc.c).
>
> So for me it looks like this error is not really related to KNN scan
> code but to some floating-arithmetic issue in distance calculation
> functions for geometry type.Would be great to figure out a fix but
> for now I didn’t manage to find a better way than comparing the
> difference of distance with FLT_EPSILON which definitely doesn't seem
> like the way to fix :(

Probably, this is caused by some compiler optimization. Could you
re-check the issue with different compilers and optimization levels?

Regarding the memory leak, could you add a corresponding regression
test to the patch (probably similar to Tom's query upthread)?

------
Regards,
Alexander Korotkov