From: Alexander Lakhin <exclusion@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-bugs@lists.postgresql.org, Andrey Borodin <amborodin86@gmail.com>
Subject: Re: BUG #17884: gist_page_items() crashes for a non-leaf page of an index with non-key columns
Date: Thu, 18 May 2023 09:00:00 +0300
Message-ID: <457826a7-95a6-96ae-dabc-6e07d8020a7b@gmail.com> (raw)
In-Reply-To: <ZGWjHcBJoOPIbzYY@paquier.xyz>
References: <17884-cb8c326522977acb@postgresql.org>
<92873f8d-a9b1-0ee8-5753-ee4a1664fb0e@gmail.com>
<ZGF/qgICp6JvNDhV@paquier.xyz>
<8a991814-97c0-81d6-1e24-60c26308f005@gmail.com>
<ZGRk3LVBpt9/dFlt@paquier.xyz>
<e4c9897a-6864-4c47-f7ea-08385f2a7db5@gmail.com>
<ZGWEZCN0PpfFona0@paquier.xyz>
<ZGWjHcBJoOPIbzYY@paquier.xyz>
18.05.2023 07:01, Michael Paquier wrote:
> On Thu, May 18, 2023 at 10:50:28AM +0900, Michael Paquier wrote:
>> I have expanded the tests to show how this applies for more data
>> types, like point or integers. Any thoughts about the v5 attached?
I like this version and think it's ready to launch.
Thank you!
I would suggest just two changes:
more parenthesis
->
more parentheses
(isn't this the plural form?)
Non-leaf pages have only the key attributes, and leaf pages have
the included attributes.
->
Non-leaf pages contain only the key attributes, and leaf pages contain
the included attributes.
(To avoid misreading as if there are some attributes of the pages.)
> Two extra things that I had in mind, as long as I don't forget about
> them.. We could have a logic closer to record_out(), and grab a copy
> of it for this specific function (hstore does that, as one example),
> but I cannot really get into this approach, it just does not seem
> worth the complications compared to the use cases.
Yeah, hstore_from_record() doesn't look simpler. For the moment, I see no
reasons to use that approach.
> Another thing would be to use separate bracket types to the groups of
> values while still using parenthesis for the whole set of key and
> included values, like:
> (a1, a2) INCLUDE (a3, a4) = ([val1], [val2]) INCLUDE ([val3], [val4])
>
> But I am not sure that we need this much complication, either.
> Compared to the point of showing all the values from the GiST tuples,
> tweaking the style is not interesting as pageinspect is for advanced
> users. More opinions or ideas are welcome, of course.
Yes, I had thought about adding "{}", too, but decided that this would
arguably improve reading, but inarguably raise formalization questions.
So I would leave "()" as done in v5.
Best regards,
Alexander
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, amborodin86@gmail.com
Subject: Re: BUG #17884: gist_page_items() crashes for a non-leaf page of an index with non-key columns
In-Reply-To: <457826a7-95a6-96ae-dabc-6e07d8020a7b@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