Date: Tue, 10 Dec 2019 14:38:15 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Adam Scott <adam.c.scott@gmail.com>, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16122: segfault pg_detoast_datum (datum=0x0) at fmgr.c:1833
 numrange query
Message-ID: <20191210053815.GI72921@paquier.xyz>
References: <16122-eb35bc248c806c15@postgresql.org>
 <20191118120312.cblcswef6vuqbijq@development>
 <CA+s62-Mnj-x8_ZXh7tBVU3WJ3cKUsUL2HAp2YaJSXQPw1iypFw@mail.gmail.com>
 <20191119014827.GC1614@paquier.xyz>
 <20191119103813.xnsloisoxkwrxoim@development>
 <20191119114056.GA516103@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="iAzLNm1y1mIRgolD"
Content-Disposition: inline
In-Reply-To: <20191119114056.GA516103@paquier.xyz>
Precedence: bulk


--iAzLNm1y1mIRgolD
Content-Type: multipart/mixed; boundary="W/+CTqSGWdiRg+8j"
Content-Disposition: inline


--W/+CTqSGWdiRg+8j
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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?=20
--
Michael

--W/+CTqSGWdiRg+8j
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="rangetype-stat-crash.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/utils/adt/rangetypes_selfuncs.c b/src/backend/util=
s/adt/rangetypes_selfuncs.c
index ab4f86e3fd..abe3f6170d 100644
--- a/src/backend/utils/adt/rangetypes_selfuncs.c
+++ b/src/backend/utils/adt/rangetypes_selfuncs.c
@@ -1045,7 +1045,9 @@ calc_hist_selectivity_contained(TypeCacheEntry *typca=
che,
 	bin_width =3D upper_bin_width;
=20
 	sum_frac =3D 0.0;
-	for (i =3D upper_index; i >=3D 0; i--)
+
+	/* Make sure to not count for the first bin */
+	for (i =3D upper_index - 1; i >=3D 0; i--)
 	{
 		double		dist;
 		double		length_hist_frac;
diff --git a/src/test/regress/expected/rangetypes.out b/src/test/regress/ex=
pected/rangetypes.out
index 6fd16bddd1..7656e5ea2c 100644
--- a/src/test/regress/expected/rangetypes.out
+++ b/src/test/regress/expected/rangetypes.out
@@ -1431,3 +1431,16 @@ create function table_fail(i anyelement) returns tab=
le(i anyelement, r anyrange)
   as $$ select $1, '[1,10]' $$ language sql;
 ERROR:  cannot determine result data type
 DETAIL:  A function returning "anyrange" must have at least one "anyrange"=
 argument.
+-- Corner case with selectivity of range operators, per bug #16122.
+CREATE TABLE rangetab (a numrange);
+INSERT INTO rangetab
+SELECT ('['|| (45.0 - a::numeric/10000000) || ',' ||
+              (45.1 + a::numeric/10000000) || ')')::numrange
+  FROM generate_series(1,1000) as a;
+ANALYZE rangetab;
+SELECT a FROM rangetab WHERE a <@ '[89.9999998611111,90.0000001388889)';
+ a=20
+---
+(0 rows)
+
+DROP TABLE rangetab;
diff --git a/src/test/regress/sql/rangetypes.sql b/src/test/regress/sql/ran=
getypes.sql
index 8960add976..b17e64113c 100644
--- a/src/test/regress/sql/rangetypes.sql
+++ b/src/test/regress/sql/rangetypes.sql
@@ -507,3 +507,13 @@ create function inoutparam_fail(inout i anyelement, ou=
t r anyrange)
 --should fail
 create function table_fail(i anyelement) returns table(i anyelement, r any=
range)
   as $$ select $1, '[1,10]' $$ language sql;
+
+-- Corner case with selectivity of range operators, per bug #16122.
+CREATE TABLE rangetab (a numrange);
+INSERT INTO rangetab
+SELECT ('['|| (45.0 - a::numeric/10000000) || ',' ||
+              (45.1 + a::numeric/10000000) || ')')::numrange
+  FROM generate_series(1,1000) as a;
+ANALYZE rangetab;
+SELECT a FROM rangetab WHERE a <@ '[89.9999998611111,90.0000001388889)';
+DROP TABLE rangetab;

--W/+CTqSGWdiRg+8j--

--iAzLNm1y1mIRgolD
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAl3vL0cACgkQnvQgOdby
QH3OJg/9Gei6TGPs6iZECUg9ajePudyo5hdIH3KF/McTBrWoqPlUCkGromhbkjQl
eio1m+2tsijqj0HA5bx3h/pWDeZmCCUA7IhrGQ4YyeHlAdN17Xq2ZWJzmM1lejLB
0hsGN57bLxdWVa9xM3lIluvqwjcDPWvmKjwIth/JRYahoBc4It5wVekXzpoWJRWf
Ps+wjd/Q69s/taa1qTF+C1U4003hS0doNxNpdBo+S3RPFAa2Bbpiu+RPqHdizE7s
HcVJMdpbT2FpK/1VpMVmMQ+g9fv+NlJrTzE+s7yUwYlDqs/Tjl8FWIoZUGuDRt5V
CRXWbj1lVB1a1y8K75/hWmVPSOLTfFGkI42pryy/vie34RI7f1jFRKiYLhRXFE+W
csm946J2G9B2jk9sjGulQiXiEYKzaIacjh0qXSpohd+kDWIxnlFV90ycfEdmeKKA
bpk5d3AM6vMmH05umg2cosbJC9tZQzXciD482EavXs+XOi5o0711Ua5d3FponBC1
I0QrjR8yg65xE4GXdeTfkHd/M0lPAnSvLB8LMBj4ADQsckJfEN5646Ca4bkoPhJb
TNgi7eLG9BgPz6nrAPRaMOGlJj0bRVjZC4EXn+YgmMpIJuQViB7yIGu6lVb2zCVj
pQr/usOBQpGNAnMbbl+BWHs4Yfim5peomScVis+LdDg39+E4NgQ=
=i8xO
-----END PGP SIGNATURE-----

--iAzLNm1y1mIRgolD--