Re: authentication/t/001_password.pl trashes ~/.psql_history

On 2023-12-22 Fr 17:11, Tom Lane wrote:
> I wrote:
>> I happened to notice this stuff getting added to my .psql_history:
>> \echo background_psql: ready
>> SET password_encryption='scram-sha-256';
>> ;
>> \echo background_psql: QUERY_SEPARATOR
>> SET scram_iterations=42;
>> ;
>> \echo background_psql: QUERY_SEPARATOR
>> \password scram_role_iter
>> \q
>> After grepping for these strings, this is evidently the fault of
>> src/test/authentication/t/001_password.pl by way of BackgroundPsql.pm,
>> which fires up an interactive psql run that is not given the -n switch.
>> Currently the only other user of interactive_psql() seems to be
>> psql/t/010_tab_completion.pl, which avoids this problem by
>> explicitly redirecting the history file. We could have 001_password.pl
>> do likewise, or we could have it pass the -n switch, but I think we're
>> going to have this problem resurface repeatedly if we leave it to the
>> outer test script to remember to do it.
>
> After studying this some more, my conclusion is that BackgroundPsql.pm
> failed to borrow as much as it should have from 010_tab_completion.pl.
> Specifically, we want all the environment-variable changes that that
> script performed to be applied in any test using an interactive psql.
> Maybe ~/.inputrc and so forth would never affect any other test scripts,
> but that doesn't seem like a great bet.
>
> So that leads me to the attached proposed patch.

Looks fine, after reading your original post I was thinking along the
same lines.

You could shorten this

+    my $history_file = $params{history_file};
+    $history_file ||= '/dev/null';
+    $ENV{PSQL_HISTORY} = $history_file;

to just

     $ENV{PSQL_HISTORY} = $params{history_file} || '/dev/null';

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com