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.
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 |