Re: Local partitioned indexes and pageinspect

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Peter Geoghegan <pg(at)bowt(dot)ie>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Local partitioned indexes and pageinspect
Date: 2018-04-24 04:58:12
Message-ID: 20180424045812.GA15322@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Apr 23, 2018 at 05:29:59PM -0700, Peter Geoghegan wrote:
> It looks like local partitioned indexes are a problem for pageinspect:
>
> pg(at)regression[28736]=# select bt_metap('at_partitioned_b_idx');
> ERROR: relation "at_partitioned_b_idx" is not a btree index

Okay, I can see the point you are making here, in this case that's
actually a btree index but it has no on-disk data, so let's improve the
error handling. At the same time hash and gin functions don't make much
effort either...

> I gather that pageinspect simply didn't get the memo about the new
> RELKIND_PARTITIONED_INDEX "placeholder" relkind. I think that it
> should at least give a friendlier error message than what we see here.
> The same applies to amcheck, and possibly a few other modules.

So that's another c08d82f3... This needs to be really checked on a
yearly-basis as the trend of the last releases is to add at least one
new relkind per major version. There are three modules at stake here:
- pg_visibility
- pgstattuple
- pageinspect
I get to wonder about anything needed for sepgsql... Let's see that on
a separate thread after I look at the problem.

> I also noticed that the new 'I' relkind is not documented within the
> pg_class docs. I think that that needs to be updated.

Good catch.

> The docs should
> explain that 'I' relations are not backed by storage, since that will
> probably affect users of one or two external tools.

Hm, the docs about taking backups with the low-level APIs don't care
much about relkind now:
https://www.postgresql.org/docs/devel/static/continuous-archiving.html#BACKUP-LOWLEVEL-BASE-BACKUP
Or do you have another section in the docs in mind?

Does the attached patch address everything you have seen? I have tried
to cope with anything I noticed as well on the way, which is made of:
- pg_visibility should have tests for partitioned indexes, the code
handles errors correctly.
- in pageinspect, get_raw_page() fails for partitioned indexes:
ERROR: could not open file "base/16384/16419": No such file or directory
- in pgstattuple, error message is incorrect for both index-related and
heap-related functions.

Thanks,
--
Michael

Attachment Content-Type Size
0001-Fix-gaps-in-modules-with-handling-of-partitioned-ind.patch text/x-diff 17.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrey Borodin 2018-04-24 05:04:22 Re: [HACKERS] Clock with Adaptive Replacement
Previous Message Amit Langote 2018-04-24 04:43:21 Re: minor fix for acquire_inherited_sample_rows