Re: Cleaning up perl code
Michael Paquier writes: > On Tue, Jul 02, 2024 at 01:55:25PM +0100, Dagfinn Ilmari Mannsåker wrote: >> For clarity, I've rebased my addional unused-variable changes (except >> the errcodes-related ones, see below) onto current master, and split it >> into separate commits with detailed explaiations for each file file, see >> attached. > > Thanks, I've squashed these three ones into a single commit for now > (good catches for 005_negotiate_encryption.pl, btw), and applied them. Thanks! - ilmari
Re: Cleaning up perl code
On Tue, Jul 02, 2024 at 01:55:25PM +0100, Dagfinn Ilmari Mannsåker wrote: > For clarity, I've rebased my addional unused-variable changes (except > the errcodes-related ones, see below) onto current master, and split it > into separate commits with detailed explaiations for each file file, see > attached. Thanks, I've squashed these three ones into a single commit for now (good catches for 005_negotiate_encryption.pl, btw), and applied them. -- Michael signature.asc Description: PGP signature
Re: Cleaning up perl code
Andrew Dunstan writes:
> On 2024-07-02 Tu 8:55 AM, Dagfinn Ilmari Mannsåker wrote:
>> Relatedly, I also had a look at prohibiting unused regex captures
>> (RegularExpressions::ProhibitUnusedCapture), which found a few real
>> cases, but also lots of false positives in Catalog.pm, because it
>> doesn't understand that %+ uses all named captures, so I won't propose a
>> patch for that until that's fixed upstream
>> (https://github.com/Perl-Critic/Perl-Critic/pull/1065).
>>
>
> We could mark Catalog.pm with a "## no critic (ProhibitUnusedCapture)"
> and then use the test elsewhere.
Yeah, that's what I've done for now. Here's a sequence of patches that
fixes the existing cases of unused captures, and adds the policy and
override.
The seg-validate.pl script seems unused, unmaintained and useless (it
doesn't actually match the syntax accepted by seg, specifcially the (+-)
syntax (which my patch fixes in passing)), so maybe we should just
delete it instead?
> cheers
>
> andrew
-ilmari
>From 17a350c8d6ec2ae887fe7784dd6b9275028dc681 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?=
Date: Mon, 20 May 2024 22:54:52 +0100
Subject: [PATCH 1/3] seg-validate.pl: Use qr// instead of strings to assemble
regexes
Also eliminate unused captures, and fix the plus/minus syntax to
accept (+-), not the erroneous '+-'.
---
contrib/seg/seg-validate.pl | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/contrib/seg/seg-validate.pl b/contrib/seg/seg-validate.pl
index 22cbca966d..518fbfb24a 100755
--- a/contrib/seg/seg-validate.pl
+++ b/contrib/seg/seg-validate.pl
@@ -5,15 +5,15 @@
use strict;
use warnings FATAL => 'all';
-my $integer = '[+-]?[0-9]+';
-my $real = '[+-]?[0-9]+\.[0-9]+';
+my $integer = qr/[+-]?[0-9]+/;
+my $real = qr/[+-]?[0-9]+\.[0-9]+/;
-my $RANGE = '(\.\.)(\.)?';
-my $PLUMIN = q(\'\+\-\');
-my $FLOAT = "(($integer)|($real))([eE]($integer))?";
-my $EXTENSION = '<|>|~';
+my $RANGE = qr/\.{2,3}/;
+my $PLUMIN = qr/\Q(+-)/;
+my $FLOAT = qr/(?:$integer|$real)(?:[eE]$integer)?/;
+my $EXTENSION = qr/[<>~]/;
-my $boundary = "($EXTENSION)?$FLOAT";
+my $boundary = qr/$EXTENSION?$FLOAT/;
my $deviation = $FLOAT;
my $rule_1 = $boundary . $PLUMIN . $deviation;
@@ -28,23 +28,23 @@
{
# s/ +//g;
- if (/^($rule_1)$/)
+ if (/^$rule_1$/)
{
print;
}
- elsif (/^($rule_2)$/)
+ elsif (/^$rule_2$/)
{
print;
}
- elsif (/^($rule_3)$/)
+ elsif (/^$rule_3$/)
{
print;
}
- elsif (/^($rule_4)$/)
+ elsif (/^$rule_4$/)
{
print;
}
- elsif (/^($rule_5)$/)
+ elsif (/^$rule_5$/)
{
print;
}
--
2.39.2
>From 31ab31f409ea3305105822d91fd688ecd35b594d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?=
Date: Tue, 2 Jul 2024 16:25:15 +0100
Subject: [PATCH 2/3] Fix unused regular expression captures
Remove unused captures, actually use the capture, or fix the capture
number used, as appropriate.
---
src/backend/catalog/Catalog.pm | 2 +-
src/backend/utils/activity/generate-wait_event_types.pl | 5 ++---
src/backend/utils/mb/Unicode/convutils.pm | 4 ++--
3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index 8e709524cb..59ebe5f434 100644
--- a/src/backend/catalog/Catalog.pm
+++ b/src/backend/catalog/Catalog.pm
@@ -69,7 +69,7 @@ sub ParseHeader
if (!$is_client_code)
{
# Strip C-style comments.
- s;/\*(.|\n)*\*/;;g;
+ s;/\*.*\*/;;gs;
if (m;/\*;)
{
diff --git a/src/backend/utils/activity/generate-wait_event_types.pl b/src/backend/utils/activity/generate-wait_event_types.pl
index 2f2fa5bb8f..4e8dc63e6b 100644
--- a/src/backend/utils/activity/generate-wait_event_types.pl
+++ b/src/backend/utils/activity/generate-wait_event_types.pl
@@ -55,10 +55,9 @@
next if /^\s*$/;
# Get waitclassname based on the section
- if (/^Section: ClassName(.*)/)
+ if (/^Section: ClassName - (\w+)/)
{
- $section_name = $_;
- $section_name =~ s/^.*- //;
+ $section_name = $1;
$abi_compatibility = 0;
next;
}
diff --git a/src/backend/utils/mb/Unicode/convutils.pm b/src/backend/utils/mb/Unicode/convutils.pm
index ee780acbe3..1b453d1eba 100644
--- a/src/backend/utils/mb/Unicode/convutils.pm
+++ b/src/backend/utils/mb/Unicode/convutils.pm
@@ -41,7 +41,7 @@ sub read_source
next if (/^$/);# Ignore empty lines
- next if (/^0x([0-9A-F]+)\s+(#.*)$/);
+ next if (/^0x[0-9A-F]+\s+#.*$/);
# The Unicode source files have three columns
# 1: The "foreign" code (in hex)
@@ -55,7 +55,7 @@ sub read_source
my $out = {
code => hex($1),
ucs => hex($2),
- comment => $4,
+ comment => $3,
direction => BOTH,
f => $fname,
l => $.
--
2.39.2
>From 3558190b163378d810b5af405992ebd475c1f8a7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?=
Date: Tue, 2 Jul 2024 16:26:10 +0100
Subject: [PATCH 3/3] Prohibit unused regular
Re: Cleaning up perl code
On 2024-07-02 Tu 8:55 AM, Dagfinn Ilmari Mannsåker wrote: Relatedly, I also had a look at prohibiting unused regex captures (RegularExpressions::ProhibitUnusedCapture), which found a few real cases, but also lots of false positives in Catalog.pm, because it doesn't understand that %+ uses all named captures, so I won't propose a patch for that until that's fixed upstream (https://github.com/Perl-Critic/Perl-Critic/pull/1065). We could mark Catalog.pm with a "## no critic (ProhibitUnusedCapture)" and then use the test elsewhere. cheers andrew -- Andrew Dunstan EDB:https://www.enterprisedb.com
Re: Cleaning up perl code
Michael Paquier writes:
> On Fri, May 24, 2024 at 02:09:49PM +0900, Michael Paquier wrote:
>> For now, I have staged for commit the attached, that handles most of
>> the changes from Alexander (msvc could go for more cleanup?).
>
> This one has been applied as of 0c1aca461481 now that v18 is
> open.
>
>> I'll look at the changes from Dagfinn after that, including if perlcritic
>> could be changed. I'll handle the first part when v18 opens up, as
>> that's cosmetic.
For clarity, I've rebased my addional unused-variable changes (except
the errcodes-related ones, see below) onto current master, and split it
into separate commits with detailed explaiations for each file file, see
attached.
> I'm still biased about the second set of changes proposed here,
> though. ProhibitUnusedVariables would have benefits when writing perl
> code in terms of clarity because we would avoid useless stuff, but it
> seems to me that we should put more efforts into the unification of
> the errcodes parsing paths first to have a cleaner long-term picture.
>
> That's not directly the fault of this proposal that we have the same
> parsing rules spread across three PL languages, so perhaps what's
> proposed is fine as-is, at the end.
It turns out there are a couple more places that parse errcodes.txt,
namely doc/src/sgml/generate-errcodes-table.pl and
src/backend/utils/generate-errcodes.pl. I'll have a go refactoring all
of these into a common function à la Catalog::ParseHeader() that returns
a data structure these scripts can use as as appropriate.
> Any thoughts or comments from others more familiar with
> ProhibitUnusedVariables?
Relatedly, I also had a look at prohibiting unused regex captures
(RegularExpressions::ProhibitUnusedCapture), which found a few real
cases, but also lots of false positives in Catalog.pm, because it
doesn't understand that %+ uses all named captures, so I won't propose a
patch for that until that's fixed upstream
(https://github.com/Perl-Critic/Perl-Critic/pull/1065).
- ilmari
>From 8e3c7721da86a670b279bb24b0fe9ae82351628f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?=
Date: Tue, 2 Jul 2024 13:25:16 +0100
Subject: [PATCH 1/3] Remove unused variables in libq encryption negotiation
test
The $node_conf parameter to test_matrix() was only ever used to set
the %params hash (removed in commit d39a49c1e4598048), but that hash
was itself never used for anything, so it (and the $server_config)
hash were always useless.
---
.../libpq/t/005_negotiate_encryption.pl | 24 +--
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/src/interfaces/libpq/t/005_negotiate_encryption.pl b/src/interfaces/libpq/t/005_negotiate_encryption.pl
index c3f70d31bc..251c405926 100644
--- a/src/interfaces/libpq/t/005_negotiate_encryption.pl
+++ b/src/interfaces/libpq/t/005_negotiate_encryption.pl
@@ -215,11 +215,6 @@ BEGIN
my @all_sslmodes = ('disable', 'allow', 'prefer', 'require');
my @all_sslnegotiations = ('postgres', 'direct');
-my $server_config = {
- server_ssl => 0,
- server_gss => 0,
-};
-
###
### Run tests with GSS and SSL disabled in the server
###
@@ -272,7 +267,7 @@ BEGIN
};
note("Running tests with SSL and GSS disabled in the server");
-test_matrix($node, $server_config,
+test_matrix($node,
['testuser'], \@all_gssencmodes, \@all_sslmodes, \@all_sslnegotiations,
parse_table($test_table));
@@ -311,19 +306,15 @@ BEGIN
# Enable SSL in the server
$node->adjust_conf('postgresql.conf', 'ssl', 'on');
$node->reload;
- $server_config->{server_ssl} = 1;
note("Running tests with SSL enabled in server");
- test_matrix(
- $node, $server_config,
- [ 'testuser', 'ssluser', 'nossluser' ], ['disable'],
- \@all_sslmodes, \@all_sslnegotiations,
+ test_matrix($node, [ 'testuser', 'ssluser', 'nossluser' ],
+ ['disable'], \@all_sslmodes, \@all_sslnegotiations,
parse_table($test_table));
# Disable SSL again
$node->adjust_conf('postgresql.conf', 'ssl', 'off');
$node->reload;
- $server_config->{server_ssl} = 0;
}
###
@@ -336,7 +327,6 @@ BEGIN
$krb->create_principal('gssuser', $gssuser_password);
$krb->create_ticket('gssuser', $gssuser_password);
- $server_config->{server_gss} = 1;
$test_table = q{
# USER GSSENCMODE SSLMODE SSLNEGOTIATION EVENTS -> OUTCOME
@@ -400,7 +390,7 @@ BEGIN
}
note("Running tests with GSS enabled in server");
- test_matrix($node, $server_config, [ 'testuser', 'gssuser', 'nogssuser' ],
+ test_matrix($node, [ 'testuser', 'gssuser', 'nogssuser' ],
\@all_gssencmodes, $sslmodes, $sslnegotiations,
parse_table($test_table));
}
@@ -423,7 +413,6 @@ BEGIN
# Enable SSL
$node->adjust_conf('postgresql.conf', 'ssl', 'on');
$node->reload;
- $server_config->{server_ssl} = 1;
$test_table = q{
# USER GSSENCMODE SSLMODE SSLNEGOTIATION EVENTS -> OUTCOME
@@ -510,7 +499,6 @@ BEGIN
note("Running tests with both GSS and SSL
Re: Cleaning up perl code
On Fri, May 24, 2024 at 02:09:49PM +0900, Michael Paquier wrote: > For now, I have staged for commit the attached, that handles most of > the changes from Alexander (msvc could go for more cleanup?). This one has been applied as of 0c1aca461481 now that v18 is open. > I'll look at the changes from Dagfinn after that, including if perlcritic > could be changed. I'll handle the first part when v18 opens up, as > that's cosmetic. I'm still biased about the second set of changes proposed here, though. ProhibitUnusedVariables would have benefits when writing perl code in terms of clarity because we would avoid useless stuff, but it seems to me that we should put more efforts into the unification of the errcodes parsing paths first to have a cleaner long-term picture. That's not directly the fault of this proposal that we have the same parsing rules spread across three PL languages, so perhaps what's proposed is fine as-is, at the end. Any thoughts or comments from others more familiar with ProhibitUnusedVariables? -- Michael signature.asc Description: PGP signature
Re: Cleaning up perl code
On Tue, May 21, 2024 at 02:33:16PM +0900, Michael Paquier wrote:
> Nice catches from both of you. The two ones in
> generate-wait_event_types.pl are caused by me, actually.
>
> Not sure about the changes in the errcodes scripts, though. The
> current state of thing can be also useful when it comes to debugging
> the parsing, and it does not hurt to keep the parsing rules the same
> across the board.
For now, I have staged for commit the attached, that handles most of
the changes from Alexander (msvc could go for more cleanup?). I'll
look at the changes from Dagfinn after that, including if perlcritic
could be changed. I'll handle the first part when v18 opens up, as
that's cosmetic.
The incorrect comment in 024_add_drop_pub.pl has been fixed as of
53785d2a2aaa, as that was a separate issue.
--
Michael
From 270e212e9f7524b96383387d62765185034fd59f Mon Sep 17 00:00:00 2001
From: Michael Paquier
Date: Fri, 24 May 2024 13:59:00 +0900
Subject: [PATCH] Cleanup perl code from unused variables and routines
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This commit removes unused variables and routines from some perl code
that have accumulated across the years. This concerns various areas:
the script for wait events, AdjustUpgrade.pm and TAP code.
Author: Alexander Lakhin
Reviewed-by: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/70b340bc-244a-589d-ef8b-d8aebb707...@gmail.com
---
.../utils/activity/generate-wait_event_types.pl | 2 --
src/bin/pg_dump/t/003_pg_dump_with_server.pl | 1 -
src/bin/pg_dump/t/005_pg_dump_filterfile.pl | 1 -
.../t/001_mutated_bindpasswd.pl | 4
.../ssl_passphrase_callback/t/001_testfunc.pl | 1 -
src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm| 1 -
src/test/recovery/t/021_row_visibility.pl | 1 -
src/test/recovery/t/032_relfilenode_reuse.pl | 1 -
.../recovery/t/035_standby_logical_decoding.pl| 5 +
contrib/amcheck/t/001_verify_heapam.pl| 15 +--
contrib/amcheck/t/002_cic.pl | 2 +-
11 files changed, 3 insertions(+), 31 deletions(-)
diff --git a/src/backend/utils/activity/generate-wait_event_types.pl b/src/backend/utils/activity/generate-wait_event_types.pl
index 42f36f405b..b2d61bd8ba 100644
--- a/src/backend/utils/activity/generate-wait_event_types.pl
+++ b/src/backend/utils/activity/generate-wait_event_types.pl
@@ -42,8 +42,6 @@ my @abi_compatibility_lines;
my @lines;
my $abi_compatibility = 0;
my $section_name;
-my $note;
-my $note_name;
# Remove comments and empty lines and add waitclassname based on the section
while (<$wait_event_names>)
diff --git a/src/bin/pg_dump/t/003_pg_dump_with_server.pl b/src/bin/pg_dump/t/003_pg_dump_with_server.pl
index b5a1445550..3f549f44e7 100644
--- a/src/bin/pg_dump/t/003_pg_dump_with_server.pl
+++ b/src/bin/pg_dump/t/003_pg_dump_with_server.pl
@@ -26,7 +26,6 @@ $node->safe_psql('postgres', "CREATE SERVER s1 FOREIGN DATA WRAPPER dummy");
$node->safe_psql('postgres', "CREATE SERVER s2 FOREIGN DATA WRAPPER dummy");
$node->safe_psql('postgres', "CREATE FOREIGN TABLE t0 (a int) SERVER s0");
$node->safe_psql('postgres', "CREATE FOREIGN TABLE t1 (a int) SERVER s1");
-my ($cmd, $stdout, $stderr, $result);
command_fails_like(
[ "pg_dump", '-p', $port, '--include-foreign-data=s0', 'postgres' ],
diff --git a/src/bin/pg_dump/t/005_pg_dump_filterfile.pl b/src/bin/pg_dump/t/005_pg_dump_filterfile.pl
index a80e13a0d3..6025bb296c 100644
--- a/src/bin/pg_dump/t/005_pg_dump_filterfile.pl
+++ b/src/bin/pg_dump/t/005_pg_dump_filterfile.pl
@@ -81,7 +81,6 @@ $node->safe_psql('sourcedb',
#
# Test interaction of correctly specified filter file
#
-my ($cmd, $stdout, $stderr, $result);
# Empty filterfile
open $inputfile, '>', "$tempdir/inputfile.txt"
diff --git a/src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl b/src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl
index b990e7d101..82b1cb88e9 100644
--- a/src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl
+++ b/src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl
@@ -12,10 +12,6 @@ use Test::More;
use lib "$FindBin::RealBin/../../../ldap";
use LdapServer;
-my ($slapd, $ldap_bin_dir, $ldap_schema_dir);
-
-$ldap_bin_dir = undef;# usually in PATH
-
if ($ENV{with_ldap} ne 'yes')
{
plan skip_all => 'LDAP not supported by this build';
diff --git a/src/test/modules/ssl_passphrase_callback/t/001_testfunc.pl b/src/test/modules/ssl_passphrase_callback/t/001_testfunc.pl
index 7a63539f39..f71d0ff3e0 100644
--- a/src/test/modules/ssl_passphrase_callback/t/001_testfunc.pl
+++ b/src/test/modules/ssl_passphrase_callback/t/001_testfunc.pl
@@ -15,7 +15,6 @@ unless (($ENV{with_ssl} || "") eq 'openssl')
plan skip_all => 'OpenSSL not supported by this build';
}
-my $clearpass = "FooBaR1";
my $rot13pass = "SbbOnE1";
# see the Makefile for how the certificate and key
Re: Cleaning up perl code
On Tue, May 21, 2024 at 06:00:00AM +0300, Alexander Lakhin wrote: > I reviewed my collection of unica I gathered for several months, but had > found some of them too minor/requiring more analysis. > Then I added more with perlcritic's policy UnusedVariables, and also > checked for unused subs with a script from blogs.perl.org (and it confirmed > my only previous find of that kind). Nice catches from both of you. The two ones in generate-wait_event_types.pl are caused by me, actually. Not sure about the changes in the errcodes scripts, though. The current state of thing can be also useful when it comes to debugging the parsing, and it does not hurt to keep the parsing rules the same across the board. >> The scripts parsing errcodes.txt really should be refactored into using >> a common module, but that's a patch for another day. > > Agree, and I would leave 005_negotiate_encryption.pl (with $node_conf, > $server_config unused since d39a49c1e) aside for another day too. I'm not sure about these ones as each one of these scripts have their own local tweaks. Now, if there is a cleaner picture with a .pm module I don't see while reading the whole, why not as long as it improves the code. -- Michael signature.asc Description: PGP signature
Re: Cleaning up perl code
Hello Dagfinn, Thank you for paying attention to it and improving the possible fix! 20.05.2024 23:39, Dagfinn Ilmari Mannsåker wrote: Nice cleanup! Did you use some static analysis tool, or did look for them manually? I reviewed my collection of unica I gathered for several months, but had found some of them too minor/requiring more analysis. Then I added more with perlcritic's policy UnusedVariables, and also checked for unused subs with a script from blogs.perl.org (and it confirmed my only previous find of that kind). If I add [Variables::ProhibitUnusedVariables] to src/tools/perlcheck/perlcriticrc, it finds a few more, see the attached patch. Yes, I saw unused $sqlstates, but decided that they are meaningful enough to stay. Though maybe enabling ProhibitUnusedVariables justifies fixing them too. The scripts parsing errcodes.txt really should be refactored into using a common module, but that's a patch for another day. Agree, and I would leave 005_negotiate_encryption.pl (with $node_conf, $server_config unused since d39a49c1e) aside for another day too. Best regards, Alexander
Re: Cleaning up perl code
Alexander Lakhin writes:
> Hello hackers,
>
> Please look at a bunch of unused variables and a couple of other defects
> I found in the perl code, maybe you'll find them worth fixing:
Nice cleanup! Did you use some static analysis tool, or did look for
them manually? If I add [Variables::ProhibitUnusedVariables] to
src/tools/perlcheck/perlcriticrc, it finds a few more, see the attached
patch.
The scripts parsing errcodes.txt really should be refactored into using
a common module, but that's a patch for another day.
- ilmari
>From 6b096a39753338bb91add5fcf1ed963024e58c15 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?=
Date: Mon, 20 May 2024 19:55:20 +0100
Subject: [PATCH] Prohibit unused variables
---
src/pl/plpgsql/src/generate-plerrcodes.pl | 6 ++
src/pl/plpython/generate-spiexceptions.pl | 6 ++
src/pl/tcl/generate-pltclerrcodes.pl | 6 ++
src/tools/perlcheck/perlcriticrc | 2 ++
src/tools/pgindent/pgindent | 2 +-
5 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/src/pl/plpgsql/src/generate-plerrcodes.pl b/src/pl/plpgsql/src/generate-plerrcodes.pl
index 1c662bc967..e969a4b33e 100644
--- a/src/pl/plpgsql/src/generate-plerrcodes.pl
+++ b/src/pl/plpgsql/src/generate-plerrcodes.pl
@@ -23,10 +23,8 @@
# Skip section headers
next if /^Section:/;
- die unless /^([^\s]{5})\s+([EWS])\s+([^\s]+)(?:\s+)?([^\s]+)?/;
-
- (my $sqlstate, my $type, my $errcode_macro, my $condition_name) =
- ($1, $2, $3, $4);
+ my ($type, $errcode_macro, $condition_name) =
+ /^[^\s]{5}\s+([EWS])\s+([^\s]+)(?:\s+)?([^\s]+)?/ or die;
# Skip non-errors
next unless $type eq 'E';
diff --git a/src/pl/plpython/generate-spiexceptions.pl b/src/pl/plpython/generate-spiexceptions.pl
index f0c5142be3..984017f212 100644
--- a/src/pl/plpython/generate-spiexceptions.pl
+++ b/src/pl/plpython/generate-spiexceptions.pl
@@ -23,10 +23,8 @@
# Skip section headers
next if /^Section:/;
- die unless /^([^\s]{5})\s+([EWS])\s+([^\s]+)(?:\s+)?([^\s]+)?/;
-
- (my $sqlstate, my $type, my $errcode_macro, my $condition_name) =
- ($1, $2, $3, $4);
+ my ($type, $errcode_macro, $condition_name) =
+ /^[^\s]{5}\s+([EWS])\s+([^\s]+)(?:\s+)?([^\s]+)?/ or die;
# Skip non-errors
next unless $type eq 'E';
diff --git a/src/pl/tcl/generate-pltclerrcodes.pl b/src/pl/tcl/generate-pltclerrcodes.pl
index fcac4d00a6..58eb6afefe 100644
--- a/src/pl/tcl/generate-pltclerrcodes.pl
+++ b/src/pl/tcl/generate-pltclerrcodes.pl
@@ -23,10 +23,8 @@
# Skip section headers
next if /^Section:/;
- die unless /^([^\s]{5})\s+([EWS])\s+([^\s]+)(?:\s+)?([^\s]+)?/;
-
- (my $sqlstate, my $type, my $errcode_macro, my $condition_name) =
- ($1, $2, $3, $4);
+ my ($type, $errcode_macro, $condition_name) =
+ /^[^\s]{5}\s+([EWS])\s+([^\s]+)(?:\s+)?([^\s]+)?/ or die;
# Skip non-errors
next unless $type eq 'E';
diff --git a/src/tools/perlcheck/perlcriticrc b/src/tools/perlcheck/perlcriticrc
index 4739e9f4f1..6053dfcc2a 100644
--- a/src/tools/perlcheck/perlcriticrc
+++ b/src/tools/perlcheck/perlcriticrc
@@ -15,6 +15,8 @@ verbose = %f: %m at line %l, column %c. %e. ([%p] Severity: %s)\n
# Note: for policy descriptions see https://metacpan.org/dist/Perl-Critic
+[Variables::ProhibitUnusedVariables]
+severity = 5
# allow octal constants with leading zeros
[-ValuesAndExpressions::ProhibitLeadingZeros]
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index 48d83bc434..063ec8ce63 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -22,7 +22,7 @@ my $indent_opts =
my $devnull = File::Spec->devnull;
my ($typedefs_file, $typedef_str, @excludes,
- $indent, $build, $diff,
+ $indent, $diff,
$check, $help, @commits,);
$help = 0;
--
2.39.2
Cleaning up perl code
Hello hackers,
Please look at a bunch of unused variables and a couple of other defects
I found in the perl code, maybe you'll find them worth fixing:
contrib/amcheck/t/001_verify_heapam.pl
$result # unused since introduction in 866e24d47
unused sub:
get_toast_for # not used since 860593ec3
contrib/amcheck/t/002_cic.pl
$result # orphaned since 7f580aa5d
src/backend/utils/activity/generate-wait_event_types.pl
$note, $note_name # unused since introduction in fa8892847
src/bin/pg_dump/t/003_pg_dump_with_server.pl
$cmd, $stdout, $stderr, $result # unused since introduction in 2f9eb3132
src/bin/pg_dump/t/005_pg_dump_filterfile.pl
$cmd, $stdout, $stderr, $result # unused since introduciton in a5cf808be
src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl
$slapd, $ldap_schema_dir # unused since introduction in 419a8dd81
src/test/modules/ssl_passphrase_callback/t/001_testfunc.pl
$clearpass # orphaned since b846091fd
src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm
$ostmt # unused since introduction in 47bb9db75
src/test/recovery/t/021_row_visibility.pl
$ret # unused since introduction in 7b28913bc
src/test/recovery/t/032_relfilenode_reuse.pl
$ret # unused since introduction in e2f65f425
src/test/recovery/t/035_standby_logical_decoding.pl
$stdin, $ret, $slot # unused since introduction in fcd77d532
$subscriber_stdin, $subscriber_stdout, $subscriber_stderr # unused since
introduction in 376dc8205
src/test/subscription/t/024_add_drop_pub.pl
invalid reference in a comment:
tab_drop_refresh -> tab_2 # introduced with 1046a69b3
(invalid since v6-0001-...patch in the commit's thread)
src/tools/msvc_gendef.pl
@def # unused since introduction in 933b46644?
I've attached a patch with all of these changes (tested with meson build
on Windows and check-world on Linux).
Best regards,
Alexanderdiff --git a/contrib/amcheck/t/001_verify_heapam.pl b/contrib/amcheck/t/001_verify_heapam.pl
index 9de3148277..481e4dbe4f 100644
--- a/contrib/amcheck/t/001_verify_heapam.pl
+++ b/contrib/amcheck/t/001_verify_heapam.pl
@@ -9,7 +9,7 @@ use PostgreSQL::Test::Utils;
use Test::More;
-my ($node, $result);
+my $node;
#
# Test set-up
@@ -87,19 +87,6 @@ sub relation_filepath
return "$pgdata/$rel";
}
-# Returns the fully qualified name of the toast table for the named relation
-sub get_toast_for
-{
- my ($relname) = @_;
-
- return $node->safe_psql(
- 'postgres', qq(
- SELECT 'pg_toast.' || t.relname
- FROM pg_catalog.pg_class c, pg_catalog.pg_class t
- WHERE c.relname = '$relname'
- AND c.reltoastrelid = t.oid));
-}
-
# (Re)create and populate a test table of the given name.
sub fresh_test_table
{
diff --git a/contrib/amcheck/t/002_cic.pl b/contrib/amcheck/t/002_cic.pl
index 53a3db9745..0207407ca0 100644
--- a/contrib/amcheck/t/002_cic.pl
+++ b/contrib/amcheck/t/002_cic.pl
@@ -10,7 +10,7 @@ use PostgreSQL::Test::Utils;
use Test::More;
-my ($node, $result);
+my $node;
#
# Test set-up
diff --git a/src/backend/utils/activity/generate-wait_event_types.pl b/src/backend/utils/activity/generate-wait_event_types.pl
index 42f36f405b..b2d61bd8ba 100644
--- a/src/backend/utils/activity/generate-wait_event_types.pl
+++ b/src/backend/utils/activity/generate-wait_event_types.pl
@@ -42,8 +42,6 @@ my @abi_compatibility_lines;
my @lines;
my $abi_compatibility = 0;
my $section_name;
-my $note;
-my $note_name;
# Remove comments and empty lines and add waitclassname based on the section
while (<$wait_event_names>)
diff --git a/src/bin/pg_dump/t/003_pg_dump_with_server.pl b/src/bin/pg_dump/t/003_pg_dump_with_server.pl
index b5a1445550..3f549f44e7 100644
--- a/src/bin/pg_dump/t/003_pg_dump_with_server.pl
+++ b/src/bin/pg_dump/t/003_pg_dump_with_server.pl
@@ -26,7 +26,6 @@ $node->safe_psql('postgres', "CREATE SERVER s1 FOREIGN DATA WRAPPER dummy");
$node->safe_psql('postgres', "CREATE SERVER s2 FOREIGN DATA WRAPPER dummy");
$node->safe_psql('postgres', "CREATE FOREIGN TABLE t0 (a int) SERVER s0");
$node->safe_psql('postgres', "CREATE FOREIGN TABLE t1 (a int) SERVER s1");
-my ($cmd, $stdout, $stderr, $result);
command_fails_like(
[ "pg_dump", '-p', $port, '--include-foreign-data=s0', 'postgres' ],
diff --git a/src/bin/pg_dump/t/005_pg_dump_filterfile.pl b/src/bin/pg_dump/t/005_pg_dump_filterfile.pl
index a80e13a0d3..6025bb296c 100644
--- a/src/bin/pg_dump/t/005_pg_dump_filterfile.pl
+++ b/src/bin/pg_dump/t/005_pg_dump_filterfile.pl
@@ -81,7 +81,6 @@ $node->safe_psql('sourcedb',
#
# Test interaction of correctly specified filter file
#
-my ($cmd, $stdout, $stderr, $result);
# Empty filterfile
open $inputfile, '>', "$tempdir/inputfile.txt"
diff --git a/src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl b/src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl
index b990e7d101..82b1cb88e9 100644
--- a/src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl
+++ b/src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl
@@ -12,10 +12,6 @@ use Test::More;
