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

2017-03-10 Thread Dagfinn Ilmari Mannsåker
Jeff Janes  writes:

> On Thu, Mar 9, 2017 at 3:20 PM, Robert Haas  wrote:
>
>> Committed.   Hopefully this doesn't contain any Perl bits that are
>> sufficiently new as to cause problems for our older BF members ... I
>> guess we'll see.
>
> Bad luck there.  I'm getting this error on CentOS6.8, perl v5.10.1
>
> Can't locate object method "input_line_number" via package "IO::Handle" at
[...]
> I think we can just save $. and use that, as in the attached.

Another option would have been to add an explicit 'use IO::Handle;',
which makes the OO interface available on filehandles on all versions
back to 5.8 (5.14 and newer do this automatically).

- ilmari

-- 
"I use RMS as a guide in the same way that a boat captain would use
 a lighthouse.  It's good to know where it is, but you generally
 don't want to find yourself in the same spot." - Tollef Fog Heen


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2017-03-09 Thread Robert Haas
On Thu, Mar 9, 2017 at 8:47 PM, Tom Lane  wrote:
> Amit Langote  writes:
>> On 2017/03/10 9:14, Jeff Janes wrote:
>>> I think we can just save $. and use that, as in the attached.
>
>> The patch works for me.
>
> Me too.  Pushed; we'll soon see if that makes the oldest buildfarm
> critters happy.

$. has been around since the stone age (a.k.a. when I was a teenager)
so we should be fine there.  I hope.  That whole input_line_number
thing was making me a bit nervous, but I couldn't find any reference
to when that had been added in a quick Google search, which gave me
hope that it was old enough that we'd be OK.

Sorry for the hassle.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2017-03-09 Thread Tom Lane
Amit Langote  writes:
> On 2017/03/10 9:14, Jeff Janes wrote:
>> I think we can just save $. and use that, as in the attached.

> The patch works for me.

Me too.  Pushed; we'll soon see if that makes the oldest buildfarm
critters happy.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2017-03-09 Thread Amit Langote
On 2017/03/10 9:14, Jeff Janes wrote:
> On Thu, Mar 9, 2017 at 3:20 PM, Robert Haas  > wrote:
> 
> On Mon, Mar 6, 2017 at 11:37 AM, Dagfinn Ilmari Mannsåker
> > wrote:
> > David Christensen > 
> writes:
> >>> Hi David,
> >>>
> >>> Here's a review of your patch.
> >>
> >> Hi Ilmari, thanks for your time and review.  I’m fine with the revised 
> version.
> >
> > Okay, I've marked the patch as Ready For Committer.
> 
> Committed.   Hopefully this doesn't contain any Perl bits that are
> sufficiently new as to cause problems for our older BF members ... I
> guess we'll see.
> 
> 
> 
> Bad luck there.  I'm getting this error on CentOS6.8, perl v5.10.1
>
> Can't locate object method "input_line_number" via package "IO::Handle" at
> ../../../src/backend/catalog/Catalog.pm line 82,  line 148.
> make[3]: *** [fmgrtab.c] Error 25
> make[2]: *** [utils/fmgroids.h] Error 2
> make[2]: *** Waiting for unfinished jobs
> Can't locate object method "input_line_number" via package "IO::Handle" at
> ../../../src/backend/catalog/Catalog.pm line 82,  line 148.
> make[3]: *** [postgres.bki] Error 25
> make[2]: *** [submake-schemapg] Error 2
> make[1]: *** [all-backend-recurse] Error 2
> make: *** [all-src-recurse] Error 2

Same here.  Some buildfarm animals failed too.

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dromedary=2017-03-10%2000%3A55%3A42

> I think we can just save $. and use that, as in the attached.

The patch works for me.

Thanks,
Amit




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2017-03-09 Thread Jeff Janes
On Thu, Mar 9, 2017 at 3:20 PM, Robert Haas  wrote:

> On Mon, Mar 6, 2017 at 11:37 AM, Dagfinn Ilmari Mannsåker
>  wrote:
> > David Christensen  writes:
> >>> Hi David,
> >>>
> >>> Here's a review of your patch.
> >>
> >> Hi Ilmari, thanks for your time and review.  I’m fine with the revised
> version.
> >
> > Okay, I've marked the patch as Ready For Committer.
>
> Committed.   Hopefully this doesn't contain any Perl bits that are
> sufficiently new as to cause problems for our older BF members ... I
> guess we'll see.
>


Bad luck there.  I'm getting this error on CentOS6.8, perl v5.10.1

