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

tglsfdc · tglsfdc · commit 5372c2c84135 · 2018-04-18T18:17:02.000-04:00
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
@@ -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
@@ -280,13 +280,17 @@
 
 	print $bki "\n (\n";
 	my $schema = $catalog->{columns};
+	my %attnames;
 	my $attnum = 0;
 	foreach my $column (@$schema)
 	{
 		$attnum++;
 		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 @@
 	{
 		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';
 		}
