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.
* I'm not exactly convinced that there's a use-case for STATUS
that's not covered as well or better by ERROR. Client code that
looks at PQresStatus for anything beyond error/not-error is
usually doing that because it's library code that doesn't know
what kind of query it's working on. It seems like a stretch that
a psql script would not know that. Also, PQresultStatus memorializes
some legacy distinctions, like "fatal" vs "nonfatal" error, that
I think we'd be better off not exposing in psql scripting.
* 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.
* 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.
* 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.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
