Re: Making background psql nicer to use in tap tests

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
Date: 2023-01-31 00:00:47
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


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, but that ended up being awkward. So I bit the bullet
and introduced a new class (in, 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/, 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?


Andres Freund

Attachment Content-Type Size
v1-0001-WIP-test-Introduce-BackgroundPsql-class.patch text/x-diff 26.4 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
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