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