Re: Local partitioned indexes and pageinspect

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Peter Geoghegan <pg@bowt.ie>, Alvaro Herrera <alvherre@2ndquadrant.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2018-05-02T02:05:33Z
Lists: pgsql-hackers

Attachments

On Tue, May 01, 2018 at 12:30:44PM -0400, Robert Haas wrote:
> That's probably going to cause some translation problems.  The form of
> "a" that you need will vary: "a" vs. "an" in English, "un" vs. "una"
> in Spanish, etc.  And it wouldn't be surprising if there are problems
> in some languages even if you make it "This relation is %s".  There's
> a reason why the documentation advises against building up
> translatable strings from constituent parts.  If we go this route,
> it's probably best to separately translate "This relation is a
> table.", "This relation is an index.", etc.

Yeah, I get the feeling that this is not going to be much
consistent-proof either, so while I have not been able to come up with a
better idea, let's not go through this route.

> However, backing up a minute, I don't think "relation \"%s\" is not a
> btree index" is such a terrible message.  These modules are intended
> to be intended by people who Know What They Are Doing.  If we do want
> to change the message, I submit that the only thing that makes it a
> little unclear is that a user might fail to realize that a partitioned
> index is not an index.  But that could be fixed just by adding a
> separate message for that one case (index \"%s\" is partitioned) and
> sticking with the existing message for other cases.

I have been chewing on that, and I come to agree that there is perhaps
little point to complicate the code as long as a failure is properly
reported to the user.  I propose hence the attached, which adds test
cases in the contrib module set for partitioned indexes (amcheck also
lacked tests for partition tables and indexes), and fixes a set of code
paths to be consistent with the presence of this new relkind.

Visibly I have found a bug in git, format-patch reports the following
line in the stats:
 .../pg_visibility/expected/pg_visibility.out    | 13 ++++++++++++-
But the patch contents are actually correct...
--
Michael

Commits

  1. pgstatindex, pageinspect: handle partitioned indexes

  2. Add relkind 'I' to catalog.sgml's list relkinds