|From:||Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>|
|To:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|Cc:||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|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
Here is a version 6.
> A few thoughts about this patch:
> * I think the ERROR_CODE variable should instead be named SQLSTATE.
> That is what the SQL standard calls this string, and it's also what
> just about all our documentation calls it; see PG_DIAG_SQLSTATE
> in libpq, or the SQLSTATE 'xxxxx' construct in pl/pgsql, or the
> sqlstate attribute of an exception object in plpython, etc etc.
ERROR_CODE -> SQLSTATE.
> * I'm not exactly convinced that there's a use-case for STATUS
Removed, but I think it was nice to have, it is easier to interpret than
error codes and their classes that I have not memorized yet:-)
> * It might be better if SQLSTATE and ERROR_MESSAGE were left
> unchanged by a non-error query. That would reduce the need to
> copy them into other variables just because you needed to do
> something else before printing them. It'd save a few cycles too.
Added LAST_ERROR_SQLSTATE & MESSAGE, only reset when an error occured.
> * Speaking of which, has anyone tried to quantify the performance
> impact of this patch? It might well be negligible, but I do not
> think we should assume that without evidence.
My guess is negligible. Not sure how to measure this negligible, as many
very fast query should be executed to have something significant. Maybe
100,000 "SELECT 1;" in a script?
> * I wonder why you didn't merge this processing into ProcessResult,
> instead of inventing an extra function (whose call site seems rather
> poorly chosen anyhow --- what's the justification for not counting this
> overhead as part of the query runtime?). You could probably eliminate
> the refactoring you did, since it wouldn't be necessary to recompute
> AcceptResult's result that way.
Variable setting moved at then end of ProcessResult, no new functions,
result is clean, so I should have done it like that in the beginning.
Forgotten help stuff added.
|Next Message||Kyotaro HORIGUCHI||2017-09-06 08:36:02||Re: [BUGS] Bug in Physical Replication Slots (at least 9.5)?|
|Previous Message||Haribabu Kommi||2017-09-06 07:45:20||Re: VACUUM and ANALYZE disagreeing on what reltuples means|