From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Inconsistent error message wording for REINDEX CONCURRENTLY
Date: Sun, 05 May 2019 13:32:39 -0400
Message-ID: <16468.1557077559@sss.pgh.pa.us> (raw)
In-Reply-To: <20190505141645.GA7392@paquier.xyz>
References: <23694.1556806002@sss.pgh.pa.us>
<20190504085543.GE2174@paquier.xyz>
<4469.1556982011@sss.pgh.pa.us>
<20190505141645.GA7392@paquier.xyz>
Michael Paquier <michael@paquier.xyz> writes:
> The message you are referring to in index_create() has been introduced
> as of e093dcdd with the introduction of CREATE INDEX CONCURRENTLY, and
> it can be perfectly hit without REINDEX:
> =# show allow_system_table_mods;
> allow_system_table_mods
> -------------------------
> on
> (1 row)
Oh, yeah, if you do that you can get to it.
> What do you think about something like the attached then? HEAD does
> not check after system indexes with REINDEX INDEX CONCURRENTLY, and I
> have moved all the catalog-related tests to reindex_catalog.sql.
OK as far as the wording goes, but now that I look at the specific tests
that are being applied, they seem rather loony, as well as inconsistent
between the two cases. IsSystemRelation *sounds* like the right thing,
but it's not, because it forbids user-relation toast tables which seems
like a restriction we need not make. I think IsCatalogRelation is the
test we actually want there. In the other place, checking
IsSystemNamespace isn't even approximately the correct way to proceed,
since it fails to reject reindexing system catalogs' toast tables.
We should be doing the equivalent of IsCatalogRelation there too.
(It's a bit of a pain that catalog.c doesn't offer a function that
makes that determination from just an OID. Should we add one?
There might be other callers for it.)
I concur that we shouldn't need a separate check for relisshared,
since all shared rels should be system catalogs.
I"m not sure I'd move these error-case tests to reindex_catalog.sql ---
bear in mind that later today, that test is either going away entirely
or at least not getting run by default anymore.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, michael@paquier.xyz, pgsql-hackers@lists.postgresql.org
Subject: Re: Inconsistent error message wording for REINDEX CONCURRENTLY
In-Reply-To: <16468.1557077559@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox