genbki stricter error handling
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
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