Bonsoir Daniel,
>> 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.
>
> It's not as if the patch issued an explicit call SetVariable("ERROR", "true")
> that could be commented, or something like that. The assignment
> of the variable happens as a consequence of patched code that aims at
> being correct in its error handling.
> So I'm for leaving this decision to a maintainer, because I don't agree
> with your conclusion that the current patch should be changed in
> that regard.
Ok, fine with me.
To summarize the debate, when fixing "\g filename" for "COPY TO STDOUT"
the patch does not set error consistently on:
sql> COPY (SELECT 1) TO STDOUT \g /BAD
# Permission denied on "/BAD"
-> :ERROR is true
# the command is executed but could not write to the file
compared to the existing behavior:
sql> SELECT 1 \g BAD
# Permission denied on "/BAD"
-> :ERROR is false
# the SQL command is fine, although writing to the file failed
Should we include this inconsistency and then fix the problem later, or
replicate the initial (doubtful?) behavior for consistency and then fix
the problem later, anyway?
Fixing the problem envolves deciding what is the right behavior, and
update the documentation and the implementation accordingly. Currently the
documentation suggests that :ERROR is about SQL error (subject to
interpretation), and the implementation is more or less consistent with
that view, but not always, as pointed out by Daniel.
Note that as the author of the initial patch adding :ERROR and others
(69835bc8), I'm probably somehow responsible for this situation, so shame
on me.
--
Fabien.
