Horton

Re: Assert in pageinspect with NULL pages

Daria Lepikhova <d.lepikhova@postgrespro.ru>

View thread View on postgr.es
From: Daria Lepikhova <d.lepikhova@postgrespro.ru>
To: Michael Paquier <michael@paquier.xyz>
Cc: rjuju123@gmail.com, exclusion@gmail.com, pgsql-hackers@postgresql.org
Date: 2022-02-23T07:09:02Z
Lists: pgsql-hackers

Attachments

First of all, thanks for the review and remarks!

18.02.2022 08:02, Michael Paquier пишет:
> On Thu, Feb 17, 2022 at 05:40:41PM +0800, Julien Rouhaud wrote:
>> About the patch, it's incorrectly using a hardcoded 8192 block-size rather than
>> using the computed :block_size (or computing one when it's not already the
>> case).
> Well, the tests of pageinspect fail would already fail when using a
> different page size than 8k, like the checksum ones :)
>
> Anywa, I agree with your point that if this is not a reason to not
> make the tests more portable if we can do easily.

Fixed.

>> I'm also wondering if it wouldn't be better to return NULL rather than throwing
>> an error for uninitialized pages.
> Agreed that this is a sensible choice.  NULL would be helpful for the
> case where one compiles all the checksums of a relation with a full
> scan based on the relation size, for example.  All these behaviors
> ought to be documented properly, as well.  For SRFs, this should just
> return no rows rather than generating an error.

So, I tried to implement this remark. However, further getting rid of 
throwing an error and replacing it with a NULL return led to reproduce 
the problem that Alexander Lakhin mentioned And here I need little more 
time to figure it out.


And one more addition. In the previous version of the patch, I forgot to 
add tests for the gist index, but the described problem is also relevant 
for it.

-- 
Daria Lepikhova
Postgres Professional

Commits

  1. pageinspect: Fix handling of all-zero pages

  2. pageinspect: Add more sanity checks to prevent out-of-bound reads

  3. pageinspect: Fix handling of page sizes and AM types

  4. Teach pageinspect about nbtree deduplication.