generic copy options

This looks good. Shoud I try to elaborate on that for the patch with
error logging and autopartitioning in COPY?

manu

Robert Haas wrote:

Here's a half-baked proof of concept for the above approach. This
probably needs more testing than I've given it, and I haven't
attempted to fix the psql parser or update the documentation, but it's
at least an outline of a solution. I did patch all the regression
tests to use the new syntax, so you can look at that part of the patch
to get a flavor for it. If this is broadly acceptable I can attempt
to nail down the details, or someone else is welcome to pick it up.
It's on my git repo as well, as usual.

--
Emmanuel Cecchet
Aster Data Systems
Web: http://www.asterdata.com

On Mon, Sep 14, 2009 at 2:51 PM, Emmanuel Cecchet <manu@asterdata.com> wrote:

This looks good. Shoud I try to elaborate on that for the patch with error
logging and autopartitioning in COPY?

That make sense to me. You shouldn't need to do anything else in
gram.y; whatever you want to add should just involve changing copy.c.
If not, please post the details.

We also need to fix the psql end of this, and the docs... any
interest in taking a crack at either of those?

...Robert

Robert Haas wrote:

On Mon, Sep 14, 2009 at 2:51 PM, Emmanuel Cecchet <manu@asterdata.com> wrote:

This looks good. Shoud I try to elaborate on that for the patch with error
logging and autopartitioning in COPY?

That make sense to me. You shouldn't need to do anything else in
gram.y; whatever you want to add should just involve changing copy.c.
If not, please post the details.

Ok, I'll keep you posted.

We also need to fix the psql end of this, and the docs... any
interest in taking a crack at either of those?

I can certainly help with the doc.
I have never looked at the psql code but that could be a good way to get
started on that. If you can point me at where to look at, I'll give it a
try.

Emmanuel

--
Emmanuel Cecchet
Aster Data Systems
Web: http://www.asterdata.com

On Mon, Sep 14, 2009 at 3:25 PM, Emmanuel Cecchet <manu@asterdata.com> wrote:

Robert Haas wrote:

On Mon, Sep 14, 2009 at 2:51 PM, Emmanuel Cecchet <manu@asterdata.com>
wrote:

This looks good. Shoud I try to elaborate on that for the patch with
error
logging and autopartitioning in COPY?

That make sense to me.  You shouldn't need to do anything else in
gram.y; whatever you want to add should just involve changing copy.c.
If not, please post the details.

Ok, I'll keep you posted.

We also need to fix the psql end of this, and the docs...  any
interest in taking a crack at either of those?

I can certainly help with the doc.

If you have the time to revise the docs to describe this new syntax,
that would be great.

I have never looked at the psql code but that could be a good way to get
started on that. If you can point me at where to look at, I'll give it a
try.

I don't know either off the top of my head, but I'll go look for it
when I get a chance.

...Robert

Robert Haas <robertmhaas@gmail.com> writes:

On Mon, Sep 14, 2009 at 3:25 PM, Emmanuel Cecchet <manu@asterdata.com> wrote:

I have never looked at the psql code but that could be a good way to get
started on that. If you can point me at where to look at, I'll give it a
try.

I don't know either off the top of my head, but I'll go look for it
when I get a chance.

src/bin/psql/copy.c ...

regards, tom lane

Robert,

Here is a new version of the patch with an updated doc and psql.
I changed the name of the CSV options to prefix them with csv_ to avoid
confusion with any future options. I also had to change the grammar to
allow '*' as a parameter (needed for cvs_force_quote).

When we decide to drop the old syntax (in 8.6?), we will be able to
clean a lot especially in psql.

Emmanuel

On Mon, Sep 14, 2009 at 2:51 PM, Emmanuel Cecchet <manu@asterdata.com> wrote:

This looks good. Shoud I try to elaborate on that for the patch with error
logging and autopartitioning in COPY?

That make sense to me. You shouldn't need to do anything else in
gram.y; whatever you want to add should just involve changing copy.c.
If not, please post the details.

We also need to fix the psql end of this, and the docs... any
interest in taking a crack at either of those?

...Robert

--
Emmanuel Cecchet
Aster Data Systems
Web: http://www.asterdata.com

On Wed, Sep 16, 2009 at 6:43 PM, Emmanuel Cecchet <manu@asterdata.com> wrote:

Here is a new version of the patch with an updated doc and psql.

Thanks, that's great.

I don't think the way the doc changes are formatted is consistent with
what we've done elsewhere. I think that breaking the options out as a
separate block could be OK (because otherwise they have to be
duplicated between COPY TO and COPY FROM) but it should be done more
like the way that the SELECT page is done. Also, you haven't
documented the syntax 100% correctly: the boolean options work just
like the boolean explain options - they take an optional argument
which if omitted defaults to true, but you can also specify 0, 1,
true, false, on, off. See defGetBoolean. So those should be
specified as:

BINARY [boolean]
OIDS [boolean]
CSV [boolean]
CSV_HEADER [boolean]

See how we did it in sql-explain.html.

I changed the name of the CSV options to prefix them with csv_ to avoid
confusion with any future options. I also had to change the grammar to allow
'*' as a parameter (needed for cvs_force_quote).

You seem to have introduced a LARGE number of unnecessary whitespace
changes here which are not going to fly. You need to go through and
revert all of those. It's hard to tell what you've really changed
here, but also every whitespace change that gets committed is a
potential merge conflict for someone else; plus pgindent will
eventually change it back, thus creating another potential merge
conflict for someone else.

I am not 100% sold on renaming all of the CSV-specific options to add
"csv_". I would like to get an opinion from someone else on whether
that is a good idea or not. I am fairly certain it is NOT a good idea
to support BOTH the old and new option names, as you've done here. If
you're going to rename them, you should update gram.y and change the
makeDefElem() calls within the copy_opt_list productions to emit the
new names.

When we decide to drop the old syntax (in 8.6?), we will be able to clean a
lot especially in psql.

Considering that we are still carrying syntax that was deprecated in
7.3, I don't think it's likely that we'll phase out the present syntax
anywhere nearly that quickly. But it's reasonable to ask whether we
should think about removing support for the pre-7.3 syntax altogether
for 8.5. It doesn't seem to cost us much to keep that support around,
but then again it's been deprecated for seven major releases, so it
might be about time.

...Robert

Robert Haas wrote:

I don't think the way the doc changes are formatted is consistent with
what we've done elsewhere. I think that breaking the options out as a
separate block could be OK (because otherwise they have to be
duplicated between COPY TO and COPY FROM) but it should be done more
like the way that the SELECT page is done.

I looked at the way it is done in SELECT and there is a section per
clause (from clause, where clause, ...). So I am not sure how you want
to apply that here besides the copy parameters and the option clause.

Also, you haven't
documented the syntax 100% correctly: the boolean options work just
like the boolean explain options - they take an optional argument
which if omitted defaults to true, but you can also specify 0, 1,
true, false, on, off. See defGetBoolean. So those should be
specified as:

BINARY [boolean]
OIDS [boolean]
CSV [boolean]
CSV_HEADER [boolean]

See how we did it in sql-explain.html.

Ok, fixed.

I changed the name of the CSV options to prefix them with csv_ to avoid
confusion with any future options. I also had to change the grammar to allow
'*' as a parameter (needed for cvs_force_quote).

You seem to have introduced a LARGE number of unnecessary whitespace
changes here which are not going to fly. You need to go through and
revert all of those. It's hard to tell what you've really changed
here, but also every whitespace change that gets committed is a
potential merge conflict for someone else; plus pgindent will
eventually change it back, thus creating another potential merge
conflict for someone else.

Sorry, I overlooked a format in Eclipse that formatted the whole file
instead of the block I was working on. This should be fixed now.

I am not 100% sold on renaming all of the CSV-specific options to add
"csv_". I would like to get an opinion from someone else on whether
that is a good idea or not. I am fairly certain it is NOT a good idea
to support BOTH the old and new option names, as you've done here. If
you're going to rename them, you should update gram.y and change the
makeDefElem() calls within the copy_opt_list productions to emit the
new names.

