"COPY foo FROM STDOUT" and ecpg

Started by Heikki Linnakangasalmost 13 years ago10 messages

hlinnakangas@vmware.com

almost 13 years ago

1 attachment(s)

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

Attachments:

remove-copy-from-stdout-check.patchtext/x-diff; name=remove-copy-from-stdout-check.patchDownload

*** a/src/interfaces/ecpg/preproc/ecpg.addons
--- b/src/interfaces/ecpg/preproc/ecpg.addons
***************
*** 193,207 **** ECPG: where_or_current_clauseWHERECURRENT_POFcursor_name block
  		$$ = cat_str(2,mm_strdup("where current of"), cursor_marker);
  	}
  ECPG: CopyStmtCOPYopt_binaryqualified_nameopt_column_listopt_oidscopy_fromcopy_file_namecopy_delimiteropt_withcopy_options addon
! 			if (strcmp($6, "to") == 0 && strcmp($7, "stdin") == 0)
! 				mmerror(PARSE_ERROR, ET_ERROR, "COPY TO STDIN is not possible");
! 			else if (strcmp($6, "from") == 0 && strcmp($7, "stdout") == 0)
! 				mmerror(PARSE_ERROR, ET_ERROR, "COPY FROM STDOUT is not possible");
! 			else if (strcmp($6, "from") == 0 && strcmp($7, "stdin") == 0)
  				mmerror(PARSE_ERROR, ET_WARNING, "COPY FROM STDIN is not implemented");
- ECPG: CopyStmtCOPYselect_with_parensTOcopy_file_nameopt_withcopy_options addon
- 			if (strcmp($4, "stdin") == 0)
- 				mmerror(PARSE_ERROR, ET_ERROR, "COPY TO STDIN is not possible");
  ECPG: var_valueNumericOnly addon
  		if ($1[0] == '$')
  		{
--- 193,201 ----
  		$$ = cat_str(2,mm_strdup("where current of"), cursor_marker);
  	}
  ECPG: CopyStmtCOPYopt_binaryqualified_nameopt_column_listopt_oidscopy_fromcopy_file_namecopy_delimiteropt_withcopy_options addon
! 			if (strcmp($6, "from") == 0 &&
! 			   (strcmp($7, "stdin") == 0 || strcmp($7, "stdout") == 0))
  				mmerror(PARSE_ERROR, ET_WARNING, "COPY FROM STDIN is not implemented");
  ECPG: var_valueNumericOnly addon
  		if ($1[0] == '$')
  		{

Tom Lane

tgl@sss.pgh.pa.us

almost 13 years ago

In reply to: Heikki Linnakangas (#1)

Re: "COPY foo FROM STDOUT" and ecpg

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Heikki Linnakangas

hlinnakangas@vmware.com

almost 13 years ago

In reply to: Tom Lane (#2)

Re: "COPY foo FROM STDOUT" and ecpg

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane

tgl@sss.pgh.pa.us

almost 13 years ago

In reply to: Heikki Linnakangas (#3)

Re: "COPY foo FROM STDOUT" and ecpg

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Greg Stark

stark@mit.edu

almost 13 years ago

In reply to: Heikki Linnakangas (#3)

Re: "COPY foo FROM STDOUT" and ecpg

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Heikki Linnakangas

hlinnakangas@vmware.com

almost 13 years ago

In reply to: Tom Lane (#4)

Re: "COPY foo FROM STDOUT" and ecpg

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Michael Meskes

meskes@postgresql.org

almost 13 years ago

In reply to: Heikki Linnakangas (#1)

Re: "COPY foo FROM STDOUT" and ecpg

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas

robertmhaas@gmail.com

almost 13 years ago

In reply to: Heikki Linnakangas (#6)

Re: "COPY foo FROM STDOUT" and ecpg

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Heikki Linnakangas

hlinnakangas@vmware.com

almost 13 years ago

In reply to: Michael Meskes (#7)

Re: "COPY foo FROM STDOUT" and ecpg

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10

Michael Meskes

meskes@postgresql.org

almost 13 years ago

In reply to: Heikki Linnakangas (#9)

Re: "COPY foo FROM STDOUT" and ecpg

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.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers