Re: 001_password.pl fails with --without-readline
Oleg Tselebrovskiy <o.tselebrovskiy@postgrespro.ru>
From: Oleg Tselebrovskiy <o.tselebrovskiy@postgrespro.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org, Soumya S Murali <soumyamurali.work@gmail.com>
Date: 2026-01-19T08:37:39Z
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
Commits
-
Make psql/t/030_pager.pl more robust.
- 6918434a4acb 19 (unreleased) landed
-
Improve guards against false regex matches in BackgroundPsql.pm.
- fc84b3977e20 14.21 landed
- f356e2888cdb 17.8 landed
- bb6aedeca8d4 15.16 landed
- a1d7ae2b2e38 19 (unreleased) landed
- 92b3cc5a28f1 18.2 landed
- 6548e4a10d18 16.12 landed
-
Fix unsafe pushdown of quals referencing grouping Vars
- 34740b90bc12 19 (unreleased) cited
-
tests: BackgroundPsql: Fix potential for lost errors on windows
- 8b886a4e3488 18.0 cited