Agreed for the makeDefElem().
For changing the names, I think that names like 'header', 'escape' and
'quote' are too generic to not conflict with something that is not csv.
If you think of another format that could be added to copy, it is likely
to re-use the same variable names. The only thing that seems odd is that
if you use a CSV_* option, you still have to add CSV [on] to the option
list which seems kind of redundant.

When we decide to drop the old syntax (in 8.6?), we will be able to clean a
lot especially in psql.

Considering that we are still carrying syntax that was deprecated in
7.3, I don't think it's likely that we'll phase out the present syntax
anywhere nearly that quickly. But it's reasonable to ask whether we
should think about removing support for the pre-7.3 syntax altogether
for 8.5. It doesn't seem to cost us much to keep that support around,
but then again it's been deprecated for seven major releases, so it
might be about time.

While I understand the need for the server to still support the syntax,
is it necessary for newer version of psql to support the old syntax?

I am attaching the new version of the patch with the current
modifications addressing your comments.

Emmanuel

--
Emmanuel Cecchet
Aster Data Systems
Web: http://www.asterdata.com

Hello

I am not sure about syntax change. Isn't better solve this problem
well. This is too simple solution. I thinking, so we able to add new
parser for COPY statement and share this paraser between SQL and psql.

regards
Pavel Stehule

2009/9/17 Emmanuel Cecchet <manu@asterdata.com>:

Emmanuel Cecchet <manu@asterdata.com> writes:

Robert Haas wrote:

When we decide to drop the old syntax (in 8.6?), we will be able to clean a
lot especially in psql.

Considering that we are still carrying syntax that was deprecated in
7.3, I don't think it's likely that we'll phase out the present syntax
anywhere nearly that quickly.

While I understand the need for the server to still support the syntax,
is it necessary for newer version of psql to support the old syntax?

psql has MORE need to support old syntax than the backend does, because
it's supposed to work against old servers.

I wonder though if we couldn't simplify matters. Offhand it seems to me
that psql doesn't need to validate the command's syntax fully. All it
really needs to do is find the target filename and replace it with
STDIN/STDOUT. Could we have it just treat the remainder of the line
literally, and not worry about the details of what the options might be?
Let the backend worry about throwing an error if they're bad.

regards, tom lane

Pavel,

I am not sure about syntax change. Isn't better solve this problem
well. This is too simple solution. I thinking, so we able to add new
parser for COPY statement and share this paraser between SQL and psql.

Refactoring COPY to put new parsers seems to be another project. The
idea here is that the parser does not have to be changed again if we add
new options, we just have to handle them in the COPY code (which will
probably have to be refactored at some point as it was discussed in a
previous thread).

manu

2009/9/17 Emmanuel Cecchet <manu@asterdata.com>:

Robert Haas wrote:

I don't think the way the doc changes are formatted is consistent with
what we've done elsewhere. I think that breaking the options out as a
separate block could be OK (because otherwise they have to be
duplicated between COPY TO and COPY FROM) but it should be done more
like the way that the SELECT page is done.

I looked at the way it is done in SELECT and there is a section per clause
(from clause, where clause, ...). So I am not sure how you want to apply
that here besides the copy parameters and the option clause.

Also, you haven't
documented the syntax 100% correctly: the boolean options work just
like the boolean explain options - they take an optional argument
which if omitted defaults to true, but you can also specify 0, 1,
true, false, on, off. See defGetBoolean. So those should be
specified as:

BINARY [boolean]
OIDS [boolean]
CSV [boolean]
CSV_HEADER [boolean]

See how we did it in sql-explain.html.

Ok, fixed.

I changed the name of the CSV options to prefix them with csv_ to avoid
confusion with any future options. I also had to change the grammar to
allow
'*' as a parameter (needed for cvs_force_quote).

You seem to have introduced a LARGE number of unnecessary whitespace
changes here which are not going to fly. You need to go through and
revert all of those. It's hard to tell what you've really changed
here, but also every whitespace change that gets committed is a
potential merge conflict for someone else; plus pgindent will
eventually change it back, thus creating another potential merge
conflict for someone else.

Sorry, I overlooked a format in Eclipse that formatted the whole file
instead of the block I was working on. This should be fixed now.

I am not 100% sold on renaming all of the CSV-specific options to add
"csv_". I would like to get an opinion from someone else on whether
that is a good idea or not. I am fairly certain it is NOT a good idea
to support BOTH the old and new option names, as you've done here. If
you're going to rename them, you should update gram.y and change the
makeDefElem() calls within the copy_opt_list productions to emit the
new names.

Agreed for the makeDefElem().
For changing the names, I think that names like 'header', 'escape' and
'quote' are too generic to not conflict with something that is not csv. If
you think of another format that could be added to copy, it is likely to
re-use the same variable names. The only thing that seems odd is that if you
use a CSV_* option, you still have to add CSV [on] to the option list which
seems kind of redundant.

When we decide to drop the old syntax (in 8.6?), we will be able to clean
a
lot especially in psql.

Considering that we are still carrying syntax that was deprecated in
7.3, I don't think it's likely that we'll phase out the present syntax
anywhere nearly that quickly. But it's reasonable to ask whether we
should think about removing support for the pre-7.3 syntax altogether
for 8.5. It doesn't seem to cost us much to keep that support around,
but then again it's been deprecated for seven major releases, so it
might be about time.

While I understand the need for the server to still support the syntax, is
it necessary for newer version of psql to support the old syntax?

I am attaching the new version of the patch with the current modifications
addressing your comments.

Emmanuel

--
Emmanuel Cecchet
Aster Data Systems
Web: http://www.asterdata.com

### Eclipse Workspace Patch 1.0
#P Postgres8.5-COPY
Index: src/test/regress/sql/copy2.sql
===================================================================
RCS file: /home/manu/cvsrepo/pgsql/src/test/regress/sql/copy2.sql,v
retrieving revision 1.18
diff -u -r1.18 copy2.sql
--- src/test/regress/sql/copy2.sql      25 Jul 2009 00:07:14 -0000      1.18
+++ src/test/regress/sql/copy2.sql      17 Sep 2009 03:14:48 -0000
@@ -73,17 +73,17 @@
\.

-- various COPY options: delimiters, oids, NULL string
-COPY x (b, c, d, e) from stdin with oids delimiter ',' null 'x';
+COPY x (b, c, d, e) from stdin (oids, delimiter ',', null 'x');
500000,x,45,80,90
500001,x,\x,\\x,\\\x
500002,x,\,,\\\,,\\
\.

-COPY x from stdin WITH DELIMITER AS ';' NULL AS '';
+COPY x from stdin (DELIMITER ';', NULL '');
3000;;c;;
\.

-COPY x from stdin WITH DELIMITER AS ':' NULL AS E'\\X';
+COPY x from stdin (DELIMITER ':', NULL E'\\X');
4000:\X:C:\X:\X
4001:1:empty::
4002:2:null:\X:\X
@@ -108,13 +108,13 @@
INSERT INTO no_oids (a, b) VALUES (20, 30);

-- should fail
-COPY no_oids FROM stdin WITH OIDS;
-COPY no_oids TO stdout WITH OIDS;
+COPY no_oids FROM stdin (OIDS);
+COPY no_oids TO stdout (OIDS);

-- check copy out
COPY x TO stdout;
COPY x (c, e) TO stdout;
-COPY x (b, e) TO stdout WITH NULL 'I''m null';
+COPY x (b, e) TO stdout (NULL 'I''m null');