Can't locate object method "input_line_number" via package "IO::Handle" at
../../../src/backend/catalog/Catalog.pm line 82,  line 148.
make[3]: *** [fmgrtab.c] Error 25
make[2]: *** [utils/fmgroids.h] Error 2
make[2]: *** Waiting for unfinished jobs
Can't locate object method "input_line_number" via package "IO::Handle" at
../../../src/backend/catalog/Catalog.pm line 82,  line 148.
make[3]: *** [postgres.bki] Error 25
make[2]: *** [submake-schemapg] Error 2
make[1]: *** [all-backend-recurse] Error 2
make: *** [all-src-recurse] Error 2


I think we can just save $. and use that, as in the attached.


I as sabotaged a random line in src/include/catalog/pg_amop.h and it seems
to report the error correctly.

Cheers,

Jeff
diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
new file mode 100644
index 767a2ec..cb5fcc8
*** a/src/backend/catalog/Catalog.pm
--- b/src/backend/catalog/Catalog.pm
*** sub Catalogs
*** 65,70 
--- 65,71 
$_ .= $next_line;
redo;
}
+   my $input_line_number=$.;
  
# Strip useless whitespace and trailing semicolons.
chomp;
*** sub Catalogs
*** 80,86 
elsif 
(/^DATA\(insert(\s+OID\s+=\s+(\d+))?\s+\(\s*(.*)\s*\)\s*\)$/)
{
check_natts($filename, $catalog{natts}, $3,
!   $input_file, 
INPUT_FILE->input_line_number);
  
push @{ $catalog{data} }, { oid => $2, 
bki_values => $3 };
}
--- 81,87 
elsif 
(/^DATA\(insert(\s+OID\s+=\s+(\d+))?\s+\(\s*(.*)\s*\)\s*\)$/)
{
check_natts($filename, $catalog{natts}, $3,
!   $input_file, 
$input_line_number);
  
push @{ $catalog{data} }, { oid => $2, 
bki_values => $3 };
}

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2017-03-09 Thread Robert Haas
On Mon, Mar 6, 2017 at 11:37 AM, Dagfinn Ilmari Mannsåker
 wrote:
> David Christensen  writes:
>>> Hi David,
>>>
>>> Here's a review of your patch.
>>
>> Hi Ilmari, thanks for your time and review.  I’m fine with the revised 
>> version.
>
> Okay, I've marked the patch as Ready For Committer.

Committed.   Hopefully this doesn't contain any Perl bits that are
sufficiently new as to cause problems for our older BF members ... I
guess we'll see.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2017-03-06 Thread Dagfinn Ilmari Mannsåker
David Christensen  writes:

>> Hi David,
>> 
>> Here's a review of your patch.
>
> Hi Ilmari, thanks for your time and review.  I’m fine with the revised 
> version.

Okay, I've marked the patch as Ready For Committer.

Thanks,

Ilmari

-- 
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
 the consequences of."  -- Skud's Meta-Law


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2017-03-06 Thread Dagfinn Ilmari Mannsåker
Hi David,

Here's a review of your patch.

David Christensen  writes:

> Throws a build error if we encounter a different number of fields in a
> DATA() line than we expect for the catalog in question.

The patch is a good idea, and as-is implements the suggested feature.
Tested by removing an attribute from a line in catalog/pg_proc.h and
observing the expected failure.  With the attribute in place, it builds and
passes make check-world.

Detailed code review:

[…]
> diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
[…]
> + check_natts($filename, $catalog{natts},$3) or
> +   die sprintf "Wrong number of Natts in DATA() 
> line %s:%d\n", $input_file,INPUT_FILE->input_line_number;

Including the expected/actual number of attributes in the error message
would be nice.  In fact, the 'die' could be moved into check_natts(),
where the actual number is already available, and would clutter the main
loop less.

> + unless ($natts)
> + {
> + die "Could not find definition for Natts_${catname} before 
> start of DATA()\n";
> + }

More idiomatically written as:

die 
  unless $natts;

> diff --git a/src/backend/utils/Gen_fmgrtab.pl 
> b/src/backend/utils/Gen_fmgrtab.pl

The changes to this file are redundant, since it calls Catalog::Catalogs(),
which already checks that the number of attributes matches.

Attached is a suggested revised patch.

- ilmari

-- 
- Twitter seems more influential [than blogs] in the 'gets reported in
  the mainstream press' sense at least.   - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
  to a mainstream media article.  - Calle Dybedahl

>From b78b281983e7a0406c15461340391c21d6cddef9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= 
Date: Mon, 6 Mar 2017 14:51:50 +
Subject: [PATCH] Teach Catalog.pm how many attributes there should be per
 DATA() line

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.
---
 src/backend/catalog/Catalog.pm | 28 +++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index e1f3c3a5ee..85a537ad95 100644
--- a/src/backend/catalog/Catalog.pm
+++ b/src/backend/catalog/Catalog.pm
@@ -46,6 +46,9 @@ sub Catalogs
 
 		open(INPUT_FILE, '<', $input_file) || die "$input_file: $!";
 
+		my ($filename) = ($input_file =~ m/(\w+)\.h$/);
+		my $natts_pat = "Natts_$filename";
+
 		# Scan the input file.
 		while ()
 		{
@@ -70,8 +73,15 @@ sub Catalogs
 			s/\s+/ /g;
 
 			# Push the data into the appropriate data structure.
-			if (/^DATA\(insert(\s+OID\s+=\s+(\d+))?\s+\(\s*(.*)\s*\)\s*\)$/)
+			if (/$natts_pat\s+(\d+)/)
+			{
+$catalog{natts} = $1;
+			}
+			elsif (/^DATA\(insert(\s+OID\s+=\s+(\d+))?\s+\(\s*(.*)\s*\)\s*\)$/)
 			{
+check_natts($filename, $catalog{natts}, $3,
+			$input_file, INPUT_FILE->input_line_number);
+
 push @{ $catalog{data} }, { oid => $2, bki_values => $3 };
 			}
 			elsif (/^DESCR\(\"(.*)\"\)$/)
@@ -216,4 +226,20 @@ sub RenameTempFile
 	rename($temp_name, $final_name) || die "rename: $temp_name: $!";
 }
 
+# verify the number of fields in the passed-in bki structure
+sub check_natts
+{
+	my ($catname, $natts, $bki_val, $file, $line) = @_;
+	die "Could not find definition for Natts_${catname} before start of DATA() in $file\n"
+		unless defined $natts;
+
+	# we're working with a copy and need to count the fields only, so collapse
+	$bki_val =~ s/"[^"]*?"/xxx/g;
+	my @atts = split /\s+/ => $bki_val;
+
+	die sprintf
+		"Wrong number of attributes in DATA() entry at %s:%d (expected %d but got %d)\n",
+		$file, $line, $natts, scalar @atts
+	  unless $natts == @atts;
+}
 1;
-- 
2.11.0


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2017-02-17 Thread Peter Eisentraut
On 2/15/17 10:40, David Christensen wrote:
> 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 would be useful.  I haven't reviewed the code, but the
general idea looks reasonable.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2017-02-15 Thread David Christensen
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.
---
 src/backend/catalog/Catalog.pm   | 26 +-
 src/backend/utils/Gen_fmgrtab.pl | 18 --
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index e1f3c3a..86f5b59 100644
--- a/src/backend/catalog/Catalog.pm
+++ b/src/backend/catalog/Catalog.pm
@@ -46,6 +46,9 @@ sub Catalogs
 
open(INPUT_FILE, '<', $input_file) || die "$input_file: $!";
 
+   my ($filename) = ($input_file =~ m/(\w+)\.h$/);
+   my $natts_pat = "Natts_$filename";
+
# Scan the input file.
while ()
{
@@ -70,8 +73,15 @@ sub Catalogs
s/\s+/ /g;
 
# Push the data into the appropriate data structure.
-   if 
(/^DATA\(insert(\s+OID\s+=\s+(\d+))?\s+\(\s*(.*)\s*\)\s*\)$/)
+   if (/$natts_pat\s+(\d+)/)
+   {
+   $catalog{natts} = $1;
+   }
+   elsif 
(/^DATA\(insert(\s+OID\s+=\s+(\d+))?\s+\(\s*(.*)\s*\)\s*\)$/)
{
+   check_natts($filename, $catalog{natts},$3) or
+ die sprintf "Wrong number of Natts in DATA() 
line %s:%d\n", $input_file, INPUT_FILE->input_line_number;
+
push @{ $catalog{data} }, { oid => $2, 
bki_values => $3 };
}
elsif (/^DESCR\(\"(.*)\"\)$/)
@@ -216,4 +226,18 @@ sub RenameTempFile
rename($temp_name, $final_name) || die "rename: $temp_name: $!";
 }
 
+# verify the number of fields in the passed-in bki structure
+sub check_natts
+{
+   my ($catname, $natts, $bki_val) = @_;
+   unless ($natts)
+   {
+   die "Could not find definition for Natts_${catname} before 
start of DATA()\n";
+   }
+
+   # we're working with a copy and need to count the fields only, so 
collapse
+   $bki_val =~ s/"[^"]*?"/xxx/g;
+
+   return (split /\s+/ => $bki_val) == $natts;
+}
 1;
diff --git a/src/backend/utils/Gen_fmgrtab.pl b/src/backend/utils/Gen_fmgrtab.pl
index cdd603a..49a5d80 100644
--- a/src/backend/utils/Gen_fmgrtab.pl
+++ b/src/backend/utils/Gen_fmgrtab.pl
@@ -56,9 +56,11 @@ foreach my $column (@{ $catalogs->{pg_proc}->{columns} })
 }
 
 my $data = $catalogs->{pg_proc}->{data};
+my $natts = $catalogs->{pg_proc}->{natts};
+my $elem = 0;
+
 foreach my $row (@$data)
 {
-
# To construct fmgroids.h and fmgrtab.c, we need to inspect some
# of the individual data fields.  Just splitting on whitespace
# won't work, because some quoted fields might contain internal
@@ -67,7 +69,19 @@ foreach my $row (@$data)
# fields that might need quoting, so this simple hack is
# sufficient.
$row->{bki_values} =~ s/"[^"]*"/"xxx"/g;
-   @{$row}{@attnames} = split /\s+/, $row->{bki_values};
+my @bki_values = split /\s+/, $row->{bki_values};
+
+# verify we've got the expected number of data fields
+if (@bki_values != $natts)
+{
+die sprintf <

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

2015-10-09 Thread David Christensen

> On Oct 9, 2015, at 2:17 PM, Robert Haas  wrote:
> 
> On Thu, Oct 8, 2015 at 12:43 PM, David Christensen  wrote:
>> I’m happy to move it around, but If everything is in order, how will this 
>> affect things at all?  If we’re in a good state this condition should never 
>> trigger.
> 
> Right, but I think it ought to be Catalog.pm's job to parse the config
> file.  The job of complaining about what it contains is a job worth
> doing, but it's not the same job.  Personally, I hate it when parsers
> take it upon themselves to do semantic analysis, because then what
> happens if you want to write, say, a tool to repair a broken file?
> You need to be able to read it in without erroring out so that you can
> frob whatever's busted and write it back out, and the parser is
> getting in your way.  Maybe that's not going to come up here, but I'm
> just explaining my general philosophy on these things…

Not disagreeing with you in general, but this is a very specific use case and I 
think we lose the niceness of being able to tie back to the specific line 
number for the file in question—the alternative being to track that information 
as well in a separate structure which we then pass around, which seems like 
overkill.

The only two consumers of the catalog-specific data lines (at least via direct 
access in Perl) are genbki.pl and Gen_fmgtab.pl.  We would need to add these 
checks anyway in both call sites, so to me it seems important to bail early if 
we see any issues, so I think putting the failure as soon as we notice it with 
as much context to fix it (i.e., as written) is the right choice.  We can 
certainly pretty up the messages.

The consistency of the system catalogs in the development state is something 
that is fundamental to whether there is any information that is sensible to 
query, and by definition if we are missing columns in the data rows this is a 
mistake and whatever parsed data in here will be worse than useless (as who 
knows the order of the missing column, data can/will end up being misaligned).  
Thus I don’t believe that we’d want other (hypothetical) Catalog.pm consumers 
to try to use data that we know is bad.
--
David Christensen
PostgreSQL Team Manager
End Point Corporation
da...@endpoint.com
785-727-1171







-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2015-10-09 Thread Robert Haas
On Thu, Oct 8, 2015 at 12:43 PM, David Christensen  wrote:
> I’m happy to move it around, but If everything is in order, how will this 
> affect things at all?  If we’re in a good state this condition should never 
> trigger.

Right, but I think it ought to be Catalog.pm's job to parse the config
file.  The job of complaining about what it contains is a job worth
doing, but it's not the same job.  Personally, I hate it when parsers
take it upon themselves to do semantic analysis, because then what
happens if you want to write, say, a tool to repair a broken file?
You need to be able to read it in without erroring out so that you can
frob whatever's busted and write it back out, and the parser is
getting in your way.  Maybe that's not going to come up here, but I'm
just explaining my general philosophy on these things...

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2015-10-08 Thread Robert Haas
On Tue, Oct 6, 2015 at 9:15 AM, David Christensen  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!


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2015-10-08 Thread David Christensen

> On Oct 8, 2015, at 11:23 AM, Robert Haas  wrote:
> 
> On Tue, Oct 6, 2015 at 9:15 AM, David Christensen  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”.

That’s fine with me; my main consideration was to make sure nothing broke in 
the status quo, including dependencies I was unaware of.

> 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)"

I’m happy to move it around, but If everything is in order, how will this 
affect things at all?  If we’re in a good state this condition should never 
trigger.
--
David Christensen
PostgreSQL Team Manager
End Point Corporation
da...@endpoint.com
785-727-1171







-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2015-10-06 Thread David Christensen
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.



0001-Teach-Catalog.pm-how-many-attributes-there-should-be.patch
Description: Binary data



--
David Christensen
PostgreSQL Team Manager
End Point Corporation
da...@endpoint.com
785-727-1171






-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers