BEGIN;
UPDATE pgbench_branches SET bbalance = bbalance + 1 RETURNING * \g /bad
// the update is performed, the transaction is not rollbacked,
// *but* the output file was not written, "COMMIT" save changes.
if PQexec() could not store the results for lack of memory, it would
yield a PGRES_FATAL_ERROR, then :ERROR would be set to true, and yet
the server-side operation would have been performed. Additionally, If
that BEGIN was not there, the statement would also have been
committed, so its effect would be durable independently of the value
of :ERROR.
Indeed, OOM is a special case when the client is unable to know whether
the command was actually executed. However ISTM that the connection is
likely to be aborted, so if there is an explicit transaction it would be
aborted as well.
My point is that client-side issues already affect :ERROR,
so it can't be assumed that :ERROR=true implies that the SQL
statement did not have effects on the server.
Not from my reading of the doc (which I probably wrote, and did the
implementation without a clear view of the different client-specific error
conditions, so all this is really my fault BTW), where I understand that
ERROR as true says that the SQL failed.
In that sense, the patch in its current state does not break this
guarantee, since it does not exist in the first place.
Sure, but the patch in its current state creates an inconsistency between
an SQL command with "\g /BAD" and the same command in "COPY (...) TO
STDOUT \g /BAD". I think that it must at the minimum be consistent.
OTOH I hope that :ERROR = false is a true guarantee that there have been
no problem whatsoever in the execution of the last statement.
on the contrary I'm basically ok with changing ERROR
documentation and implementation (what I called option 1),
The doc only says:
ERROR
true if the last SQL query failed, false if it succeeded.
See also SQLSTATE.
If the failure to retrieve results is included in "query failed", which
seems a reasonable interpretation to me, it doesn't need to be changed.
I understand "SQL query failed" as a server issue, especially as SQLSTATE
is the reported command status at the PQ protocol level.
What's not right is "SELECT ... \g /bad" having a different effect on
:ERROR than "COPY... \g /bad".
Yep, I've been saying that.
I plan to follow up on that because there are other problems with
SELECT ... \g anyway, for instance, when a disk full occurs,
it's not reported at all. But that problem is not in the code path
of COPY.
Sure. As there are several bugs (doubtful features) uncovered, a first
patch could fix "COPY ...TO STDOUT \g file", but probably replicate ERROR
current behavior however debatable it is (i.e. your patch without the
ERROR change, which is unrelated to the bug being fixed), and then another
patch should fix/modify the behavior around ERROR (everywhere and
consistently), and probably IMHO add an SQL_ERROR.
--
Fabien.
