Horton

Thread

Commits

  1. Avoid uninitialized value error in TAP tests' Cluster->psql

  1. Cluster.pm psql() undefined $$stderr

    Oleg Tselebrovskiy <o.tselebrovskiy@postgrespro.ru> — 2025-06-04T14:01:54Z

    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

  2. Re: Cluster.pm psql() undefined $$stderr

    Andrew Dunstan <andrew@dunslane.net> — 2025-06-13T14:53:50Z

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


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::run
>
> Is 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

  3. Re: Cluster.pm psql() undefined $$stderr

    Oleg Tselebrovskiy <o.tselebrovskiy@postgrespro.ru> — 2025-06-16T04:23:20Z

    > 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

  4. Re: Cluster.pm psql() undefined $$stderr

    Andrew Dunstan <andrew@dunslane.net> — 2025-06-27T19:33:20Z

    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