Re: [HACKERS] pgbench - allow to store select results intovariables - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: [HACKERS] pgbench - allow to store select results intovariables
Date
Msg-id alpine.DEB.2.21.1901102058150.27692@lancre
Whole thread Raw
In response to Re: [HACKERS] pgbench - allow to store select results into variables  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Hello Tom,

> So who needs that?  Just merge the queries, if it's so important that
> you avoid multiple round trips.

Pgbench is about testing (measuring) performance in various settings and 
realistic scenarii: queries prepared or not, possible combined, and so on. 
As postgres allows to send several queries into one message, I think it 
interesting to be able to test the performance impact of doing that (the 
answer is very significant, esp wrt lantency), and it should not preclude 
to get the results out as a client application could do.

Queries can be "merged", but ISTM syntax is not especially friendly 
(UNION, SELECT of SELECT, CROSS JOIN not sure which one you had in 
mind...) nor reprensentative of what a client application would really do, 
and it would mess with readability, message size, planning and so. Also, 
compound queries need to return all one row, but this constraint is 
neeeded for variable.

>> We can take it out I guess, but my impression was that we already pretty
>> much had a consensus that it was wanted.
>
> Maybe if the implementation weren't a pile of junk it'd be all right,
> but as-is this is a mess.

Thanks:-)

> The dependency on counting \; in particular is setting me off, because 
> that has little if anything to do with the number of query results to be 
> expected.

The kludge is because there is a kludge (aka optimizations:-) server side 
to silently ignore empty queries. On "SELECT 1 \; /**/ \; SELECT 2 ;" the 
server sends two results back instead of 3, whereas it should logically 
return an empty result on the empty query.

Now pgbench could detect that there is an empty query (possibly skipping 
comments and so), an early version of the patch did that AFAICR, but the 
code did not seem worth it, it seemed cleaner to just document the 
restriction, so it was removed.

> I imagine the argument will be that nobody would write the sort of 
> queries that break that assumption in a pgbench script;

Detecting empty queries is possible, although the code for doing that is 
kind of ugly and would look bad in the lexer, on which it seemed desirable 
to have minimum changes.

> but I don't find that kind of design to be up to project standards, 
> especially not when the argument for the feature is tissue-thin in the 
> first place.

The "first place" is to be able to implement more realistic scenarii, and 
to have getting results into variables orthogonal to combined queries.

Although I'm not specially thrilled by the resulting syntax, the point is 
to provide a feature pertinent to performance testing, not to have a 
incredibly well designed syntax. It just goes on with the existing 
backslash approach used by psql & pgbench, which has the significant 
advantage that it is mostly SQL with a few things around.

-- 
Fabien.


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: NOTIFY and pg_notify performance when deduplicating notifications
Next
From: Robert Haas
Date:
Subject: Re: MERGE SQL statement for PG12