From: Alexander Lakhin <exclusion@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17950: Incorrect memory access in gtsvector_picksplit()
Date: Mon, 4 Sep 2023 17:00:00 +0300
Message-ID: <81a75261-2dec-53f6-40f9-5d3ed13f42ee@gmail.com> (raw)
In-Reply-To: <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>
<ZPV6vblm1jmgs1VW@paquier.xyz>
Hi Mchael,
Thank you for committing the fix!
04.09.2023 09:35, Michael Paquier wrote:
> Regarding the changes in gtsvectorout(), the output produced is indeed
> confusing when ISALLTRUE is set.
>
> - int siglen = GETSIGLEN(key);
> - int cnttrue = (ISALLTRUE(key)) ? SIGLENBIT(siglen) : sizebitvec(GETSIGN(key), siglen);
> + if (ISALLTRUE(key))
> + sprintf(outbuf, "all true bits");
> + else
> + {
> + int siglen = GETSIGLEN(key);
> + int cnttrue = (ISALLTRUE(key)) ? SIGLENBIT(siglen) : sizebitvec(GETSIGN(key), siglen);
>
> - sprintf(outbuf, SINGOUTSTR, cnttrue, (int) SIGLENBIT(siglen) - cnttrue);
> + sprintf(outbuf, SINGOUTSTR, cnttrue, (int) SIGLENBIT(siglen) - cnttrue);
> + }
>
> In the false branch of ISALLTRUE(key), why isn't cnttrue always
> calculated with sizebitvec()?
Yes, that expression should be simpler there. Please look at the v2 attached.
> 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.
I agree. Let's not backpatch it.
Best regards,
Alexander
Attachments:
[text/x-patch] v2-02-fix-gtsvectorout.patch (797B, ../81a75261-2dec-53f6-40f9-5d3ed13f42ee@gmail.com/2-v2-02-fix-gtsvectorout.patch)
download | inline diff:diff --git a/src/backend/utils/adt/tsgistidx.c b/src/backend/utils/adt/tsgistidx.cindex 12a4111e99..273f9b0925 100644--- a/src/backend/utils/adt/tsgistidx.c+++ b/src/backend/utils/adt/tsgistidx.c@@ -115,10 +115,15 @@ gtsvectorout(PG_FUNCTION_ARGS)
sprintf(outbuf, ARROUTSTR, (int) ARRNELEM(key));
else
{
- int siglen = GETSIGLEN(key);- int cnttrue = (ISALLTRUE(key)) ? SIGLENBIT(siglen) : sizebitvec(GETSIGN(key), siglen);+ if (ISALLTRUE(key))+ sprintf(outbuf, "all true bits");+ else+ {+ int siglen = GETSIGLEN(key);+ int cnttrue = sizebitvec(GETSIGN(key), siglen);- sprintf(outbuf, SINGOUTSTR, cnttrue, (int) SIGLENBIT(siglen) - cnttrue);+ sprintf(outbuf, SINGOUTSTR, cnttrue, (int) SIGLENBIT(siglen) - cnttrue);+ }
}
PG_FREE_IF_COPY(key, 0);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: exclusion@gmail.com, michael@paquier.xyz, tgl@sss.pgh.pa.us, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17950: Incorrect memory access in gtsvector_picksplit()
In-Reply-To: <81a75261-2dec-53f6-40f9-5d3ed13f42ee@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox