Improve error detection/reporting in Catalog.pm and genbki.pl.

Clean up error messages relating to mistakes in .dat files: make sure they
provide the .dat file name and line number, not the place in the Perl
script that's reporting the problem.  Adopt more uniform message phrasing,
too.

Make genbki.pl spit up on unrecognized field names in the input hashes.
Previously, it just silently ignored such fields, which could make a
misspelled field name into a very hard-to-decipher problem.  (This is in
genbki.pl, *not* Catalog.pm, because we don't want reformat_dat_file.pl to
complain about unrecognized fields.  We'd rather it silently dropped them,
to facilitate removing unwanted fields after a reorganization.)

diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index 0b9c05e2317259249cc229a04985de975d08c616..6305a2b36290a181c822db1ec864584bc507834a 100644 (file)
--- a/src/backend/catalog/Catalog.pm
+++ b/src/backend/catalog/Catalog.pm
@@ -226,7 +226,7 @@ sub ParseData
 
        open(my $ifd, '<', $input_file) || die "$input_file: $!";
        $input_file =~ /(\w+)\.dat$/
-         or die "Input file needs to be a .dat file.\n";
+         or die "Input file $input_file needs to be a .dat file.\n";
        my $catname = $1;
        my $data = [];
 
@@ -245,7 +245,9 @@ sub ParseData
 
                        # Quick hack to detect when we have a full hash ref to
                        # parse. We can't just use a regex because of values in
-                       # pg_aggregate and pg_proc like '{0,0}'.
+                       # pg_aggregate and pg_proc like '{0,0}'.  This will need
+                       # work if we ever need to allow unbalanced braces within
+                       # a field value.
                        my $lcnt = tr/{//;
                        my $rcnt = tr/}//;
 
@@ -254,29 +256,30 @@ sub ParseData
                                eval '$hash_ref = ' . $_;
                                if (!ref $hash_ref)
                                {
-                                       die "Error parsing $_\n$!";
+                                       die "$input_file: error parsing line $.:\n$_\n";
                                }
 
+                               # Annotate each hash with the source line number.
+                               $hash_ref->{line_number} = $.;
+
                                # Expand tuples to their full representation.
                                AddDefaultValues($hash_ref, $schema, $catname);
                        }
                        else
                        {
                                my $next_line = <$ifd>;
-                               die "$input_file: ends within Perl hash\n"
+                               die "$input_file: file ends within Perl hash\n"
                                  if !defined $next_line;
                                $_ .= $next_line;
                                redo;
                        }
                }
 
-               # If we found a hash reference, keep it
-               # and annotate the line number.
+               # If we found a hash reference, keep it.
                # Only keep non-data strings if we
                # are told to preserve formatting.
                if (defined $hash_ref)
                {
-                       $hash_ref->{line_number} = $.;
                        push @$data, $hash_ref;
                }
                elsif ($preserve_formatting)
@@ -324,14 +327,8 @@ sub AddDefaultValues
 
        if (@missing_fields)
        {
-               my $msg = "Failed to form full tuple for $catname\n";
-               $msg .= "Missing values for: " . join(', ', @missing_fields);
-               $msg .= "\nOther values for row:\n";
-               while (my($key, $value) = each %$row)
-               {
-                       $msg .= "$key => $value, ";
-               }
-               die $msg;
+               die sprintf "missing values for field(s) %s in %s.dat line %s\n",
+                       join(', ', @missing_fields), $catname, $row->{line_number};
        }
 }
 

diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index 938686f0ca76a4b07a38e5411e510fd71b5a4354..90506b6a8048724f5273c8770bd7cc3c0d559969 100644 (file)
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -280,6 +280,7 @@ EOM
 
        print $bki "\n (\n";
        my $schema = $catalog->{columns};
+       my %attnames;
        my $attnum = 0;
        foreach my $column (@$schema)
        {
@@ -287,6 +288,9 @@ EOM
                my $attname = $column->{name};
                my $atttype = $column->{type};
 
+               # Build hash of column names for use later
+               $attnames{$attname} = 1;
+
                # Emit column definitions
                if (!$first)
                {
@@ -338,6 +342,16 @@ EOM
        {
                my %bki_values = %$row;
 
+               # Complain about unrecognized keys; they are presumably misspelled
+               foreach my $key (keys %bki_values)
+               {
+                       next if $key eq "oid" || $key eq "oid_symbol" || $key eq "descr"
+                               || $key eq "line_number";
+                       die sprintf "unrecognized field name \"%s\" in %s.dat line %s\n",
+                               $key, $catname, $bki_values{line_number}
+                               if (!exists($attnames{$key}));
+               }
+
                # Perform required substitutions on fields
                foreach my $column (@$schema)
                {
@@ -724,6 +738,9 @@ sub morph_row_for_schemapg
 # Perform OID lookups on an array of OID names.
 # If we don't have a unique value to substitute, warn and
 # leave the entry unchanged.
+# (A warning seems sufficient because the bootstrap backend will reject
+# non-numeric values anyway.  So we might as well detect multiple problems
+# within this genbki.pl run.)
 sub lookup_oids
 {
        my ($lookup, $catname, $bki_values, @lookupnames) = @_;
@@ -739,7 +756,7 @@ sub lookup_oids
                else
                {
                        push @lookupoids, $lookupname;
-                       warn sprintf "unresolved OID reference \"%s\" in %s.dat line %s",
+                       warn sprintf "unresolved OID reference \"%s\" in %s.dat line %s\n",
                                $lookupname, $catname, $bki_values->{line_number}
                                if $lookupname ne '-' and $lookupname ne '0';
                }