Re: Do we need use more meaningful variables to replace 0 in catalog head files?

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Magnus Hagander <magnus(at)hagander(dot)net>, Jan de Visser <jan(at)de-visser(dot)net>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Hao Lee <mixtrue(at)gmail(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: Do we need use more meaningful variables to replace 0 in catalog head files?
Date: 2016-11-14 20:12:24
Message-ID: f102c3ae-6f10-1d61-0eb4-b5582f6aff12@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 11/13/2016 11:11 AM, Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> I'm not convinced the line prefix part is necessary, though. What I'm
>> thinking of is something like this:
>> PROCDATA( oid=1242 name=boolin isstrict=t volatile=i parallel=s nargs=1
>> rettype=bool argtypes="cstring" src=boolin );
> We could go in that direction too, but the apparent flexibility to split
> entries into multiple lines is an illusion, at least if you try to go
> beyond a few lines; you'd end up with duplicated line sequences in
> different entries and thus ambiguity for patch(1). I don't have any
> big objection to the above, but it's not obviously better either.

Yeah, I looked and there are too many cases where the name would be
outside the normal 3 lines of context.

>
> Some things we should try to resolve before settling definitively on
> a data representation:
>
> 1. Are we going to try to keep these things in the .h files, or split
> them out? I'd like to get them out, as that eliminates both the need
> to keep the things looking like macro calls, and the need for the data
> within the macro call to be at least minimally parsable as C.

That would work fine for pg_proc.h, less so for pg_type.h where we have
a whole lot of

#define FOOOID nn

directives in among the data lines. Moving these somewhere remote from
the catalog lines they relate to seems like quite a bad idea.

>
> 2. Andrew's example above implies some sort of mapping between the
> keywords and the actual column names (or at least column positions).
> Where and how is that specified?

There are several possibilities. The one I was leaning towards was to
parse out the Anum_pg_foo_* definitions.

>
> 3. Also where are we going to provide the per-column default values?
> How does the converter script know which columns to convert to type oids,
> proc oids, etc? Is it going to do any data validation beyond that, and
> if so on what basis?

a) something like DATA_DEFAULTS( foo=bar );
b) something like DATA_TYPECONV ( rettype argtypes allargtypes );

Hadn't thought about procoids, but something similar.

>
> 4. What will we do about the #define's that some of the .h files provide
> for (some of) their object OIDs? I assume that we want to move in the
> direction of autogenerating those macros a la fmgroids.h, but this needs
> a concrete spec as well. If we don't want this change to result in a big
> hit to the source code, we're probably going to need to be able to specify
> the macro names to generate in the data files.

Yeah, as I noted above it's a bit messy,

>
> 5. One of the requirements that was mentioned in previous discussions
> was to make it easier to add new columns to catalogs. This format
> does that only to the extent that you don't have to touch entries that
> can use the default value for such a column. Is that good enough, and
> if not, what might we be able to do to make it better?

I think it is good enough, at least for a first cut.

>
>> I'd actually like to roll up the DESCR lines in pg_proc.h into this too,
>> they strike me as a bit of a wart. But I'm flexible on that.
> +1, if we can come up with a better syntax. This together with the
> OID-macro issue suggests that there will be items in each data entry that
> correspond to something other than columns of the target catalog. But
> that seems fine.
>
>> If we can generalize this to other catalogs, then that will be good, but
>> my inclination is to handle the elephant in the room (pg_proc.h) and
>> worry about the gnats later.
> I think we want to do them all. pg_proc.h is actually one of the easier
> catalogs to work on presently, IMO, because the only kind of
> cross-references it has are type OIDs. Things like pg_amop are a mess.
> And I really don't want to be dealing with multiple notations for catalog
> data. Also I think this will be subject to Polya's paradox: designing a
> general solution will be easier and cleaner than a hack that works only
> for one catalog.

I don't know that we need to handle everything at once, as long as the
solution is sufficiently general.

cheers

andrew

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2016-11-14 20:14:10 Re: Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.
Previous Message Peter Geoghegan 2016-11-14 18:21:53 Re: Pinning a buffer in TupleTableSlot is unnecessary