Re: psql - add special variable to reflect the last query status

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: psql - add special variable to reflect the last query status
Date: 2017-09-13 08:30:50
Message-ID: alpine.DEB.2.20.1709131013370.20876@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Hello Tom,

>> I put back SetResultVariables function which is called twice, for SQL
>> queries and the new descriptions. It worked out of the box with DECLARE
>> which is just another SQL statement, so maybe I did not understood the
>> cursor issue you were signaling...
>
> No, I was concerned about ExecQueryUsingCursor(), which is used when
> FETCH_COUNT is enabled. It's sort of a pain because you have to
> accumulate the row count across multiple PGresults. If you don't,
> then FETCH_COUNT mode isn't transparent, which it's supposed to be.

Please allow me to disagree a little with this semantics.

ISTM that the semantics of the simple previous implementation was fine,
"number of rows returned by previous statement". If you do "FETCH 3 ...",
then you get between 0 and 3 rows... Good. If you do it again, same...

I'm not sure having an accumulation semantics helps a lot, because it
creates an exception, and moreover I can think of legitimate use case
where counting only last statement rows would be useful, eg to check that
we are done with a cursor and it can be closed.

If someone really wants to accumulate, it can be done by hand quite
simply, currently as:

SELECT :ROW_COUNT + :accum AS accum \gset

or client side:

\set accum `expr :ROW_COUNT + :accum`

and maybe some day something like:

\let accum :ROW_COUNT + :accum

> I did some performance testing of my own, based on this possibly-silly
> test case: [...]
>
> The idea was to run a trivial query and minimize all other psql overhead,
> particularly results-printing. With this, "perf" told me that
> SetResultVariables and its called functions accounted for 1.5% of total
> CPU (including the server processes).

> That's kind of high, but it's probably tolerable considering that any
> real application would involve both far more server work per query and
> far more psql work (at least for SELECTs).

This seems pretty reasonable to me, and is consistent with my 1% elapsed
time measure on a silent "SELECT;".

> One thing we could think about if this seems too high is to drop
> ROW_COUNT. I'm unconvinced that it has a real use-case, and it seems
> to be taking more than its share of the work in non-error cases, because
> it turns out that PQcmdTuples() is not an amazingly cheap function.

I do think that a small overhead on a contrived case is worth removing the
feature, as it is really insignificant on any realistic case.

--
Fabien.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2017-09-13 08:32:36 Re: [Proposal] Allow users to specify multiple tables in VACUUM commands
Previous Message Kyotaro HORIGUCHI 2017-09-13 08:28:20 Re: [Proposal] Allow users to specify multiple tables in VACUUM commands