On Fri, Sep 11, 2009 at 5:45 PM, Robert Haas <[email protected]> wrote:
> On Fri, Sep 11, 2009 at 5:32 PM, Tom Lane <[email protected]> wrote:
>> Robert Haas <[email protected]> writes:
>>> The biggest problem I have with this change is that it's going to
>>> massively break anyone who is using the existing COPY syntax.
>>
>> Why? We'd certainly still support the old syntax for existing options,
>> just as we did with EXPLAIN.
>
> None of the syntax proposals upthread had that property, which doesn't
> mean we can't do it. However, we'd need some way to differentiate the
> old syntax from the new one. I guess we could throw an unnecessary set
> of parentheses around the option list (blech), but you have to be able
> to tell from the first token which kind of list you're reading if you
> want to support both sets of syntax.
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.
...Robert
*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
***************
*** 25,30 ****
--- 25,31 ----
#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,751 **** DoCopy(const CopyStmt *stmt, const char *queryString)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->binary = intVal(defel->arg);
}
else if (strcmp(defel->defname, "oids") == 0)
{
--- 746,752 ----
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->binary = defGetBoolean(defel);
}
else if (strcmp(defel->defname, "oids") == 0)
{
***************
*** 753,759 **** DoCopy(const CopyStmt *stmt, const char *queryString)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->oids = intVal(defel->arg);
}
else if (strcmp(defel->defname, "delimiter") == 0)
{
--- 754,760 ----
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->oids = defGetBoolean(defel);
}
else if (strcmp(defel->defname, "delimiter") == 0)
{
***************
*** 761,767 **** DoCopy(const CopyStmt *stmt, const char *queryString)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->delim = strVal(defel->arg);
}
else if (strcmp(defel->defname, "null") == 0)
{
--- 762,768 ----
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->delim = defGetString(defel);
}
else if (strcmp(defel->defname, "null") == 0)
{
***************
*** 769,775 **** DoCopy(const CopyStmt *stmt, const char *queryString)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->null_print = strVal(defel->arg);
}
else if (strcmp(defel->defname, "csv") == 0)
{
--- 770,776 ----
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->null_print = defGetString(defel);
}
else if (strcmp(defel->defname, "csv") == 0)
{
***************
*** 777,783 **** DoCopy(const CopyStmt *stmt, const char *queryString)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->csv_mode = intVal(defel->arg);
}
else if (strcmp(defel->defname, "header") == 0)
{
--- 778,784 ----
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->csv_mode = defGetBoolean(defel);
}
else if (strcmp(defel->defname, "header") == 0)
{
***************
*** 785,791 **** DoCopy(const CopyStmt *stmt, const char *queryString)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->header_line = intVal(defel->arg);
}
else if (strcmp(defel->defname, "quote") == 0)
{
--- 786,792 ----
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->header_line = defGetBoolean(defel);
}
else if (strcmp(defel->defname, "quote") == 0)
{
***************
*** 793,799 **** DoCopy(const CopyStmt *stmt, const char *queryString)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->quote = strVal(defel->arg);
}
else if (strcmp(defel->defname, "escape") == 0)
{
--- 794,800 ----
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->quote = defGetString(defel);
}
else if (strcmp(defel->defname, "escape") == 0)
{
***************
*** 801,818 **** DoCopy(const CopyStmt *stmt, const char *queryString)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->escape = strVal(defel->arg);
}
else if (strcmp(defel->defname, "force_quote") == 0)
{
if (force_quote || force_quote_all)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
if (defel->arg && IsA(defel->arg, A_Star))
force_quote_all = true;
! else
force_quote = (List *) defel->arg;
}
else if (strcmp(defel->defname, "force_notnull") == 0)
{
--- 802,837 ----
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! cstate->escape = defGetString(defel);
}
else if (strcmp(defel->defname, "force_quote") == 0)
{
+
if (force_quote || force_quote_all)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
if (defel->arg && IsA(defel->arg, A_Star))
force_quote_all = true;
! 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)
{
***************
*** 820,830 **** DoCopy(const CopyStmt *stmt, const char *queryString)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! force_notnull = (List *) defel->arg;
}
else
! elog(ERROR, "option \"%s\" not recognized",
! defel->defname);
}
/* Check for incompatible options */
--- 839,857 ----
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or redundant options")));
! 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
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("option \"%s\" not recognized",
! defel->defname)));
}
/* Check for incompatible options */
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 373,378 **** static TypeName *TableFuncTypeName(List *columns);
--- 373,382 ----
%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,1947 **** ClosePortalStmt:
/*****************************************************************************
*
* QUERY :
! * COPY relname ['(' columnList ')'] FROM/TO file [WITH options]
! *
! * BINARY, OIDS, and DELIMITERS kept in old locations
! * for backward compatibility. 2002-06-18
*
* COPY ( SELECT ... ) TO file [WITH options]
! * This form doesn't have the backwards-compatible option
! * syntax.
*
*****************************************************************************/
--- 1938,1956 ----
/*****************************************************************************
*
* QUERY :
! * 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]
! *
! * 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,2006 **** copy_file_name:
--- 2010,2016 ----
copy_opt_list:
copy_opt_list copy_opt_item { $$ = lappend($1, $2); }
+ | '(' copy_generic_option_list ')' { $$ = $2 ; }
| /* EMPTY */ { $$ = NIL; }
;
***************
*** 2084,2089 **** opt_using:
--- 2094,2145 ----
| /*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; }
+ | '(' ')' { $$ = 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; }
+ ;
+
/*****************************************************************************
*
*** a/src/test/regress/expected/aggregates.out
--- b/src/test/regress/expected/aggregates.out
***************
*** 326,332 **** FROM bitwise_test;
|
(1 row)
! COPY bitwise_test FROM STDIN NULL 'null';
SELECT
BIT_AND(i2) AS "1",
BIT_AND(i4) AS "1",
--- 326,332 ----
|
(1 row)
! COPY bitwise_test FROM STDIN (NULL 'null');
SELECT
BIT_AND(i2) AS "1",
BIT_AND(i4) AS "1",
***************
*** 401,407 **** FROM bool_test;
|
(1 row)
! COPY bool_test FROM STDIN NULL 'null';
SELECT
BOOL_AND(b1) AS "f",
BOOL_AND(b2) AS "t",
--- 401,407 ----
|
(1 row)
! COPY bool_test FROM STDIN (NULL 'null');
SELECT
BOOL_AND(b1) AS "f",
BOOL_AND(b2) AS "t",
*** a/src/test/regress/expected/copy2.out
--- b/src/test/regress/expected/copy2.out
***************
*** 47,55 **** COPY x from stdin;
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';
-- check results of copy in
SELECT * FROM x;
a | b | c | d | e
--- 47,55 ----
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 (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,97 **** CREATE TABLE no_oids (
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;
ERROR: table "no_oids" does not have OIDs
! COPY no_oids TO stdout WITH OIDS;
ERROR: table "no_oids" does not have OIDs
-- check copy out
COPY x TO stdout;
--- 89,97 ----
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 (OIDS);
ERROR: table "no_oids" does not have OIDs
! COPY no_oids TO stdout (OIDS);
ERROR: table "no_oids" does not have OIDs
-- check copy out
COPY x TO stdout;
***************
*** 146,152 **** stuff after trigger fired
stuff after trigger fired
stuff after trigger fired
stuff after trigger fired
! COPY x (b, e) TO stdout WITH NULL 'I''m null';
I'm null before trigger fired
21 before trigger fired
22 before trigger fired
--- 146,152 ----
stuff after trigger fired
stuff after trigger fired
stuff after trigger fired
! COPY x (b, e) TO stdout (NULL 'I''m null');
I'm null before trigger fired
21 before trigger fired
22 before trigger fired
***************
*** 197,207 **** COPY y TO stdout WITH 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;
-- test end of copy marker
CREATE TEMP TABLE testeoc (a text);
! COPY testeoc FROM stdin CSV;
! COPY testeoc TO stdout CSV;
a\.
\.b
c\.d
--- 197,207 ----
"",
--test that we read consecutive LFs properly
CREATE TEMP TABLE testnl (a int, b text, c int);
! 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);
a\.
\.b
c\.d
*** a/src/test/regress/expected/copyselect.out
--- b/src/test/regress/expected/copyselect.out
***************
*** 93,99 **** v_e
--
-- Test headers, CSV and quotes
--
! copy (select t from test1 where id = 1) to stdout csv header force quote t;
t
"a"
--
--- 93,99 ----
--
-- Test headers, CSV and quotes
--
! copy (select t from test1 where id = 1) to stdout (csv, header, force_quote (t));
t
"a"
--
*** a/src/test/regress/sql/aggregates.sql
--- b/src/test/regress/sql/aggregates.sql
***************
*** 104,110 **** SELECT
BIT_OR(i4) AS "?"
FROM bitwise_test;
! 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
--- 104,110 ----
BIT_OR(i4) AS "?"
FROM bitwise_test;
! 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,177 **** SELECT
BOOL_OR(b3) AS "n"
FROM bool_test;
! COPY bool_test FROM STDIN NULL 'null';
TRUE null FALSE null
FALSE TRUE null null
null TRUE FALSE null
--- 171,177 ----
BOOL_OR(b3) AS "n"
FROM bool_test;
! COPY bool_test FROM STDIN (NULL 'null');
TRUE null FALSE null
FALSE TRUE null null
null TRUE FALSE null
*** a/src/test/regress/sql/copy2.sql
--- b/src/test/regress/sql/copy2.sql
***************
*** 73,89 **** COPY x from stdin;
\.
-- various COPY options: delimiters, oids, NULL string
! COPY x (b, c, d, e) from stdin with 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 '';
3000;;c;;
\.
! COPY x from stdin WITH DELIMITER AS ':' NULL AS E'\\X';
4000:\X:C:\X:\X
4001:1:empty::
4002:2:null:\X:\X
--- 73,89 ----
\.
-- various COPY options: delimiters, oids, NULL string
! 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 (DELIMITER ';', NULL '');
3000;;c;;
\.
! COPY x from stdin (DELIMITER ':', NULL E'\\X');
4000:\X:C:\X:\X
4001:1:empty::
4002:2:null:\X:\X
***************
*** 108,120 **** 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 TO stdout WITH 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';
CREATE TEMP TABLE y (
col1 text,
--- 108,120 ----
INSERT INTO no_oids (a, b) VALUES (20, 30);
-- should fail
! 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 (NULL 'I''m null');
CREATE TEMP TABLE y (
col1 text,
***************
*** 134,140 **** COPY y TO stdout WITH CSV FORCE QUOTE *;
CREATE TEMP TABLE testnl (a int, b text, c int);
! COPY testnl FROM stdin CSV;
1,"a field with two LFs
inside",2
--- 134,140 ----
CREATE TEMP TABLE testnl (a int, b text, c int);
! COPY testnl FROM stdin (CSV);
1,"a field with two LFs
inside",2
***************
*** 143,156 **** inside",2
-- test end of copy marker
CREATE TEMP TABLE testeoc (a text);
! COPY testeoc FROM stdin CSV;
a\.
\.b
c\.d
"\."
\.
! COPY testeoc TO stdout CSV;
DROP TABLE x, y;
DROP FUNCTION fn_x_before();
--- 143,156 ----
-- test end of copy marker
CREATE TEMP TABLE testeoc (a text);
! COPY testeoc FROM stdin (CSV);
a\.
\.b
c\.d
"\."
\.
! COPY testeoc TO stdout (CSV);
DROP TABLE x, y;
DROP FUNCTION fn_x_before();
*** a/src/test/regress/sql/copyselect.sql
--- b/src/test/regress/sql/copyselect.sql
***************
*** 61,67 **** copy (select * from (select t from test1 where id = 1 UNION select * from v_test
--
-- Test headers, CSV and quotes
--
! copy (select t from test1 where id = 1) to stdout csv header force quote t;
--
-- Test psql builtins, plain table
--
--- 61,67 ----
--
-- Test headers, CSV and quotes
--
! copy (select t from test1 where id = 1) to stdout (csv, header, force_quote (t));
--
-- Test psql builtins, plain table
--
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