CREATE TEMP TABLE y (
col1 text,
@@ -130,11 +130,23 @@
COPY y TO stdout WITH CSV FORCE QUOTE col2 ESCAPE E'\\';
COPY y TO stdout WITH CSV FORCE QUOTE *;

+-- Test new 8.5 syntax
+
+COPY y TO stdout (CSV);
+COPY y TO stdout (CSV, CSV_QUOTE '''', DELIMITER '|');
+COPY y TO stdout (CSV, CSV_FORCE_QUOTE (col2), CSV_ESCAPE E'\\');
+COPY y TO stdout (CSV, CSV_FORCE_QUOTE *);
+
+\COPY y TO stdout (CSV)
+\COPY y TO stdout (CSV, CSV_QUOTE '''', DELIMITER '|')
+\COPY y TO stdout (CSV, CSV_FORCE_QUOTE (col2), CSV_ESCAPE E'\\')
+\COPY y TO stdout (CSV, CSV_FORCE_QUOTE *)
+
--test that we read consecutive LFs properly

CREATE TEMP TABLE testnl (a int, b text, c int);

-COPY testnl FROM stdin CSV;
+COPY testnl FROM stdin (CSV);
1,"a field with two LFs

inside",2
@@ -143,14 +155,14 @@
-- test end of copy marker
CREATE TEMP TABLE testeoc (a text);

-COPY testeoc FROM stdin CSV;
+COPY testeoc FROM stdin (CSV);
a\.
\.b
c\.d
"\."
\.

-COPY testeoc TO stdout CSV;
+COPY testeoc TO stdout (CSV);

DROP TABLE x, y;
DROP FUNCTION fn_x_before();
Index: src/test/regress/sql/aggregates.sql
===================================================================
RCS file: /home/manu/cvsrepo/pgsql/src/test/regress/sql/aggregates.sql,v
retrieving revision 1.15
diff -u -r1.15 aggregates.sql
--- src/test/regress/sql/aggregates.sql 25 Apr 2009 16:44:56 -0000      1.15
+++ src/test/regress/sql/aggregates.sql 17 Sep 2009 03:14:48 -0000
@@ -104,7 +104,7 @@
BIT_OR(i4)  AS "?"
FROM bitwise_test;

-COPY bitwise_test FROM STDIN NULL 'null';
+COPY bitwise_test FROM STDIN (NULL 'null');
1      1       1       1       1       B0101
3      3       3       null    2       B0100
7      7       7       3       4       B1100
@@ -171,7 +171,7 @@
BOOL_OR(b3)    AS "n"
FROM bool_test;

-COPY bool_test FROM STDIN NULL 'null';
+COPY bool_test FROM STDIN (NULL 'null');
TRUE   null    FALSE   null
FALSE  TRUE    null    null
null   TRUE    FALSE   null
Index: src/test/regress/sql/copyselect.sql
===================================================================
RCS file: /home/manu/cvsrepo/pgsql/src/test/regress/sql/copyselect.sql,v
retrieving revision 1.2
diff -u -r1.2 copyselect.sql
--- src/test/regress/sql/copyselect.sql 7 Aug 2008 01:11:52 -0000       1.2
+++ src/test/regress/sql/copyselect.sql 17 Sep 2009 03:14:48 -0000
@@ -61,7 +61,7 @@
--
-- Test headers, CSV and quotes
--
-copy (select t from test1 where id = 1) to stdout csv header force quote t;
+copy (select t from test1 where id = 1) to stdout (csv, csv_header,
csv_force_quote (t));
--
-- Test psql builtins, plain table
--
Index: src/test/regress/expected/aggregates.out
===================================================================
RCS file:
/home/manu/cvsrepo/pgsql/src/test/regress/expected/aggregates.out,v
retrieving revision 1.19
diff -u -r1.19 aggregates.out
--- src/test/regress/expected/aggregates.out    25 Apr 2009 16:44:56 -0000
1.19
+++ src/test/regress/expected/aggregates.out    17 Sep 2009 03:14:48 -0000
@@ -326,7 +326,7 @@
|
(1 row)

-COPY bitwise_test FROM STDIN NULL 'null';
+COPY bitwise_test FROM STDIN (NULL 'null');
SELECT
BIT_AND(i2) AS "1",
BIT_AND(i4) AS "1",
@@ -401,7 +401,7 @@
|
(1 row)

-COPY bool_test FROM STDIN NULL 'null';
+COPY bool_test FROM STDIN (NULL 'null');
SELECT
BOOL_AND(b1)     AS "f",
BOOL_AND(b2)     AS "t",
Index: src/test/regress/expected/copyselect.out
===================================================================
RCS file:
/home/manu/cvsrepo/pgsql/src/test/regress/expected/copyselect.out,v
retrieving revision 1.2
diff -u -r1.2 copyselect.out
--- src/test/regress/expected/copyselect.out    7 Aug 2008 01:11:52 -0000
1.2
+++ src/test/regress/expected/copyselect.out    17 Sep 2009 03:14:48 -0000
@@ -93,7 +93,7 @@
--
-- Test headers, CSV and quotes
--
-copy (select t from test1 where id = 1) to stdout csv header force quote t;
+copy (select t from test1 where id = 1) to stdout (csv, csv_header,
csv_force_quote (t));
t
"a"
--
Index: src/test/regress/expected/copy2.out
===================================================================
RCS file: /home/manu/cvsrepo/pgsql/src/test/regress/expected/copy2.out,v
retrieving revision 1.27
diff -u -r1.27 copy2.out
--- src/test/regress/expected/copy2.out 25 Jul 2009 00:07:14 -0000      1.27
+++ src/test/regress/expected/copy2.out 17 Sep 2009 03:14:48 -0000
@@ -47,9 +47,9 @@
ERROR:  extra data after last expected column
CONTEXT:  COPY x, line 1: "2002        232     40      50      60      70
80"
-- various COPY options: delimiters, oids, NULL string
-COPY x (b, c, d, e) from stdin with oids delimiter ',' null 'x';
-COPY x from stdin WITH DELIMITER AS ';' NULL AS '';
-COPY x from stdin WITH DELIMITER AS ':' NULL AS E'\\X';
+COPY x (b, c, d, e) from stdin (oids, delimiter ',', null 'x');
+COPY x from stdin (DELIMITER ';', NULL '');
+COPY x from stdin (DELIMITER ':', NULL E'\\X');
-- check results of copy in
SELECT * FROM x;
a   | b  |     c      |   d    |          e
@@ -89,9 +89,9 @@
INSERT INTO no_oids (a, b) VALUES (5, 10);
INSERT INTO no_oids (a, b) VALUES (20, 30);
-- should fail
-COPY no_oids FROM stdin WITH OIDS;
+COPY no_oids FROM stdin (OIDS);
ERROR:  table "no_oids" does not have OIDs
-COPY no_oids TO stdout WITH OIDS;
+COPY no_oids TO stdout (OIDS);
ERROR:  table "no_oids" does not have OIDs
-- check copy out
COPY x TO stdout;
@@ -146,7 +146,7 @@
stuff  after trigger fired
stuff  after trigger fired
stuff  after trigger fired
-COPY x (b, e) TO stdout WITH NULL 'I''m null';
+COPY x (b, e) TO stdout (NULL 'I''m null');
I'm null       before trigger fired
21     before trigger fired
22     before trigger fired
@@ -195,13 +195,46 @@
"Jackson, Sam","\h"
"It is ""perfect""."," "
"",
+-- Test new 8.5 syntax
+COPY y TO stdout (CSV);
+"Jackson, Sam",\h
+"It is ""perfect"".",
+"",
+COPY y TO stdout (CSV, CSV_QUOTE '''', DELIMITER '|');
+Jackson, Sam|\h
+It is "perfect".|
+''|
+COPY y TO stdout (CSV, CSV_FORCE_QUOTE (col2), CSV_ESCAPE E'\\');
+"Jackson, Sam","\\h"
+"It is \"perfect\".","        "
+"",
+COPY y TO stdout (CSV, CSV_FORCE_QUOTE *);
+"Jackson, Sam","\h"
+"It is ""perfect"".","        "
+"",
+\COPY y TO stdout (CSV)
+"Jackson, Sam",\h
+"It is ""perfect"".",
+"",
+\COPY y TO stdout (CSV, CSV_QUOTE '''', DELIMITER '|')
+Jackson, Sam|\h
+It is "perfect".|
+''|
+\COPY y TO stdout (CSV, CSV_FORCE_QUOTE (col2), CSV_ESCAPE E'\\')
+"Jackson, Sam",\h
+"It is \"perfect\".",
+"",
+\COPY y TO stdout (CSV, CSV_FORCE_QUOTE *)
+"Jackson, Sam",\h
+"It is ""perfect"".",
+"",
--test that we read consecutive LFs properly
CREATE TEMP TABLE testnl (a int, b text, c int);
-COPY testnl FROM stdin CSV;
+COPY testnl FROM stdin (CSV);
-- test end of copy marker
CREATE TEMP TABLE testeoc (a text);
-COPY testeoc FROM stdin CSV;
-COPY testeoc TO stdout CSV;
+COPY testeoc FROM stdin (CSV);
+COPY testeoc TO stdout (CSV);
a\.
\.b
c\.d
Index: doc/src/sgml/ref/copy.sgml
===================================================================
RCS file: /home/manu/cvsrepo/pgsql/doc/src/sgml/ref/copy.sgml,v
retrieving revision 1.87
diff -u -r1.87 copy.sgml
--- doc/src/sgml/ref/copy.sgml  5 Sep 2009 23:58:01 -0000       1.87
+++ doc/src/sgml/ref/copy.sgml  17 Sep 2009 03:14:48 -0000
@@ -24,27 +24,24 @@
<synopsis>
COPY <replaceable class="parameter">tablename</replaceable> [ (
<replaceable class="parameter">column</replaceable> [, ...] ) ]
FROM { '<replaceable class="parameter">filename</replaceable>' | STDIN }
-    [ [ WITH ]
-          [ BINARY ]
-          [ OIDS ]
-          [ DELIMITER [ AS ] '<replaceable
class="parameter">delimiter</replaceable>' ]
-          [ NULL [ AS ] '<replaceable class="parameter">null
string</replaceable>' ]
-          [ CSV [ HEADER ]
-                [ QUOTE [ AS ] '<replaceable
class="parameter">quote</replaceable>' ]
-                [ ESCAPE [ AS ] '<replaceable
class="parameter">escape</replaceable>' ]
-                [ FORCE NOT NULL <replaceable
class="parameter">column</replaceable> [, ...] ]
+    [ ( options [,...] ) ]

COPY { <replaceable class="parameter">tablename</replaceable> [ (
<replaceable class="parameter">column</replaceable> [, ...] ) ] | (
<replaceable class="parameter">query</replaceable> ) }
TO { '<replaceable class="parameter">filename</replaceable>' | STDOUT }
-    [ [ WITH ]
-          [ BINARY ]
-          [ OIDS ]
-          [ DELIMITER [ AS ] '<replaceable
class="parameter">delimiter</replaceable>' ]
-          [ NULL [ AS ] '<replaceable class="parameter">null
string</replaceable>' ]
-          [ CSV [ HEADER ]
-                [ QUOTE [ AS ] '<replaceable
class="parameter">quote</replaceable>' ]
-                [ ESCAPE [ AS ] '<replaceable
class="parameter">escape</replaceable>' ]
-                [ FORCE QUOTE { <replaceable
class="parameter">column</replaceable> [, ...] | * } ]
+    [ ( options [,...] ) ]
+
+Currently available options are:
+    - BINARY [ <replaceable class="parameter">boolean</replaceable> ]
+    - OIDS [ <replaceable class="parameter">boolean</replaceable> ]
+    - DELIMITER '<replaceable class="parameter">delimiter</replaceable>'
+    - NULL '<replaceable class="parameter">null string</replaceable>'
+    - CSV [ <replaceable class="parameter">boolean</replaceable> ]
+    - CSV_HEADER [ <replaceable class="parameter">boolean</replaceable> ]
+    - CSV_QUOTE '<replaceable class="parameter">quote</replaceable>'
+    - CSV_ESCAPE '<replaceable class="parameter">escape</replaceable>'
+    - CSV_FORCE_QUOTE { ( <replaceable
class="parameter">column</replaceable> [, ...] ) | * }
+    - CSV_FORCE_NOT_NULL ( <replaceable
class="parameter">column</replaceable> [, ...] )
+
</synopsis>
</refsynopsisdiv>

@@ -143,6 +140,27 @@
</para>
</listitem>
</varlistentry>
+   </variablelist>
+ </refsect1>
+
+ <refsect1>
+  <title>Options</title>
+
+   <variablelist>
+
+   <varlistentry>
+    <term><replaceable class="parameter">boolean</replaceable></term>
+    <listitem>
+     <para>
+      Specifies whether the selected option should be turned on or off.
+      You can write <literal>TRUE</literal>, <literal>ON</>, or
+      <literal>1</literal> to enable the option, and
<literal>FALSE</literal>,
+      <literal>OFF</>, or <literal>0</literal> to disable it.  The
+      <replaceable class="parameter">boolean</replaceable> value can also
+      be omitted, in which case <literal>TRUE</literal> is assumed.
+     </para>
+    </listitem>
+   </varlistentry>

<varlistentry>
<term><literal>BINARY</literal></term>
@@ -168,7 +186,7 @@
</varlistentry>

<varlistentry>
-    <term><replaceable class="parameter">delimiter</replaceable></term>
+    <term><literal>DELIMITER</literal></term>
<listitem>
<para>
The single ASCII character that separates columns within each row
@@ -179,7 +197,7 @@
</varlistentry>

<varlistentry>
-    <term><replaceable class="parameter">null string</replaceable></term>
+    <term><literal>NULL</literal></term>
<listitem>
<para>
The string that represents a null value. The default is
@@ -211,7 +229,7 @@
</varlistentry>

<varlistentry>
-    <term><literal>HEADER</literal></term>
+    <term><literal>CSV_HEADER</literal></term>
<listitem>
<para>
Specifies that the file contains a header line with the names of each
@@ -222,7 +240,7 @@
</varlistentry>

<varlistentry>
-    <term><replaceable class="parameter">quote</replaceable></term>
+    <term><literal>CSV_QUOTE</literal></term>
<listitem>
<para>
Specifies the ASCII quotation character in <literal>CSV</> mode.
@@ -232,18 +250,18 @@
</varlistentry>

<varlistentry>
-    <term><replaceable class="parameter">escape</replaceable></term>
+    <term><literal>CSV_ESCAPE</literal></term>
<listitem>
<para>
Specifies the ASCII character that should appear before a
-      <literal>QUOTE</> data character value in <literal>CSV</> mode.
-      The default is the <literal>QUOTE</> value (usually double-quote).
+      <literal>CSV_QUOTE</> data character value in <literal>CSV</> mode.
+      The default is the <literal>CSV_QUOTE</> value (usually
double-quote).
</para>
</listitem>
</varlistentry>

<varlistentry>
-    <term><literal>FORCE QUOTE</></term>
+    <term><literal>CSV_FORCE_QUOTE</></term>
<listitem>
<para>
In <literal>CSV</> <command>COPY TO</> mode, forces quoting to be
@@ -256,7 +274,7 @@
</varlistentry>

<varlistentry>
-    <term><literal>FORCE NOT NULL</></term>
+    <term><literal>CSV_FORCE_NOT_NULL</></term>
<listitem>
<para>
In <literal>CSV</> <command>COPY FROM</> mode, process each
@@ -295,7 +313,7 @@
</para>

<para>
-    The <literal>BINARY</literal> key word causes all data to be
+    The <literal>BINARY</literal> option causes all data to be
stored/read as binary format rather than as text.  It is
somewhat faster than the normal text mode, but a binary-format
file is less portable across machine architectures and
@@ -538,12 +556,12 @@
<para>
The values in each record are separated by the <literal>DELIMITER</>
character. If the value contains the delimiter character, the
-    <literal>QUOTE</> character, the <literal>NULL</> string, a carriage
+    <literal>CSV_QUOTE</> character, the <literal>NULL</> string, a
carriage
return, or line feed character, then the whole value is prefixed and
-    suffixed by the <literal>QUOTE</> character, and any occurrence
-    within the value of a <literal>QUOTE</> character or the
-    <literal>ESCAPE</> character is preceded by the escape character.
-    You can also use <literal>FORCE QUOTE</> to force quotes when
outputting
+    suffixed by the <literal>CSV_QUOTE</> character, and any occurrence
+    within the value of a <literal>CSV_QUOTE</> character or the
+    <literal>CSV_ESCAPE</> character is preceded by the escape character.
+    You can also use <literal>CSV_FORCE_QUOTE</> to force quotes when
outputting
non-<literal>NULL</> values in specific columns.
</para>

@@ -557,7 +575,7 @@
settings, a <literal>NULL</> is written as an unquoted empty
string, while an empty string is written with double quotes
(<literal>""</>). Reading values follows similar rules. You can
-    use <literal>FORCE NOT NULL</> to prevent <literal>NULL</> input
+    use <literal>CSV_FORCE_NOT_NULL</> to prevent <literal>NULL</> input
comparisons for specific columns.
</para>

@@ -577,7 +595,7 @@
<para>
In <literal>CSV</> mode, all characters are significant. A quoted value
surrounded by white space, or any characters other than
-     <literal>DELIMITER</>, will include those characters. This can cause
+     <literal>CSV_DELIMITER</>, will include those characters. This can
cause
errors if you import data from a system that pads <literal>CSV</>
lines with white space out to some fixed width. If such a situation
arises you might need to preprocess the <literal>CSV</> file to remove
@@ -759,7 +777,7 @@
The following example copies a table to the client
using the vertical bar (<literal>|</literal>) as the field delimiter:
<programlisting>
-COPY country TO STDOUT WITH DELIMITER '|';
+COPY country TO STDOUT (DELIMITER '|');
</programlisting>
</para>

@@ -809,6 +827,12 @@
0000200   M   B   A   B   W   E 377 377 377 377 377 377
</programlisting>
</para>
+  <para>
+  Multiple options are separated by a comma like:
+<programlisting>
+COPY (SELECT t FROM foo WHERE id = 1) TO STDOUT (CSV, CSV_HEADER,
CSV_FORCE_QUOTE (t));
+</programlisting>
+  </para>
</refsect1>

<refsect1>
@@ -817,7 +841,35 @@
<para>
There is no <command>COPY</command> statement in the SQL standard.
</para>
+  <para>
+     The following syntax was used before <productname>PostgreSQL</>
+   version 8.5 and is still supported:
+<synopsis>
+COPY <replaceable class="parameter">tablename</replaceable> [ (
<replaceable class="parameter">column</replaceable> [, ...] ) ]
+    FROM { '<replaceable class="parameter">filename</replaceable>' | STDIN
}
+    [ [ WITH ]
+          [ BINARY ]
+          [ OIDS ]
+          [ DELIMITER [ AS ] '<replaceable
class="parameter">delimiter</replaceable>' ]
+          [ NULL [ AS ] '<replaceable class="parameter">null
string</replaceable>' ]
+          [ CSV [ HEADER ]
+                [ QUOTE [ AS ] '<replaceable
class="parameter">quote</replaceable>' ]
+                [ ESCAPE [ AS ] '<replaceable
class="parameter">escape</replaceable>' ]
+                [ FORCE NOT NULL <replaceable
class="parameter">column</replaceable> [, ...] ]

+COPY { <replaceable class="parameter">tablename</replaceable> [ (
<replaceable class="parameter">column</replaceable> [, ...] ) ] | (
<replaceable class="parameter">query</replaceable> ) }
+    TO { '<replaceable class="parameter">filename</replaceable>' | STDOUT }
+    [ [ WITH ]
+          [ BINARY ]
+          [ OIDS ]
+          [ DELIMITER [ AS ] '<replaceable
class="parameter">delimiter</replaceable>' ]
+          [ NULL [ AS ] '<replaceable class="parameter">null
string</replaceable>' ]
+          [ CSV [ HEADER ]
+                [ QUOTE [ AS ] '<replaceable
class="parameter">quote</replaceable>' ]
+                [ ESCAPE [ AS ] '<replaceable
class="parameter">escape</replaceable>' ]
+                [ FORCE QUOTE { <replaceable
class="parameter">column</replaceable> [, ...] | * } ]
+</synopsis>
+  </para>
<para>
The following syntax was used before <productname>PostgreSQL</>
version 7.3 and is still supported:
Index: src/backend/parser/gram.y
===================================================================
RCS file: /home/manu/cvsrepo/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.677
diff -u -r2.677 gram.y
--- src/backend/parser/gram.y   18 Aug 2009 23:40:20 -0000      2.677
+++ src/backend/parser/gram.y   17 Sep 2009 03:14:48 -0000
@@ -373,6 +373,10 @@
%type <node>   explain_option_arg
%type <defelt> explain_option_elem
%type <list>   explain_option_list
+%type <str>            copy_generic_option_name
+%type <node>   copy_generic_option_arg copy_generic_option_arg_item
+%type <defelt> copy_generic_option_elem
+%type <list>   copy_generic_option_list copy_generic_option_arg_list

%type <typnam> Typename SimpleTypename ConstTypename
GenericType Numeric opt_float
@@ -1934,14 +1938,19 @@
/*****************************************************************************
*
*             QUERY :
- *                             COPY relname ['(' columnList ')'] FROM/TO
file [WITH options]
- *
- *                             BINARY, OIDS, and DELIMITERS kept in old
locations
- *                             for backward compatibility.  2002-06-18
+ *                             New, more generic syntax, supported
beginning with PostgreSQL
+ *                             8.5.  Options are comma-separated.
+ *                             COPY relname ['(' columnList ')'] FROM/TO
file '(' options ')'
*
+ *                             Older syntax, used from 7.3 to 8.4 and still
supported for
+ *                             backwards compatibility
+ *                             COPY relname ['(' columnList ')'] FROM/TO
file [WITH options]
*                             COPY ( SELECT ... ) TO file [WITH options]
- *                             This form doesn't have the
backwards-compatible option
- *                             syntax.
+ *
+ *                             Really old syntax, from versions 7.2 and
prior:
+ *                             COPY [ BINARY ] table [ WITH OIDS ] FROM/TO
file
+ *                                     [ [ USING ] DELIMITERS 'delimiter' ]
]
+ *                                     [ WITH NULL AS 'null string' ]
*
*****************************************************************************/

@@ -2001,6 +2010,7 @@

copy_opt_list:
copy_opt_list copy_opt_item
{ $$ = lappend($1, $2); }
+ | '(' copy_generic_option_list ')' { $$
= $2 ; }
| /* EMPTY */
{ $$ = NIL; }
;

@@ -2028,27 +2038,27 @@
}
| HEADER_P
{
-                                       $$ = makeDefElem("header", (Node
*)makeInteger(TRUE));
+                                       $$ = makeDefElem("csv_header", (Node
*)makeInteger(TRUE));
}
| QUOTE opt_as Sconst
{
-                                       $$ = makeDefElem("quote", (Node
*)makeString($3));
+                                       $$ = makeDefElem("csv_quote", (Node
*)makeString($3));
}
| ESCAPE opt_as Sconst
{
-                                       $$ = makeDefElem("escape", (Node
*)makeString($3));
+                                       $$ = makeDefElem("csv_escape", (Node
*)makeString($3));
}
| FORCE QUOTE columnList
{
-                                       $$ = makeDefElem("force_quote",
(Node *)$3);
+                                       $$ = makeDefElem("csv_force_quote",
(Node *)$3);
}
| FORCE QUOTE '*'
{
-                                       $$ = makeDefElem("force_quote",
(Node *)makeNode(A_Star));
+                                       $$ = makeDefElem("csv_force_quote",
(Node *)makeNode(A_Star));
}
| FORCE NOT NULL_P columnList
{
-                                       $$ = makeDefElem("force_notnull",
(Node *)$4);
+                                       $$ =
makeDefElem("csv_force_not_null", (Node *)$4);
}
;

@@ -2084,6 +2094,53 @@
| /*EMPTY*/
{}
;

+copy_generic_option_list:
+                       copy_generic_option_elem
+                               {
+                                       $$ = list_make1($1);
+                               }
+                       | copy_generic_option_list ','
copy_generic_option_elem
+                               {
+                                       $$ = lappend($1, $3);
+                               }
+               ;
+
+copy_generic_option_elem:
+                       copy_generic_option_name copy_generic_option_arg
+                               {
+                                       $$ = makeDefElem($1, $2);
+                               }
+               ;
+
+copy_generic_option_name:
+                       ColLabel
{ $$ = $1; }
+               ;
+
+copy_generic_option_arg:
+                         copy_generic_option_arg_item                  { $$
= $1; }
+                       | '(' copy_generic_option_arg_list ')'  { $$ = (Node
*) $2; }
+                       | '*'
{ $$ = (Node *)makeNode(A_Star); }
+                       | '(' ')'
{ $$ = NULL; }
+                       | /* EMPTY */
{ $$ = NULL; }
+               ;
+
+copy_generic_option_arg_list:
+                         copy_generic_option_arg_item
+                               {
+                                       $$ = list_make1($1);
+                               }
+                       | copy_generic_option_arg_list ','
copy_generic_option_arg_item
+                               {
+                                       $$ = lappend($1, $3);
+                               }
+               ;
+
+copy_generic_option_arg_item:
+                       opt_boolean                             { $$ = (Node
*) makeString($1); }
+                       | ColId_or_Sconst               { $$ = (Node *)
makeString($1); }
+                       | NumericOnly                   { $$ = (Node *) $1;
}
+               ;
+

/*****************************************************************************
*
Index: src/bin/psql/copy.c
===================================================================
RCS file: /home/manu/cvsrepo/pgsql/src/bin/psql/copy.c,v
retrieving revision 1.82
diff -u -r1.82 copy.c
--- src/bin/psql/copy.c 7 Aug 2009 20:16:11 -0000       1.82
+++ src/bin/psql/copy.c 17 Sep 2009 03:14:48 -0000
@@ -26,20 +26,27 @@
#include "prompt.h"
#include "stringutils.h"

-
/*
* parse_slash_copy
* -- parses \copy command line
*
* The documented syntax is:
+ * Since 8.5:
+ *  \copy tablename [(columnlist)] from|to filename [( options )]
+ *
+ * options is a comma separated list of options. Currently supported
options:
+ * binary, oids, delimiter, null, csv, csv_header, csv_espace, csv_quote,
+ * csv_force_not_null, csv_force_quote
+ *
+ * Prior 8.5:
*     \copy tablename [(columnlist)] from|to filename
*       [ with ] [ binary ] [ oids ] [ delimiter [as] char ] [ null [as]
string ]
- *       [ csv  [ header ] [ quote [ AS ] string ]  escape [as] string
+ *       [ csv  [ header ] [ quote [ AS ] string ] [ escape [as] string ]
*             [ force not null column [, ...] | force quote column [, ...]
| * ] ]
*
*     \copy ( select stmt ) to filename
*       [ with ] [ binary ] [ delimiter [as] char ] [ null [as] string ]
- *       [ csv  [ header ] [ quote [ AS ] string ]  escape [as] string
+ *       [ csv  [ header ] [ quote [ AS ] string ] [ escape [as] string ]
*             [ force quote column [, ...] | * ] ]
*
* Force quote only applies for copy to; force not null only applies for
@@ -103,6 +110,45 @@
*var = newvar;
}

+/* Extract parameters of the CSV_FORCE_QUOTE or CSV_FORCE_NOT_NULL options.
+ * Return true if the list was successfully extracted or false if an error
+ * occurred. */
+static bool extract_csv_force_list(char *list, char *token)
+{
+       const char *whitespace = " \t\n\r";
+       const char *separator = "(,)";
+
+       for (;;)
+       {
+               token = strtokx(NULL, whitespace, separator, "\"", 0, false,
false,
+                               pset.encoding);
+
+               if (!token)
+                       return false;
+               if (strchr(separator, token[0]) != NULL)
+                       continue; /* fetch next token */
+               if (!list)
+               {
+                       list = pg_strdup(token);
+                       if (token[0] == '*')
+                               return true;
+               }
+               else
+               {
+                       if (token[0] == '*') /* '*' can only be accepted as
a single argument */
+                               return false;
+                       xstrcat(&list, token);
+               }
+               token = strtokx(NULL, whitespace, separator, "\"", 0, false,
false,
+                               pset.encoding);
+               if (!token || strchr(")", token[0]))
+                       break;
+               if (strchr(separator, token[0]) != NULL)
+                       continue; // fetch next token
+               xstrcat(&list, token);
+       }
+       return true;
+}

static struct copy_options *
parse_slash_copy(const char *args)
@@ -241,18 +287,29 @@
expand_tilde(&result->file);
}

-       token = strtokx(NULL, whitespace, NULL, NULL,
+       token = strtokx(NULL, whitespace, "(", NULL,
0, false, false, pset.encoding);

if (token)
{
+               char *option_separator = NULL;
+
+               /* New 8.5 syntax, option are between () */
+               if (token[0] == '(')
+               {
+                       option_separator = ",)";
+                       token = strtokx(NULL, whitespace, NULL,
option_separator, 0, false, false,
+                                       pset.encoding);
+               }
/*
* WITH is optional.  Also, the backend will allow WITH
followed by
* nothing, so we do too.
*/
-               if (pg_strcasecmp(token, "with") == 0)
-                       token = strtokx(NULL, whitespace, NULL, NULL,
-                                                       0, false, false,
pset.encoding);
+               else if (pg_strcasecmp(token, "with") == 0)
+               {
+                       token = strtokx(NULL, whitespace, NULL, NULL, 0,
false, false,
+                                       pset.encoding);
+               }

while (token)
{
@@ -266,7 +323,8 @@
result->binary = true;
else if (pg_strcasecmp(token, "csv") == 0)
result->csv_mode = true;
-                       else if (pg_strcasecmp(token, "header") == 0)
+                       else if ((pg_strcasecmp(token, "header") == 0) ||
(pg_strcasecmp(
+                                       token, "csv_header") == 0))
result->header = true;
else if (pg_strcasecmp(token, "delimiter") == 0)
{
@@ -296,7 +354,8 @@
else
goto error;
}
-                       else if (pg_strcasecmp(token, "quote") == 0)
+                       else if ((pg_strcasecmp(token, "quote") == 0) ||
(pg_strcasecmp(
+                                       token, "csv_quote") == 0))
{
if (result->quote)
goto error;
@@ -310,7 +369,8 @@
else
goto error;
}
-                       else if (pg_strcasecmp(token, "escape") == 0)
+                       else if ((pg_strcasecmp(token, "escape") == 0) ||
(pg_strcasecmp(
+                                       token, "csv_escape") == 0))
{
if (result->escape)
goto error;
@@ -334,22 +394,8 @@
goto error;
/* handle column list */
fetch_next = false;
-                                       for (;;)
-                                       {
-                                               token = strtokx(NULL,
whitespace, ",", "\"",
-
0, false, false, pset.encoding);
-                                               if (!token || strchr(",",
token[0]))
-                                                       goto error;
-                                               if
(!result->force_quote_list)
-
result->force_quote_list = pg_strdup(token);
-                                               else
-
xstrcat(&result->force_quote_list, token);
-                                               token = strtokx(NULL,
whitespace, ",", "\"",
-
0, false, false, pset.encoding);
-                                               if (!token || token[0] !=
',')
-                                                       break;
-
xstrcat(&result->force_quote_list, token);
-                                       }
+                                       if
(!extract_csv_force_list(result->force_quote_list, token))
+                                               goto error;
}
else if (pg_strcasecmp(token, "not") == 0)
{
@@ -361,32 +407,40 @@
goto error;
/* handle column list */
fetch_next = false;
-                                       for (;;)
-                                       {
-                                               token = strtokx(NULL,
whitespace, ",", "\"",
-
0, false, false, pset.encoding);
-                                               if (!token || strchr(",",
token[0]))
-                                                       goto error;
-                                               if
(!result->force_notnull_list)
-
result->force_notnull_list = pg_strdup(token);
-                                               else
-
xstrcat(&result->force_notnull_list, token);
-                                               token = strtokx(NULL,
whitespace, ",", "\"",
-
0, false, false, pset.encoding);
-                                               if (!token || token[0] !=
',')
-                                                       break;
-
xstrcat(&result->force_notnull_list, token);
-                                       }
+                                       if
(!extract_csv_force_list(result->force_notnull_list,
+                                                       token))
+                                               goto error;
}
else
goto error;
}
+                       else if (pg_strcasecmp(token, "csv_force_quote") ==
0)
+                       {
+                               if (result->force_quote_list)
+                                       goto error;
+                               if
(!extract_csv_force_list(result->force_quote_list, token))
+                                       goto error;
+                       }
+                       else if (pg_strcasecmp(token, "csv_force_not_null")
== 0)
+                       {
+                               if (result->force_notnull_list)
+                                       goto error;
+                               if
(!extract_csv_force_list(result->force_notnull_list, token))
+                                       goto error;
+                       }
else
goto error;

if (fetch_next)
-                               token = strtokx(NULL, whitespace, NULL,
NULL,
-                                                               0, false,
false, pset.encoding);
+                       {
+                               token = strtokx(NULL, whitespace,
option_separator, NULL, 0,
+                                               false, false,
pset.encoding);
+                               if ((option_separator != NULL) &&
(strchr(option_separator, token[0])))
+                               {
+                                       token = strtokx(NULL, whitespace,
option_separator, NULL,
+                                                       0, false, false,
pset.encoding);
+                               }
+                       }
}
}

Index: src/test/regress/output/copy.source
===================================================================
RCS file: /home/manu/cvsrepo/pgsql/src/test/regress/output/copy.source,v
retrieving revision 1.13
diff -u -r1.13 copy.source
--- src/test/regress/output/copy.source 21 Aug 2007 01:11:31 -0000      1.13
+++ src/test/regress/output/copy.source 17 Sep 2009 03:14:48 -0000
@@ -71,3 +71,49 @@
c1,"col with , comma","col with "" quote"
1,a,1
2,b,2
+-- Repeat the above tests with the new 8.5 option syntax
+copy copytest to
'/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv'
(csv);
+truncate copytest2;
+copy copytest2 from
'/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv'
(csv);
+select * from copytest except select * from copytest2;
+ style | test | filler
+-------+------+--------
+(0 rows)
+
+truncate copytest2;
+copy copytest to
'/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv'
(csv, csv_quote '''', csv_escape E'\\');
+copy copytest2 from
'/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv'
(csv, csv_quote '''', csv_escape E'\\');
+select * from copytest except select * from copytest2;
+ style | test | filler
+-------+------+--------
+(0 rows)
+
+truncate copytest3;
+copy copytest3 from stdin (csv, csv_header);
+copy copytest3 to stdout (csv, csv_header);
+c1,"col with , comma","col with "" quote"
+1,a,1
+2,b,2
+-- Repeat the above tests with the new 8.5 option syntax from psql
+\copy copytest to
'/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv'
(csv)
+truncate copytest2;
+\copy copytest2 from
'/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv'
(csv)
+select * from copytest except select * from copytest2;
+ style | test | filler
+-------+------+--------
+(0 rows)
+
+truncate copytest2;
+\copy copytest to
'/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv'
(csv, csv_quote '''', csv_escape E'\\')
+\copy copytest2 from
'/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv'
(csv, csv_quote '''', csv_escape E'\\')
+select * from copytest except select * from copytest2;
+ style | test | filler
+-------+------+--------
+(0 rows)
+
+truncate copytest3;
+\copy copytest3 from stdin (csv, csv_header)
+\copy copytest3 to stdout (csv, csv_header)
+c1,"col with , comma","col with "" quote"
+1,a,1
+2,b,2
Index: src/test/regress/input/copy.source
===================================================================
RCS file: /home/manu/cvsrepo/pgsql/src/test/regress/input/copy.source,v
retrieving revision 1.15
diff -u -r1.15 copy.source
--- src/test/regress/input/copy.source  21 Aug 2007 01:11:31 -0000      1.15
+++ src/test/regress/input/copy.source  17 Sep 2009 03:14:48 -0000
@@ -107,3 +107,58 @@

copy copytest3 to stdout csv header;

+-- Repeat the above tests with the new 8.5 option syntax
+
+copy copytest to '@abs_builddir@/results/copytest.csv' (csv);
+
+truncate copytest2;
+
+copy copytest2 from '@abs_builddir@/results/copytest.csv' (csv);
+
+select * from copytest except select * from copytest2;
+
+truncate copytest2;
+
+copy copytest to '@abs_builddir@/results/copytest.csv' (csv, csv_quote
'''', csv_escape E'\\');
+
+copy copytest2 from '@abs_builddir@/results/copytest.csv' (csv, csv_quote
'''', csv_escape E'\\');
+
+select * from copytest except select * from copytest2;
+
+truncate copytest3;
+
+copy copytest3 from stdin (csv, csv_header);
+this is just a line full of junk that would error out if parsed
+1,a,1
+2,b,2
+\.
+
+copy copytest3 to stdout (csv, csv_header);
+
+-- Repeat the above tests with the new 8.5 option syntax from psql
+
+\copy copytest to '@abs_builddir@/results/copytest.csv' (csv)
+
+truncate copytest2;
+
+\copy copytest2 from '@abs_builddir@/results/copytest.csv' (csv)
+
+select * from copytest except select * from copytest2;
+
+truncate copytest2;
+
+\copy copytest to '@abs_builddir@/results/copytest.csv' (csv, csv_quote
'''', csv_escape E'\\')
+
+\copy copytest2 from '@abs_builddir@/results/copytest.csv' (csv, csv_quote
'''', csv_escape E'\\')
+
+select * from copytest except select * from copytest2;
+
+truncate copytest3;
+
+\copy copytest3 from stdin (csv, csv_header)
+this is just a line full of junk that would error out if parsed
+1,a,1
+2,b,2
+\.
+
+\copy copytest3 to stdout (csv, csv_header)
Index: src/backend/commands/copy.c
===================================================================
RCS file: /home/manu/cvsrepo/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.316
diff -u -r1.316 copy.c
--- src/backend/commands/copy.c 29 Jul 2009 20:56:18 -0000      1.316
+++ src/backend/commands/copy.c 17 Sep 2009 03:14:48 -0000
@@ -25,6 +25,7 @@
#include "catalog/namespace.h"
#include "catalog/pg_type.h"
#include "commands/copy.h"
+#include "commands/defrem.h"
#include "commands/trigger.h"
#include "executor/executor.h"
#include "libpq/libpq.h"
@@ -745,7 +746,7 @@
ereport(ERROR,

(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or
redundant options")));
-                       cstate->binary = intVal(defel->arg);
+                       cstate->binary = defGetBoolean(defel);
}
else if (strcmp(defel->defname, "oids") == 0)
{
@@ -753,7 +754,7 @@
ereport(ERROR,

(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or
redundant options")));
-                       cstate->oids = intVal(defel->arg);
+                       cstate->oids = defGetBoolean(defel);
}
else if (strcmp(defel->defname, "delimiter") == 0)
{
@@ -761,7 +762,7 @@
ereport(ERROR,

(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or
redundant options")));
-                       cstate->delim = strVal(defel->arg);
+                       cstate->delim = defGetString(defel);
}
else if (strcmp(defel->defname, "null") == 0)
{
@@ -769,7 +770,7 @@
ereport(ERROR,

(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or
redundant options")));
-                       cstate->null_print = strVal(defel->arg);
+                       cstate->null_print = defGetString(defel);
}
else if (strcmp(defel->defname, "csv") == 0)
{
@@ -777,33 +778,33 @@
ereport(ERROR,

(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or
redundant options")));
-                       cstate->csv_mode = intVal(defel->arg);
+                       cstate->csv_mode = defGetBoolean(defel);
}
-               else if (strcmp(defel->defname, "header") == 0)
+               else if (strcmp(defel->defname, "csv_header") == 0)
{
if (cstate->header_line)
ereport(ERROR,

(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or
redundant options")));
-                       cstate->header_line = intVal(defel->arg);
+                       cstate->header_line = defGetBoolean(defel);
}
-               else if (strcmp(defel->defname, "quote") == 0)
+               else if (strcmp(defel->defname, "csv_quote") == 0)
{
if (cstate->quote)
ereport(ERROR,

(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or
redundant options")));
-                       cstate->quote = strVal(defel->arg);
+                       cstate->quote = defGetString(defel);
}
-               else if (strcmp(defel->defname, "escape") == 0)
+               else if (strcmp(defel->defname, "csv_escape") == 0)
{
if (cstate->escape)
ereport(ERROR,

(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or
redundant options")));
-                       cstate->escape = strVal(defel->arg);
+                       cstate->escape = defGetString(defel);
}
-               else if (strcmp(defel->defname, "force_quote") == 0)
+               else if (strcmp(defel->defname, "csv_force_quote") == 0)
{
if (force_quote || force_quote_all)
ereport(ERROR,
@@ -811,20 +812,45 @@
errmsg("conflicting or
redundant options")));
if (defel->arg && IsA(defel->arg, A_Star))
force_quote_all = true;
-                       else
+                       else if (defel->arg && IsA(defel->arg, List))
+                       {
+                               ListCell *lc;
+
force_quote = (List *) defel->arg;
+                               foreach (lc, force_quote)
+                               {
+                                       if (!IsA(lfirst(lc), String))
+                                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                                        errmsg("argument to
option \"%s\" must be a list of column names",
+
defel->defname)));
+                               }
+                       }
+                       else
+                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                        errmsg("argument to option \"%s\"
must be a list of column names",
+                                                       defel->defname)));
}
-               else if (strcmp(defel->defname, "force_notnull") == 0)
+               else if (strcmp(defel->defname, "csv_force_not_null") == 0)
{
if (force_notnull)
ereport(ERROR,

(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or
redundant options")));
-                       force_notnull = (List *) defel->arg;
+                       if (defel->arg && IsA(defel->arg, List))
+                               force_notnull = (List *) defel->arg;
+                       else
+                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                        errmsg("argument to option \"%s\"
must be a list",
+                                                       defel->defname)));
}
else
-                       elog(ERROR, "option \"%s\" not recognized",
-                                defel->defname);
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_SYNTAX_ERROR),
+                                        errmsg("option \"%s\" not
recognized",
+                                                       defel->defname)));
}

/* Check for incompatible options */

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

--
Emmanuel Cecchet
Aster Data Systems
Web: http://www.asterdata.com

Tom Lane wrote:

psql has MORE need to support old syntax than the backend does, because
it's supposed to work against old servers.

I wonder though if we couldn't simplify matters. Offhand it seems to me
that psql doesn't need to validate the command's syntax fully. All it
really needs to do is find the target filename and replace it with
STDIN/STDOUT. Could we have it just treat the remainder of the line
literally, and not worry about the details of what the options might be?
Let the backend worry about throwing an error if they're bad.

Makes plenty of sense.

On a related topic, I'm not sure how we would go about providing psql
support for the suggested copy-as-from-target feature that's been
discussed recently. That could get mildly ugly.

cheers

andrew

Tom Lane wrote:

While I understand the need for the server to still support the syntax,
is it necessary for newer version of psql to support the old syntax?

psql has MORE need to support old syntax than the backend does, because
it's supposed to work against old servers.

Well, I wonder how many users just upgrade psql vs upgrade the server. I
was thinking that when users perform a database upgrade their
application often remain the same and therefore the server needs to
support the old syntax. Unless you are upgrading a machine where a bunch
of psql-based scripts are running to update various remote Postgres
instances with older versions, I would guess that it is unlikely that
someone is going to upgrade psql and keep the old instance of the server
on the same machine.
I just wonder how many users are using a single psql to manage multiple
server instances of different older versions.

I wonder though if we couldn't simplify matters. Offhand it seems to me
that psql doesn't need to validate the command's syntax fully. All it
really needs to do is find the target filename and replace it with
STDIN/STDOUT. Could we have it just treat the remainder of the line
literally, and not worry about the details of what the options might be?
Let the backend worry about throwing an error if they're bad.

As the only difference between \copy and copy seems to be the ability to
stream the file from the client, I guess that everything else should be
sent as is to the server as you suggest. I'll come with a patch for that
today.

Emmanuel

--
Emmanuel Cecchet
Aster Data Systems
Web: http://www.asterdata.com

2009/9/17 Emmanuel Cecchet <manu@asterdata.com>:

Tom Lane wrote:

While I understand the need for the server to still support the syntax,
is it necessary for newer version of psql to support the old syntax?

psql has MORE need to support old syntax than the backend does, because
it's supposed to work against old servers.

Well, I wonder how many users just upgrade psql vs upgrade the server. I was
thinking that when users perform a database upgrade their application often
remain the same and therefore the server needs to support the old syntax.
Unless you are upgrading a machine where a bunch of psql-based scripts are
running to update various remote Postgres instances with older versions, I
would guess that it is unlikely that someone is going to upgrade psql and
keep the old instance of the server on the same machine.
I just wonder how many users are using a single psql to manage multiple
server instances of different older versions.

What application, that use current copy format for fast data import? I
thing, so doing incompatible changes of copy statement syntax is very
bad idea.

regards
Pavel Stehule

Pavel Stehule wrote:

Well, I wonder how many users just upgrade psql vs upgrade the server. I was
thinking that when users perform a database upgrade their application often
remain the same and therefore the server needs to support the old syntax.
Unless you are upgrading a machine where a bunch of psql-based scripts are
running to update various remote Postgres instances with older versions, I
would guess that it is unlikely that someone is going to upgrade psql and
keep the old instance of the server on the same machine.
I just wonder how many users are using a single psql to manage multiple
server instances of different older versions.

What application, that use current copy format for fast data import? I
thing, so doing incompatible changes of copy statement syntax is very
bad idea.

The old syntax is still supported in both psql and the server but I am
not sure how many applications are relying on psql to perform a copy
operation (actually a \copy).

manu

Emmanuel Cecchet <manu@asterdata.com> wrote:

I just wonder how many users are using a single psql to manage
multiple server instances of different older versions.

I do that, but I do try to keep all the active versions on my machine,
so that I can use one which exactly matches any of our 100 servers
when it matters. (Or I can ssh to the server and use its psql.)

-Kevin

2009/9/17 Emmanuel Cecchet <manu@asterdata.com>:

Pavel Stehule wrote:

Well, I wonder how many users just upgrade psql vs upgrade the server. I
was
thinking that when users perform a database upgrade their application
often
remain the same and therefore the server needs to support the old syntax.
Unless you are upgrading a machine where a bunch of psql-based scripts
are
running to update various remote Postgres instances with older versions,
I
would guess that it is unlikely that someone is going to upgrade psql and
keep the old instance of the server on the same machine.
I just wonder how many users are using a single psql to manage multiple
server instances of different older versions.

What application, that use current copy format for fast data import? I
thing, so doing incompatible changes of copy statement syntax is very
bad idea.

The old syntax is still supported in both psql and the server but I am not
sure how many applications are relying on psql to perform a copy operation
(actually a \copy).

who knows. \copy is very useful thinks and people who imports data
from local use it. I am sure, so this feature is often used, mainly by
unix dba.

regards
Pavel

Emmanuel Cecchet <manu@asterdata.com> writes:

Tom Lane wrote:

psql has MORE need to support old syntax than the backend does, because
it's supposed to work against old servers.

Well, I wonder how many users just upgrade psql vs upgrade the server.

We have established a project policy that psql backslash commands will
support servers at least back to 7.4, and a great deal of work has
already been expended in support of that goal. It is not within the
charter of this patch to ignore or redefine that policy.

regards, tom lane

Tom Lane wrote:

Emmanuel Cecchet <manu@asterdata.com> writes:

Tom Lane wrote:

psql has MORE need to support old syntax than the backend does, because
it's supposed to work against old servers.

Well, I wonder how many users just upgrade psql vs upgrade the server.

We have established a project policy that psql backslash commands will
support servers at least back to 7.4, and a great deal of work has
already been expended in support of that goal. It is not within the
charter of this patch to ignore or redefine that policy.

Does that mean that we can drop the 7.3 syntax or should we just keep it?
For future references, where can I find the various project policies?

Thanks
Emmanuel

--
Emmanuel Cecchet
Aster Data Systems
Web: http://www.asterdata.com