|From:||Andres Freund <andres(at)anarazel(dot)de>|
|To:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|Cc:||pgsql-hackers(at)postgresql(dot)org, Melanie Plageman <melanieplageman(at)gmail(dot)com>|
|Subject:||Re: Making background psql nicer to use in tap tests|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On 2023-01-30 15:06:46 -0500, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > It's annoyingly hard to wait for the result of a query in a generic way with
> > background_psql(), and more generally for psql. background_psql() uses -XAtq,
> > which means that we'll not get "status" output (like "BEGIN" or "(1 row)"),
> > and that queries not returning anything are completely invisible.
> Yeah, the empty-query-result problem was giving me fits recently.
> +1 for wrapping this into something more convenient to use.
I've hacked some on this. I first tried to just introduce a few helper
functions in Cluster.pm, but that ended up being awkward. So I bit the bullet
and introduced a new class (in BackgroundPsql.pm), and made background_psql()
and interactive_psql() return an instance of it.
This is just a rough prototype. Several function names don't seem great, it
need POD documentation, etc.
The main convenience things it has over the old interface:
- $node->background_psql('dbname') is enough
- $psql->query(), which returns the query results as a string, is a lot easier
to use than having to pump, identify query boundaries via regex etc.
- $psql->query_safe(), which dies if any query fails (detected via stderr)
- $psql->query_until() is a helper that makes it a bit easier to start queries
that won't finish until a later point
I don't quite like the new interface yet:
- It's somewhat common to want to know if there was a failure, but also get
the query result, not sure what the best function signature for that is in
- query_until() sounds a bit too much like $node->poll_query_until(). Maybe
query_wait_until() is better? OTOH, the other function has poll in the name,
so maybe it's ok.
- right now there's a bit too much logic in background_psql() /
interactive_psql() for my taste
Those points aside, I think it already makes the tests a good bit more
readable. My WIP vacuum_defer_cleanup_age patch shrunk by half with it.
I think with a bit more polish it's easy enough to use that we could avoid a
good number of those one-off psql's that we do all over.
I didn't really know what this, insrc/test/subscription/t/015_stream.pl, is
$h->finish; # errors make the next test fail, so ignore them here
There's no further test?
I'm somewhat surprised it doesn't cause problems in another ->finish later on,
where we then afterwards just use $h again. Apparently IPC::Run just
automagically restarts psql?
|Next Message||Tom Lane||2023-01-31 00:07:35||Re: generate_series for timestamptz and time zone problem|
|Previous Message||Nathan Bossart||2023-01-30 23:30:40||monitoring usage count distribution|