Feedback-ID: i0fe9450f:Fastmail
Date: Mon, 4 Sep 2023 15:35:41 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Alexander Lakhin <exclusion@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17950: Incorrect memory access in gtsvector_picksplit()
Message-ID: <ZPV6vblm1jmgs1VW@paquier.xyz>
References: <17950-6c80a8d2b94ec695@postgresql.org>
 <da453149-fa9e-67fc-fbce-80cca399bce5@gmail.com>
 <e2f36991-0519-bfcf-1da9-c31ca645b4ec@gmail.com>
 <665394.1691937343@sss.pgh.pa.us>
 <09fec706-0691-a131-5c6c-3e64ecc134fc@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="1SlbNh5YvMk9I8ex"
Content-Disposition: inline
In-Reply-To: <09fec706-0691-a131-5c6c-3e64ecc134fc@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ZPV6vblm1jmgs1VW%40paquier.xyz>
Precedence: bulk


--1SlbNh5YvMk9I8ex
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sun, Aug 13, 2023 at 09:00:01PM +0300, Alexander Lakhin wrote:
> I've tried to make that addition as valuable as possible in the context of
> this issue, but as it is too expensive (to be honest, I was bolstered by
> the gin test duration (~10 secs too)), then let's leave it aside.

I have applied 0001 down to 11 to get the basic fix in place, but the
regression tests are really too expensive compared to the value they
bring.=20

Regarding the changes in gtsvectorout(), the output produced is indeed
confusing when ISALLTRUE is set.

-        int            siglen =3D GETSIGLEN(key);
-        int            cnttrue =3D (ISALLTRUE(key)) ? SIGLENBIT(siglen) : =
sizebitvec(GETSIGN(key), siglen);
+        if (ISALLTRUE(key))
+            sprintf(outbuf, "all true bits");
+        else
+        {
+            int            siglen =3D GETSIGLEN(key);
+            int            cnttrue =3D (ISALLTRUE(key)) ? SIGLENBIT(siglen=
) : sizebitvec(GETSIGN(key), siglen);
=20
-        sprintf(outbuf, SINGOUTSTR, cnttrue, (int) SIGLENBIT(siglen) - cnt=
true);
+            sprintf(outbuf, SINGOUTSTR, cnttrue, (int) SIGLENBIT(siglen) -=
 cnttrue);
+        }

In the false branch of ISALLTRUE(key), why isn't cnttrue always
calculated with sizebitvec()?  It's also not something I would
backpatch.  That's confusing, for sure, but there is also the argument
of keeping a consistent output in the stable branches.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmT1er0ACgkQnvQgOdby
QH3qdg/+I/40Hx72FzqiK/PvZZh3ZdNB3lNeOeYmJZSVokHJIbBz2KMqLaiPIV37
rl2k6K9VDiHj4w+vMSI4N12dh1vo8DRZSvGpF1HiEwFCc2hY6m9+AZi2hzHIxxqH
+hnRfika2NWZ6idJorXzj0MzNVpJQZBbp0Rtq4HUvFM2gSo+bG1KVhD+SCXCxfWz
RgsDw1cGAx+/yxOXSlRZh6lAI5gDF6UR2XfPmY9IS2LsDi3yRzYe4OiMNHt5T4x7
dh9CGqvz57dBtt1/+URDNBjNh2U7xgXp6klNOldS8OCsEtHptN/6zo3AUFTDTiSr
UG/vGzuL6NXKDg0Ms+Qjp8Jo2e0dMpI22UpKX6/CGnHRxJEFBDeYlpK5kNVrRIok
X5Ur9PQKSuiYqXl94DPZe/bjZHSVhYNJUFMWWeI1jvQauwqRC5KSqLi42RA8a8ad
X0qNhtfnGw1at/ZDFaBCYVesDrdAMLwl0ROvc/u6puegyQS61hvSjfkprkTICiLX
oQcWVCUn1gTbfFkL7J+rSSjsSWK36/a/Cs7sbWBZJ1Io1faj2N5N+PE0/SGFk5bU
4cF6U/wmiV/E499dM4DRyzLc89i1M0kCCEYH3h+DajBf3q47GSH9e8H4/GGAD/EK
L0urdS4HRs/1GoRKbgRTYAh3iK5E9gMVU/sLDKi2zEsKzmIEkb4=
=iz4q
-----END PGP SIGNATURE-----

--1SlbNh5YvMk9I8ex--