Re: [HACKERS] \copy (query) delimiter syntax error
I think I'll go with Tom's Plan B for HEAD, but not do anything more for 8.2 than has already been done. cheers andrew Bruce Momjian wrote: Did we come to a conclusion on this? --- Andrew Dunstan wrote: Andrew Dunstan wrote: Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: The consequence will be, though, that psql will accept a syntax for \copy (query) ... that the corresponding backend command would reject were we not transforming it. That strikes me as potentially confusing. Perhaps. What about plan B: remove the legacy syntax support in \copy? IIRC it has not been documented since 7.2, so maybe we can finally throw it overboard. Thoughts? I like it for 8.3 - but maybe my present patch would be better for 8.2, as it involves less behaviour change. While we decide this issue, which can be worked around in any case, I am going to commit the part of the patch that nobody has objected to (and which will fix Michael's original complaint), on HEAD and 8.2 stable, so we can get some testing going. cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] \copy (query) delimiter syntax error
Andrew Dunstan wrote: Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: The consequence will be, though, that psql will accept a syntax for \copy (query) ... that the corresponding backend command would reject were we not transforming it. That strikes me as potentially confusing. Perhaps. What about plan B: remove the legacy syntax support in \copy? IIRC it has not been documented since 7.2, so maybe we can finally throw it overboard. Thoughts? I like it for 8.3 - but maybe my present patch would be better for 8.2, as it involves less behaviour change. While we decide this issue, which can be worked around in any case, I am going to commit the part of the patch that nobody has objected to (and which will fix Michael's original complaint), on HEAD and 8.2 stable, so we can get some testing going. cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] \copy (query) delimiter syntax error
Andrew Dunstan [EMAIL PROTECTED] writes: oh, good point. OK, I have cut this quick patch that will continue to accept the legacy syntax in psql in non-inline-query cases, but will make psql unreservedly emit new style syntax for COPY to the backend. Does that seem reasonable, or is it too much of a change for the stable branch? We've never promised that psql's backslash commands would work at all with older backends, so I think removing compatibility with pre-7.3 backends at this point isn't a problem. OTOH, going out of your way to remove compatibility seems a bit pointless, so I'd vote against the have_query parts of this patch. Just change the output format. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] \copy (query) delimiter syntax error
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: oh, good point. OK, I have cut this quick patch that will continue to accept the legacy syntax in psql in non-inline-query cases, but will make psql unreservedly emit new style syntax for COPY to the backend. Does that seem reasonable, or is it too much of a change for the stable branch? We've never promised that psql's backslash commands would work at all with older backends, so I think removing compatibility with pre-7.3 backends at this point isn't a problem. OTOH, going out of your way to remove compatibility seems a bit pointless, so I'd vote against the have_query parts of this patch. Just change the output format. The consequence will be, though, that psql will accept a syntax for \copy (query) ... that the corresponding backend command would reject were we not transforming it. That strikes me as potentially confusing. I don't have very strong feelings either way - anybody else have an opinion? If not, I'll go with Tom's suggestion. cheers andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] \copy (query) delimiter syntax error
Andrew Dunstan [EMAIL PROTECTED] writes: The consequence will be, though, that psql will accept a syntax for \copy (query) ... that the corresponding backend command would reject were we not transforming it. That strikes me as potentially confusing. Perhaps. What about plan B: remove the legacy syntax support in \copy? IIRC it has not been documented since 7.2, so maybe we can finally throw it overboard. Thoughts? regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] \copy (query) delimiter syntax error
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: The consequence will be, though, that psql will accept a syntax for \copy (query) ... that the corresponding backend command would reject were we not transforming it. That strikes me as potentially confusing. Perhaps. What about plan B: remove the legacy syntax support in \copy? IIRC it has not been documented since 7.2, so maybe we can finally throw it overboard. Thoughts? I like it for 8.3 - but maybe my present patch would be better for 8.2, as it involves less behaviour change. cheers andrew ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
[HACKERS] \copy (query) delimiter syntax error
psql's \copy (query) with a delimiter yields a syntax error: test= \copy foo to foo.txt delimiter '|' (works) test= \copy (select * from foo) to foo.txt (works) test= \copy (select * from foo) to foo.txt delimiter '|' ERROR: syntax error at or near USING LINE 1: COPY ( select * from foo ) TO STDOUT USING DELIMITERS '|' The problem is that \copy sends USING DELIMITERS for backward compatibility (comment on line 502 of src/bin/psql/copy.c) but that COPY (query) doesn't support USING DELIMITERS: CopyStmt: COPY opt_binary qualified_name opt_column_list opt_oids copy_from copy_file_name copy_delimiter opt_with copy_opt_list ... | COPY select_with_parens TO copy_file_name opt_with copy_opt_list copy_delimiter: /* USING DELIMITERS kept for backward compatibility. 2002-06-15 */ opt_using DELIMITERS Sconst What should be fixed -- COPY or \copy? Does psql's \copy still need backward compatibility to unsupported pre-7.3? -- Michael Fuhr ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] \copy (query) delimiter syntax error
Michael Fuhr wrote: psql's \copy (query) with a delimiter yields a syntax error: test= \copy foo to foo.txt delimiter '|' (works) test= \copy (select * from foo) to foo.txt (works) test= \copy (select * from foo) to foo.txt delimiter '|' ERROR: syntax error at or near USING LINE 1: COPY ( select * from foo ) TO STDOUT USING DELIMITERS '|' The problem is that \copy sends USING DELIMITERS for backward compatibility (comment on line 502 of src/bin/psql/copy.c) but that COPY (query) doesn't support USING DELIMITERS: CopyStmt: COPY opt_binary qualified_name opt_column_list opt_oids copy_from copy_file_name copy_delimiter opt_with copy_opt_list ... | COPY select_with_parens TO copy_file_name opt_with copy_opt_list copy_delimiter: /* USING DELIMITERS kept for backward compatibility. 2002-06-15 */ opt_using DELIMITERS Sconst What should be fixed -- COPY or \copy? Does psql's \copy still need backward compatibility to unsupported pre-7.3? I'd say fix psql. Not sure how far back we should backpatch it. It's interesting that this has been there since 8.0 and is only now discovered. cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] \copy (query) delimiter syntax error
On Sat, Feb 03, 2007 at 10:52:29AM -0600, Andrew Dunstan wrote: I'd say fix psql. Not sure how far back we should backpatch it. It's interesting that this has been there since 8.0 and is only now discovered. The problem is new in 8.2 because COPY (query) doesn't support USING DELIMITERS. COPY tablename does, so it has worked all along. -- Michael Fuhr ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] \copy (query) delimiter syntax error
Michael Fuhr wrote:
On Sat, Feb 03, 2007 at 10:52:29AM -0600, Andrew Dunstan wrote:
I'd say fix psql. Not sure how far back we should backpatch it. It's
interesting that this has been there since 8.0 and is only now discovered.
The problem is new in 8.2 because COPY (query) doesn't support USING
DELIMITERS. COPY tablename does, so it has worked all along.
oh, good point. OK, I have cut this quick patch that will continue to
accept the legacy syntax in psql in non-inline-query cases, but will
make psql unreservedly emit new style syntax for COPY to the backend.
Does that seem reasonable, or is it too much of a change for the stable
branch?
cheers
andrew
Index: src/bin/psql/copy.c
===
RCS file: /cvsroot/pgsql/src/bin/psql/copy.c,v
retrieving revision 1.72
diff -c -r1.72 copy.c
*** src/bin/psql/copy.c 5 Jan 2007 22:19:49 - 1.72
--- src/bin/psql/copy.c 3 Feb 2007 19:37:34 -
***
*** 118,123
--- 118,124
char *token;
const char *whitespace = \t\n\r;
char nonstd_backslash = standard_strings() ? 0 : '\\';
+ boolhave_query = false;
if (args)
line = pg_strdup(args);
***
*** 163,168
--- 164,170
xstrcat(result-table, );
xstrcat(result-table, token);
}
+ have_query = true;
}
token = strtokx(NULL, whitespace, .,(), \,
***
*** 268,291
0, false, false, pset.encoding);
/*
! * Allows old COPY syntax for backward compatibility 2002-06-19
*/
! if (token pg_strcasecmp(token, using) == 0)
! {
! token = strtokx(NULL, whitespace, NULL, NULL,
! 0, false, false, pset.encoding);
! if (!(token pg_strcasecmp(token, delimiters) == 0))
! goto error;
! }
! if (token pg_strcasecmp(token, delimiters) == 0)
{
! token = strtokx(NULL, whitespace, NULL, ',
! nonstd_backslash, true, false, pset.encoding);
! if (!token)
! goto error;
! result-delim = pg_strdup(token);
! token = strtokx(NULL, whitespace, NULL, NULL,
! 0, false, false, pset.encoding);
}
if (token)
--- 270,297
0, false, false, pset.encoding);
/*
! * Allows old COPY syntax for backward compatibility.
! * Skip if we have an inline query instead of a table name.
*/
! if (! have_query)
{
! if (token pg_strcasecmp(token, using) == 0)
! {
! token = strtokx(NULL, whitespace, NULL, NULL,
! 0, false, false, pset.encoding);
! if (!(token pg_strcasecmp(token, delimiters) == 0))
! goto error;
! }
! if (token pg_strcasecmp(token, delimiters) == 0)
! {
! token = strtokx(NULL, whitespace, NULL, ',
! nonstd_backslash, true, false, pset.encoding);
! if (!token)
! goto error;
! result-delim = pg_strdup(token);
! token = strtokx(NULL, whitespace, NULL, NULL,
! 0, false, false, pset.encoding);
! }
}
if (token)
***
*** 480,511
printfPQExpBuffer(query, COPY );
- /* Uses old COPY syntax for backward compatibility 2002-06-19 */
- if (options-binary)
- appendPQExpBuffer(query, BINARY );
-
appendPQExpBuffer(query, %s , options-table);
if (options-column_list)
appendPQExpBuffer(query, %s , options-column_list);
- /* Uses old COPY syntax for backward compatibility 2002-06-19 */
- if (options-oids)
- appendPQExpBuffer(query, WITH OIDS );
-
if (options-from)
appendPQExpBuffer(query, FROM STDIN);
else
appendPQExpBuffer(query, TO STDOUT);
! /* Uses old COPY syntax for backward compatibility 2002-06-19 */
if (options-delim)
! emit_copy_option(query, USING DELIMITERS , options-delim);
- /* There is no backward-compatible CSV syntax */
if (options-null)
! emit_copy_option(query, WITH NULL AS , options-null);
if (options-csv_mode)
appendPQExpBuffer(query, CSV);
--- 486,513
printfPQExpBuffer(query, COPY );
appendPQExpBuffer(query, %s , options-table);
if (options-column_list)
appendPQExpBuffer(query, %s , options-column_list);
if (options-from)
appendPQExpBuffer(query, FROM STDIN);
else
appendPQExpBuffer(query, TO STDOUT);
! if (options-binary)
! appendPQExpBuffer(query, BINARY );
!
! if (options-oids)
! appendPQExpBuffer(query, OIDS );
!
if (options-delim)
! emit_copy_option(query, DELIMITER , options-delim);
if (options-null)
! emit_copy_option(query, NULL AS , options-null);
if (options-csv_mode)
appendPQExpBuffer(query, CSV);
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
