Re: [PATCH] Avoid uninitialized-value error in poll_query_until timeout diagnostic

Hello!!

Bryan Green <dbryan(dot)green(at)gmail(dot)com> writes:
> ...
> Cluster.pm runs under "use warnings FATAL => 'all'", so when $query is
> undefined this does not warn, it dies with "Use of uninitialized value
> $query in concatenation". The result is that a timeout in a connection only
> poll fails with an uninitialized-value error instead of printing the timeout
> diagnostic the code is trying to produce, hiding the actual failure.
>
> $ perl -e 'use warnings FATAL => "all"; my $q; my $s = qq(q: $q);'
> Use of uninitialized value $q in concatenation (.) or string at -e line 1.

This was a little bit tricky to reproduce, but I used the following
line:

perl -I src/test/perl -MPostgreSQL::Test::Cluster -MPostgreSQL::Test::Utils -MTest::More -e 'local $PostgreSQL::Test::Utils::timeout_default=0.1; my $n=PostgreSQL::Test::Cluster->new("poll_repro",install_path=>"$ENV{PWD}/build/tmp_install/usr/local/pgsql");ok(!$n->poll_query_until("postgres", undef, ""), "connection-only timeout"); done_testing();'

I wasn't able to reproduce this in a test, so this make sense to happen
only if there's any failure during one of the tests, but this snippet
proved the failure exists.

> The fix uses a fallback string when the query is undefined:
>
> my $msg_query = $query // '(undef - connection attempt only)';
>
> and interpolates $msg_query instead. Test-only, one line.

What about using the most common way in the code instead of using `//`
operator? Something like:

my $msg_query = '(undef - connection attempt only)' unless defined $query;

This is how is done everywhere else and is a bit more clear than the
`//` operator which looks pretty tricky even for Perl

Regards,
--
Jonathan Gonzalez V.
EDB
https://www.enterprisedb.com