Re: [PATCH] Teach Catalog.pm how many attributes there should be per DATA() line

On Tue, Oct 6, 2015 at 9:15 AM, David Christensen <david@endpoint.com> wrote:
> Fixes a build issue I ran into while adding some columns to system tables:
>
>     Throws a build error if we encounter a different number of fields in a
>     DATA() line than we expect for the catalog in question.
>
>     Previously, it was possible to silently ignore any mismatches at build
>     time which could result in symbol undefined errors at link time.  Now
>     we stop and identify the infringing line as soon as we encounter it,
>     which greatly speeds up the debugging process.

I think this is a GREAT idea, but this line made me laugh[1]:

+        warn "No Natts defined yet, silently skipping check...\n";

I suggest that we make that a fatal error.  Like "Could not find
definition Natts_pg_proc before start of DATA".

Secondly, I don't think we should check this at this point in the
code, because then it blindly affects everybody who uses Catalog.pm.
Let's pick one specific place to do this check.  I suggest genbki.pl,
inside the loop with this comment: "# Ordinary catalog with DATA
line(s)"

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

[1] Because if you're producing a warning, it's not silent!