Re: Local partitioned indexes and pageinspect

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Peter Geoghegan <pg(at)bowt(dot)ie>, 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-05-02 02:05:33
Message-ID: 20180502020533.GA2114@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2018-05-02 02:30:25 doc fixes: vacuum_cleanup_index_scale_factor
Previous Message Amit Langote 2018-05-02 01:40:11 Re: stats_ext test fails with -DCATCACHE_FORCE_RELEASE