Re: Alternative to \copy in psql modelled after \g - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: Alternative to \copy in psql modelled after \g
Date
Msg-id alpine.DEB.2.21.1901221319000.15365@lancre
Whole thread Raw
In response to Re: Alternative to \copy in psql modelled after \g  ("Daniel Verite" <daniel@manitou-mail.org>)
Responses Re: Alternative to \copy in psql modelled after \g  ("Daniel Verite" <daniel@manitou-mail.org>)
List pgsql-hackers
>>   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.


pgsql-hackers by date:

Previous
From: Etsuro Fujita
Date:
Subject: Re: [HACKERS] advanced partition matching algorithm for partition-wisejoin
Next
From: Haribabu Kommi
Date:
Subject: Re: [HACKERS] Block level parallel vacuum