Thread: "COPY foo FROM STDOUT" and ecpg
While looking at Fujita Etsuro's patch to allow copy to/from a shell command, I noticed that the grammar currently allows these: COPY foo FROM STDOUT COPY foo TO STDIN In other words, STDIN and STDOUT can be used completely interchangeably. However, the ecpg grammar is more strict about that: ERROR: COPY TO STDIN is not possible Any particular reason for ecpg to check that, while the backend doesn't care? I think we should just remove those checks from the ecpg grammar. - Heikki
Attachment
Heikki Linnakangas <hlinnakangas@vmware.com> writes:
> While looking at Fujita Etsuro's patch to allow copy to/from a shell
> command, I noticed that the grammar currently allows these:
> COPY foo FROM STDOUT
> COPY foo TO STDIN
> In other words, STDIN and STDOUT can be used completely interchangeably.
> However, the ecpg grammar is more strict about that:
> ERROR: COPY TO STDIN is not possible
> Any particular reason for ecpg to check that, while the backend doesn't
> care? I think we should just remove those checks from the ecpg grammar.
Agreed, but your draft patch doesn't do that completely. It should only
make tests that correspond to what the error message says. (I assume
the backend will bounce the other cases at some post-grammar stage.)
regards, tom lane
On 26.02.2013 18:23, Tom Lane wrote: > Heikki Linnakangas<hlinnakangas@vmware.com> writes: >> While looking at Fujita Etsuro's patch to allow copy to/from a shell >> command, I noticed that the grammar currently allows these: > >> COPY foo FROM STDOUT >> COPY foo TO STDIN > >> In other words, STDIN and STDOUT can be used completely interchangeably. >> However, the ecpg grammar is more strict about that: > >> ERROR: COPY TO STDIN is not possible > >> Any particular reason for ecpg to check that, while the backend doesn't >> care? I think we should just remove those checks from the ecpg grammar. > > Agreed, but your draft patch doesn't do that completely. It should only > make tests that correspond to what the error message says. Sorry, I don't understand what you're saying. Can you elaborate? > (I assume > the backend will bounce the other cases at some post-grammar stage.) No. All four combinations of FROM/TO and STDIN/STDOUT are accepted: postgres=# copy foo from stdin; Enter data to be copied followed by a newline. End with a backslash and a period on a line by itself.>> foo>> \. postgres=# copy foo from stdout; Enter data to be copied followed by a newline. End with a backslash and a period on a line by itself.>> bar>> \. postgres=# copy foo to stdin; foo bar postgres=# copy foo to stdout; foo bar - Heikki
Heikki Linnakangas <hlinnakangas@vmware.com> writes:
> On 26.02.2013 18:23, Tom Lane wrote:
>> (I assume
>> the backend will bounce the other cases at some post-grammar stage.)
> No. All four combinations of FROM/TO and STDIN/STDOUT are accepted:
Huh. That seems like an odd decision. If we agree that that behavior
is desirable, then your patch is ok as-is, though I do question whether
this should be tested in the grammar at all rather than at runtime.
I wonder whether this is just an oversight, or if we did it
intentionally because people were confused about which combinations
to use. It seems like maybe "TO STDIN" could be justified if you
thought about stdin of the recipient rather than stdout of the server,
but it still seems a bit sloppy thinking.
regards, tom lane
On Tue, Feb 26, 2013 at 4:34 PM, Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > No. All four combinations of FROM/TO and STDIN/STDOUT are accepted: ... > postgres=# copy foo to stdin; > foo > bar > postgres=# copy foo to stdout; > foo > bar Hm, so STDIN/STDOUT are just noise words and psql uses stdin for input and stdout for output regardless of what's specified? That seems a bit odd. I would have expected to be able to do something like cat > script.sql copy foo to stdin; copy bar to stdout; ^D psql -f script.sql 0>/tmp/foo.data 1>/tmp/bar.data But then I haven't heard any clamoring of demand for such a feature. And if there was it would make sense to implement "copy foo to fd X" and then make stdin an alias for "fd 0" rather than only support two file descriptors. It wouldn't make sense to expend all that effort just to support writing to just stdin. -- greg
On 26.02.2013 18:40, Tom Lane wrote: > Heikki Linnakangas<hlinnakangas@vmware.com> writes: >> On 26.02.2013 18:23, Tom Lane wrote: >>> (I assume >>> the backend will bounce the other cases at some post-grammar stage.) > >> No. All four combinations of FROM/TO and STDIN/STDOUT are accepted: > > Huh. That seems like an odd decision. If we agree that that behavior > is desirable, then your patch is ok as-is, though I do question whether > this should be tested in the grammar at all rather than at runtime. > > I wonder whether this is just an oversight, or if we did it > intentionally because people were confused about which combinations > to use. It seems like maybe "TO STDIN" could be justified if you > thought about stdin of the recipient rather than stdout of the server, > but it still seems a bit sloppy thinking. Yeah, I'd guess that it was an oversight. But it goes back all the way to Postgres95, so it's a bit too late to change that. - Heikki
On Tue, Feb 26, 2013 at 05:13:38PM +0200, Heikki Linnakangas wrote: > COPY foo FROM STDOUT > COPY foo TO STDIN Does this make sense? > Any particular reason for ecpg to check that, while the backend > doesn't care? I think we should just remove those checks from the > ecpg grammar. IIRC this check was added because the check for "COPY FROM STDIN" had to added anyway. Since you left that one in, the patch is fine by me, although I still don't see a reason for it. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL
On Tue, Feb 26, 2013 at 11:50 AM, Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > Yeah, I'd guess that it was an oversight. But it goes back all the way to > Postgres95, so it's a bit too late to change that. I don't see why. We've plugged holes like this before and will do so again in the future, I'm sure. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 26.02.2013 18:58, Michael Meskes wrote: > On Tue, Feb 26, 2013 at 05:13:38PM +0200, Heikki Linnakangas wrote: >> Any particular reason for ecpg to check that, while the backend >> doesn't care? I think we should just remove those checks from the >> ecpg grammar. > > IIRC this check was added because the check for "COPY FROM STDIN" had to added > anyway. Since you left that one in, the patch is fine by me, although I still > don't see a reason for it. Just less code to maintain. And it's strange to forbid something in ecpg grammar that the backend accepts, as a matter of principle. - Heikki
On Tue, Feb 26, 2013 at 07:24:44PM +0200, Heikki Linnakangas wrote: > >IIRC this check was added because the check for "COPY FROM STDIN" had to added > >anyway. Since you left that one in, the patch is fine by me, although I still > >don't see a reason for it. > > Just less code to maintain. And it's strange to forbid something in > ecpg grammar that the backend accepts, as a matter of principle. Na, I was talking about copying from STDOUT and to STDIN. Even if this is a very old "feature" I think we should fix it. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL