Cluster.pm psql() undefined $$stderr
Greetings, everyone!
If you call node->psql in not-array context, with on_error_die => 1,
but without passing stderr, you will get the following error
and the test will die, but not the way we expect:
Use of uninitialized value in concatenation (.) or string at
/path/to/source_code/src/test/perl/PostgreSQL/Test/Cluster.pm line 2258
This is because $$stderr is not defined in this case
and warnings became FATAL some time ago.
The code string in question for clarity:
...
die
"error running SQL: '$$stderr'\n..."
if $ret == 3;
Minimal reproduction is:
$node->psql('postgres', q{SELEC 1}, on_error_die => 1);
This can be reproduced at current master (30c15987)
Undefined $$stderr also should break dying on recieving a signal, here:
# We always die on signal.
if (defined $ret)
{
... die (".... $$stderr ...");
One of the ways to fix this is to initialize $$stderr with some value
to avoid Perl error (Use of uninitialized value) and replace it with
existing error: "error running SQL: '$$stderr'\n ..."
With this approach we don't lose any useful error messages in
regress_log_*
The proposed patch is attached (0001)
---------------------------------------------------------------------------
Another question I've stumbled upon when trying to fix the
aforementioned
issue is the following: Does redirecting IPC::Run::run streams work with
Postgres Perl module SimpleTee? I've tried to use it to "tee" STDERR to
both $$stderr and to test's regress_log_* file. I'm not sure I'm doing
this right, but I have not found a way to use SimpleTee with
IPC::Run::run
Is there a way to use them together?
I've tried something like (0002)
Regards, Oleg Tselebrovskiy
On 2025-06-04 We 10:01 AM, Oleg Tselebrovskiy wrote:
Greetings, everyone!
If you call node->psql in not-array context, with on_error_die => 1,
but without passing stderr, you will get the following error
and the test will die, but not the way we expect:Use of uninitialized value in concatenation (.) or string at
/path/to/source_code/src/test/perl/PostgreSQL/Test/Cluster.pm line
2258This is because $$stderr is not defined in this case
and warnings became FATAL some time ago.The code string in question for clarity:
...
die
"error running SQL: '$$stderr'\n..."
if $ret == 3;Minimal reproduction is:
$node->psql('postgres', q{SELEC 1}, on_error_die => 1);
This can be reproduced at current master (30c15987)
Undefined $$stderr also should break dying on recieving a signal, here:
# We always die on signal.
if (defined $ret)
{
... die (".... $$stderr ...");One of the ways to fix this is to initialize $$stderr with some value
to avoid Perl error (Use of uninitialized value) and replace it with
existing error: "error running SQL: '$$stderr'\n ..."With this approach we don't lose any useful error messages in
regress_log_*The proposed patch is attached (0001)
I think we need to do something slightly earlier than that. Attached is
what I propose.
---------------------------------------------------------------------------
Another question I've stumbled upon when trying to fix the aforementioned
issue is the following: Does redirecting IPC::Run::run streams work with
Postgres Perl module SimpleTee? I've tried to use it to "tee" STDERR to
both $$stderr and to test's regress_log_* file. I'm not sure I'm doing
this right, but I have not found a way to use SimpleTee with
IPC::Run::runIs there a way to use them together?
I've tried something like (0002)
I think the short answer is no, we've already hijacked STDOUT/STDERR in
Utils.pm to point to the log file, and you shouldn't mess with them. I
don't think you're using SimpleTee correctly anyway, but it's really not
meant for general use, only for the use in Utils.pm.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Attachments:
fix-cluster-stderr.patchtext/x-patch; charset=UTF-8; name=fix-cluster-stderr.patchDownload+8-0
I think we need to do something slightly earlier than that. Attached is
what I propose.
I agree, my patch could miss one case with undefined $$stderr.
Fixed my patch with your suggestion
I think the short answer is no, we've already hijacked STDOUT/STDERR in
Utils.pm to point to the log file, and you shouldn't mess with them. I
don't think you're using SimpleTee correctly anyway, but it's really
not
meant for general use, only for the use in Utils.pm.
Thanks for the info!
Regards, Oleg Tselebrovskiy
Attachments:
0001_fix_undef_stderr_v2.patchtext/x-diff; name=0001_fix_undef_stderr_v2.patchDownload+39-0
On 2025-06-16 Mo 12:23 AM, Oleg Tselebrovskiy wrote:
I think we need to do something slightly earlier than that. Attached is
what I propose.I agree, my patch could miss one case with undefined $$stderr.
Fixed my patch with your suggestion
I don't think we need your test script, and it looks rather ugly anyway.
Given we've got this far without it being a significant issue, I don't
think there's much need to backpatch it.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com