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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Daniel Verite <daniel(at)manitou-mail(dot)org>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, 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-12 20:41:25
Message-ID: 10024.1547325685@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I took a quick look at this patch. Some thoughts:

* I think the right way to proceed is to commit (and back-patch)
this without any regression test case, and maybe later look into
adding a TAP test that covers "\g file". We have no test cases
for any variant of "\g file", and because of platform variability
and suchlike considerations, adding that seems like a project of
its own --- one I'd not want to back-patch.

* I do agree with documenting this by adding some small note to the
discussion of \copy.

* I think you've made the wrong choice about how this interacts with
the pset.copyStream option. If copyStream is set, that should
continue to take priority, IMO, as anything else would break do_copy's
assumptions about what will happen. One example of why it would be
problematic is that both levels of code would think they control what
to do with the sigpipe trap. Now, it's likely that \copy's syntax
makes it impossible for both copyStream and gfname to be set at the
same time anyway, because \copy extends to the end of the line. But
if it were to happen, we don't want it to be something that do_copy
has to take into account; better therefore just to leave gfname alone.
(Note also the comment just above where you patched, which you failed
to update.)

* You need to be more careful about error cases. I note that
openQueryOutputFile is capable of returning failure with is_pipe
set true, which would lead this patch to disable the sigpipe trap
and never re-enable it.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2019-01-12 21:35:55 Re: O_DIRECT for relations and SLRUs (Prototype)
Previous Message Tom Lane 2019-01-12 19:03:35 Re: Three animals fail test-decoding-check on REL_10_STABLE