genbki stricter error handling

Started by Peter Eisentrautover 4 years ago2 messages
#1Peter Eisentraut
peter.eisentraut@enterprisedb.com
1 attachment(s)

genbki.pl says:

# 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.)

This is fine, but I have found this to be a bit cumbersome in practice
sometimes, because errors are then not easily seen at build time but
have to be extracted from some log files during test runs.

I propose the attached patch to make genbki.pl error out if it
encounters any errors in this routine, while preserving the property
that all errors in one run are reported.

Attachments:

0001-genbki-stricter-error-handling.patchtext/plain; charset=UTF-8; name=0001-genbki-stricter-error-handling.patch; x-mac-creator=0; x-mac-type=0Download
From 7a0c042afa3be15f3b4aedc4c90cc12929d4630f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 23 Jun 2021 11:17:07 +0200
Subject: [PATCH] genbki stricter error handling

Instead of just writing warnings for invalid cross-catalog lookups,
count the errors and error out at the end.
---
 src/backend/catalog/genbki.pl | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index 81363a0710..24d86a76db 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -26,6 +26,8 @@
 my $major_version;
 my $include_path;
 
+my $num_errors = 0;
+
 GetOptions(
 	'output:s'       => \$output_path,
 	'set-version:s'  => \$major_version,
@@ -788,7 +790,7 @@
 Catalog::RenameTempFile($fk_info_file,     $tmpext);
 Catalog::RenameTempFile($constraints_file, $tmpext);
 
-exit 0;
+exit ($num_errors != 0 ? 1 : 0);
 
 #################### Subroutines ########################
 
@@ -1016,8 +1018,7 @@ 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
+# (We don't exit right away so that we can detect multiple problems
 # within this genbki.pl run.)
 sub lookup_oids
 {
@@ -1037,16 +1038,20 @@ sub lookup_oids
 			push @lookupoids, $lookupname;
 			if ($lookupname eq '-' or $lookupname eq '0')
 			{
-				warn sprintf
-				  "invalid zero OID reference in %s.dat field %s line %s\n",
-				  $catname, $attname, $bki_values->{line_number}
-				  if !$lookup_opt;
+				if (!$lookup_opt)
+				{
+					warn sprintf
+					  "invalid zero OID reference in %s.dat field %s line %s\n",
+					  $catname, $attname, $bki_values->{line_number};
+					$num_errors++;
+				}
 			}
 			else
 			{
 				warn sprintf
 				  "unresolved OID reference \"%s\" in %s.dat field %s line %s\n",
 				  $lookupname, $catname, $attname, $bki_values->{line_number};
+				$num_errors++;
 			}
 		}
 	}
-- 
2.32.0

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#1)
Re: genbki stricter error handling

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

I propose the attached patch to make genbki.pl error out if it
encounters any errors in this routine, while preserving the property
that all errors in one run are reported.

+1, looks sane in a quick read-through.

regards, tom lane