Re: Alternative to \copy in psql modelled after \g

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Daniel Verite <daniel(at)manitou-mail(dot)org>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Subject: Re: Alternative to \copy in psql modelled after \g
Date: 2019-01-21 20:57:09
Message-ID: alpine.DEB.2.21.1901211719060.28235@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


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 .

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2019-01-21 21:01:07 Re: Hint and detail punctuation
Previous Message Bruce Momjian 2019-01-21 20:56:24 Re: Protect syscache from bloating with negative cache entries