Re: Patch: Remove all declarations from pg_attribute.h, consolidate BKI scripts

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: John Naylor <jcnaylor(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Patch: Remove all declarations from pg_attribute.h, consolidate BKI scripts
Date: 2009-12-21 01:23:39
Message-ID: 603c8f070912201723y45fd58ccm3ad8e3856222137f@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Dec 20, 2009 at 7:20 PM, John Naylor <jcnaylor(at)gmail(dot)com> wrote:
> Greetings,
>
> Following up on my experimental patch last month to revamp the BKI
> infrastructure, I am proposing a less invasive set of changes with the
> hope of offering something committable. Some of these were discussed
> by Robert Haas and others last summer.
>
> 1. Remove all DATA() declarations from pg_attribute.h, since they are
> easily generated. Introduce a new BKI pseudo-command
> BKI_NAILED_IN_CACHE, which indicates that relcache.c needs a
> Schema_pg_foo declaration for that catalog. Place these declarations
> in a new header schemapg.h. This will reduce the effort to add or
> change critical tables.
>
> 2. Use identical scripts on Posix and Windows systems, using Perl 5.6
> (no CPAN modules needed). The grepping of the catalog headers is done
> by Catalog.pm, which gives the scripts gen_bki.pl and gen_fmgr.pl a
> structured interface to the data. The pg_type info is saved so that
> the relevant fields can be copied into those of pg_attribute.
>
> 3. Make the BKI files, fmgrtab.c, fmgroids.h, and schemapg.h distprep
> targets, so distribution tarballs can still be built without Perl on
> Posix systems.
>
> Feedback on the Makefile changes would be appreciated, since that was
> the hardest part for me. The MSVC changes are untested.

This is really nice. I haven't done a full review, but it seems as if
you've eliminated some of the more controversial aspects of what I did
before, as well as done some good cleanup and refactoring. One minor
nit is that I think we should end up with genbki.pl rather than
gen_bki.pl, in the interest of changing nothing without a good reason.

A more important point is whether we really need to make this
dependent on Perl 5.6 or later. What features are we using here that
actually require Perl 5.6? I suspect the answer is "none, but we
don't like writing the code in a way that is backward compatible to
crufty, ancient versions of Perl", to which my response is "get over
it". :-) I always use the three-argument form of open() in all new
code, but for fixed strings the two-argument form is just as good and
has been supported for far longer. Any suggestions that we should
prefer clean code over portability are, in my opinion, non-starters.

Again, thank you for your work on this!

...Robert

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Stark 2009-12-21 01:37:51 Re: Proposal: Pre ordered aggregates, default ORDER BY clause for aggregates - median support
Previous Message KaiGai Kohei 2009-12-21 00:39:38 Re: Largeobject Access Controls (r2460)