Re: Assert in pageinspect with NULL pages

On Thu, Feb 17, 2022 at 09:00:20PM -0600, Justin Pryzby wrote:
> BRIN can also crash if passed a non-brin index.
>
> I've been sitting on this one for awhile. Feel free to include it in your
> patchset.

So, I have begun a close lookup of this thread, and the problem you
are reporting here is worth fixing on its own, before we do something
for new pages as reported originally. There is more that caught my
attention than the brin AM not being check properly, because a couple
of code paths are fuzzy with the checks on the page sizes. My
impression of the whole thing is that we'd better generalize the use
of get_page_from_raw(), improving the code on many sides when the user
can provide a raw page in input (also I'd like to think that it is a
better practice anyway as any of those functions may finish to look
8-byte fields, but the current coding would silently break in
alignment-picky environments):
- Some code paths (hash, btree) would trigger elog(ERROR) but we
cannot use that for errors that can be triggered by the user.
- bt_page_items_bytea(), page_header() and fsm_page_contents() were
fuzzy on the page size check, so it was rather easy to generate
garbage with random data.
- page_checksum_internal() used a PageHeader, where I would have
expected a Page.
- More consistency in the error strings, meaning less contents to
translate in the long-term.

This first batch leads me to the attached, with tests to stress all
that for all the functions taking raw pages in input.
--
Michael