Bonsoir Daniel,
>> sql> SELECT 1 \g /BAD
>> /BAD: Permission denied
>>
>> sql> \echo :ERROR
>> false
>
> That seems broken, because it's pointless to leave out a class of errors
> from ERROR.
Yep. That is my point, but fixing it requires to decide whether it is okay
to change ERROR documentation and behavior, and ISTM that there is some
legit case to have the SQL_ERROR information independently.
> Now if you download data with SELECT or COPY and we can't even
> create the file, how is that a good idea to intentionally have the
> script fail to detect it? What purpose does it satisfy?
It means that the client knows that the SQL command, and possible
associated side effects, were executed server-side, and that if we are in
a transaction it is still going on:
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.
>> The documentation states that ERROR is about SQL, not psql internal stuff:
>>
>> ERROR true if the last SQL query failed, false if it succeeded.
>> See also SQLSTATE.
>
> ERROR is set by SetResultVariables(PGresult *results, bool success)
> and takes the value of "success", ignoring the PGresult.
Yes and no: "success" pretty often currently depends on PGresult, eg:
if (PQresultStatus(results) != PGRES_COMMAND_OK) {
...
SetResultVariables(results, false);
results = PQexec(pset.db, buf.data);
OK = AcceptResult(results); // true if SQL is ok
...
SetResultVariables(results, OK);
results = PQexec(pset.db, buf.data);
OK = AcceptResult(results) &&
(PQresultStatus(results) == PGRES_COMMAND_OK);
if (!OK)
SetResultVariables(results, OK);
> So why is ERROR independant of the SQL result, relatively to your
> claim that it should never reflect anything else?
AFAICS I'm not claiming that, on the contrary I'm basically ok with
changing ERROR documentation and implementation (what I called option 1),
although it would have to pass through committers, *AND* I'm still
thinking that having a separate access to whether the SQL failed or not is
of interest, so there is an argument to add a SQL_ERROR which reflects the
current documentation, if not fully the implementation.
--
Fabien .