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
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 -- Bruce Momjian <[EMAIL PROTECTED]> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
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
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
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: 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: > 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
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
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: > 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