Re: BUG #16122: segfault pg_detoast_datum (datum=0x0) at fmgr.c:1833 numrange query

--000000000000510dc9059c0a09cd
Content-Type: text/plain; charset="UTF-8"

Thank you Michael!  I know this is a late response but didn't want to
clutter the message stream until we had confirmation, but I applied this
patch to our server and could not replicate the bug.  So this patch is
working for us presently.

Thanks again Michael,
Adam


On Mon, Dec 9, 2019 at 9:38 PM Michael Paquier <[email protected]> wrote:

> On Tue, Nov 19, 2019 at 08:40:56PM +0900, Michael Paquier wrote:
> > If you add an ANALYZE on the table natica_hdu_test after restoring, I
> > am rather sure that you would reproduce the crash more quickly because
> > the handling around the stats of the column are busted here.  Anyway,
> > taking my example of upthread, I have been also able to reproduce the
> > problem on REL_10_STABLE even with assertions enabled: the trick is
> > that you need to leave once the session after the analyze on the
> > table.  Then a SELECT within a new session is enough to crash the
> > server.
>
> So...  I have looked more at this one, and from my previous example it
> seems that we have a one-off error when looking up at the array
> holding the histograms for ranges (lower and upper bound).
>
> In my previous example, we get to build 101 RangeBounds when beginning
> to calculate the range operator selectivity in
> calc_hist_selectivity().  However, when we get to the point of
> calc_hist_selectivity_contained(), upper_index gets calculated at 100
> which is just at the limit of the indexed bounds, and the code would
> happily look at the last bound as well as the one-after-the-last bound
> as range_cmp_bounds() sees fit, but the latter just points to the
> void.  The code looks wrong since its introduction in 59d0bf9d and
> it seems that the changes done for free_attstatsslot() in 9aab83f make
> the issue more easily reproducible.
>
> A fix like the rough POC attached addresses the issue, but I think
> that's too naive to not count for the first bin in the ranges
> evaluated.  Tomas, you may be more familiar with this area of the code
> than I am.  What do you think?
> --
> Michael
>

--000000000000510dc9059c0a09cd
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>Thank you Michael!=C2=A0 I know this is a late respon=
se but didn&#39;t want to clutter the message stream until we had confirmat=
ion, but I applied this patch to our server and could not replicate the bug=
.=C2=A0 So this patch is working for us presently.</div><div><br></div><div=
>Thanks again Michael,</div><div>Adam</div><div><br></div></div><br><div cl=
ass=3D"gmail_quote"><div dir=3D"ltr" class=3D"gmail_attr">On Mon, Dec 9, 20=
19 at 9:38 PM Michael Paquier &lt;<a href=3D"mailto:[email protected]">mi=
[email protected]</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote"=
 style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);p=
adding-left:1ex">On Tue, Nov 19, 2019 at 08:40:56PM +0900, Michael Paquier =
wrote:<br>
&gt; If you add an ANALYZE on the table natica_hdu_test after restoring, I<=
br>
&gt; am rather sure that you would reproduce the crash more quickly because=
<br>
&gt; the handling around the stats of the column are busted here.=C2=A0 Any=
way,<br>
&gt; taking my example of upthread, I have been also able to reproduce the<=
br>
&gt; problem on REL_10_STABLE even with assertions enabled: the trick is<br=
>
&gt; that you need to leave once the session after the analyze on the<br>
&gt; table.=C2=A0 Then a SELECT within a new session is enough to crash the=
<br>
&gt; server.<br>
<br>
So...=C2=A0 I have looked more at this one, and from my previous example it=
<br>
seems that we have a one-off error when looking up at the array<br>
holding the histograms for ranges (lower and upper bound).<br>
<br>
In my previous example, we get to build 101 RangeBounds when beginning<br>
to calculate the range operator selectivity in<br>
calc_hist_selectivity().=C2=A0 However, when we get to the point of<br>
calc_hist_selectivity_contained(), upper_index gets calculated at 100<br>
which is just at the limit of the indexed bounds, and the code would<br>
happily look at the last bound as well as the one-after-the-last bound<br>
as range_cmp_bounds() sees fit, but the latter just points to the<br>
void.=C2=A0 The code looks wrong since its introduction in 59d0bf9d and<br>
it seems that the changes done for free_attstatsslot() in 9aab83f make<br>
the issue more easily reproducible.<br>
<br>
A fix like the rough POC attached addresses the issue, but I think<br>
that&#39;s too naive to not count for the first bin in the ranges<br>
evaluated.=C2=A0 Tomas, you may be more familiar with this area of the code=
<br>
than I am.=C2=A0 What do you think? <br>
--<br>
Michael<br>
</blockquote></div>

--000000000000510dc9059c0a09cd--