Re: Documentation for bootstrap data conversion

From: John Naylor <jcnaylor(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Documentation for bootstrap data conversion
Date: 2018-04-19 09:50:39
Message-ID: CAJVSVGUeJmFB3h-NJ18P32NPa+kzC165nm7GSoGHfPaN80Wxcw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 4/18/18, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> John Naylor <jcnaylor(at)gmail(dot)com> writes:
>> and dug through a bit to find cases where 'catalog' is clearly a
>> better term. Most of these are in the pg_*.h/.dat file boilerplate
>> comments, which would be easy enough to change with a script. If we're
>> going to do that, the word order of this phrase now seems a bit
>> awkward:
>> definition of the system "access method" catalog (pg_am)
>> so we may as well go the extra step and do:
>> definition of the "access method" system catalog (pg_am)
>
>> Thoughts?
>
> Makes sense to me --- I suspect that the original phrasing was chosen
> by somebody whose native language wasn't English.
>
>> Beyond that, there are many cases where the language might be
>> debatable, or at least it's not obvious to the casual observer whether
>> it could also be referring to an index or toast table, so it's
>> probably best to leave well enough alone for most cases.
>
> Agreed, there's no need to go overboard here. But let's fix that
> awkward construction in the catalog header files, as well as anyplace
> else where it seems like a clear win. Will you send a patch?

I've attached a patch that mostly touches boilerplate comments in the
headers and data files. I couldn't resist editing some comments for
clarity and consistency.

Not in the patch, but I'll also mention this stanza in tablecmds.c
around line 4370, where the comment doesn't quite match the code and
error message:

/*
* We don't support rewriting of system catalogs; there are too
* many corner cases and too little benefit. In particular this
* is certainly not going to work for mapped catalogs.
*/
if (IsSystemRelation(OldHeap))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot rewrite system relation \"%s\"",
RelationGetRelationName(OldHeap))));

BTW, thanks committing the quoting imrovements.

-John Naylor

Attachment Content-Type Size
0001-s-relation-catalog.patch text/x-patch 54.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2018-04-19 09:58:34 Re: partitioning code reorganization
Previous Message Darafei Komяpa Praliaskouski 2018-04-19 09:11:12 Re: Is a modern build system acceptable for older platforms