Re: BUG #17950: Incorrect memory access in gtsvector_picksplit()

From: Alexander Lakhin <exclusion(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #17950: Incorrect memory access in gtsvector_picksplit()
Date: 2023-09-04 14:00:00
Message-ID: 81a75261-2dec-53f6-40f9-5d3ed13f42ee@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

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

Attachment Content-Type Size
v2-02-fix-gtsvectorout.patch text/x-patch 797 bytes

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message PG Bug reporting form 2023-09-04 14:36:54 BUG #18082: coredump during initdb
Previous Message PG Bug reporting form 2023-09-04 09:12:06 BUG #18081: Spurious "function with OID ###### does not exist" error