Re: 001_password.pl fails with --without-readline

From: Oleg Tselebrovskiy <o(dot)tselebrovskiy(at)postgrespro(dot)ru>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org, Soumya S Murali <soumyamurali(dot)work(at)gmail(dot)com>
Subject: Re: 001_password.pl fails with --without-readline
Date: 2026-01-19 08:37:39
Message-ID: 1768811859.80915427@fmail1.qdit
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Answering both emails at once

-----
BackgroundPsql.pm

> While debugging that I got annoyed that a match failure results
> in a timeout exit with absolutely no data logged about what output
> the test got.  So v3-0001 also changes timeout() --- which creates
> a timeout that aborts the test --- to timer() --- which does what
> the test author clearly expected, namely just stop waiting for
> more input.  (There's a thread somewhere around here about making
> that change more globally, but I went ahead and did it here.)

I've found your thread about this - [1], and I agree, using
timer() is better here, we get the stdout and stderr of a timed-out
query

> On second thought, that's far more complicated than necessary.
> It should be sufficient to insert quote marks in the echo commands.

Quote marks really work! Very elegant change, it works with or without
readline - I like it! v3 approach with \set was good too, but I too
prefer quote marks

Also, thanks for making both "pump until" blocks identical, it seemed
a little strange to have them be different. At first I thought it was
made deliberately, but when I was debugging pump_until function I saw
how two streams were filling and it seems that both "pump until"
blocks work the same way

-----
030_pager.pl

> As for 0002, I agree that we can't do much except not insist that
> the match cover the whole line.  However, then the question is how
> much risk are we taking of a false match?  It's not too bad for the
> first three tests, where we know what the query will output so we
> can be sure that the pattern will (or won't) appear if the pager
> is or isn't invoked.  However, testing for just "\d+\n" seems fairly
> scary from that standpoint.  It happens not to match to the current
> contents of information_schema.referential_constraints, but that's
> a very hairy query that's subject to change.  I think we had better
> take this test out of the business of relying on the contents of that
> view, and instead create our own simple view that we can be sure of.

I like the idea of using a hand-crafted view for the test instead of
information_schema.referential_constraints, since its d+ output can
change between PostgreSQL versions, and hand-crafted view won't change
suddenly, only deliberately

> On a more nitpicky level, writing "^.*" is useless, because it'll
> match anything at all, so we might as well just remove it from the
> pattern.

Well, yeah, there's no difference between "^.*39" and just plain "39"
for such a regex

All in all - v4 seems to be both elegant and robust

[1] - https://www.postgresql.org/message-id/1100715.1712265845@sss.pgh.pa.us
 
--
Regards,
Oleg

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Oleg Tselebrovskiy 2026-01-19 08:44:45 Re: 001_password.pl fails with --without-readline
Previous Message Chao Li 2026-01-19 08:20:06 Re: docs: clarify ALTER TABLE behavior on partitioned tables