Thread: Add switches for DELIMITER and NULL in pg_dump COPY
Folks, >From the earlier discussion, it appears that there is a variety of opinions on what the COPY delimiter should be in pg_dump. This patch allows people to set it and the NULL string. Thanks to Gavin Sherry for help with the pointers :) I didn't patch pg_dumpall, but it would be trivial if there's a use case. Cheers, D -- David Fetter david@fetter.org http://fetter.org/ phone: +1 415 235 3778 Remember to vote!
Attachment
David Fetter <david@fetter.org> writes:
> From the earlier discussion, it appears that there is a variety of
> opinions on what the COPY delimiter should be in pg_dump. This patch
> allows people to set it and the NULL string.
Did anyone provide a convincing use case for this? It's of zero value
from the perspective of pg_dump itself; the only possible argument is
that it makes it easier for program-foo to parse the output of pg_dump.
But I don't see any programs around to parse arbitrary SQL scripts,
especially not the pretty-PG-specific scripts that pg_dump emits.
I think it much more likely that people needing this sort of thing would
be using something like "psql -c 'copy foo to stdout'", so as to get the
data without any added overhead.
So this seems like mere creeping featurism to me. pg_dump has too many
switches already.
regards, tom lane
On Wed, 2006-03-08 at 07:47 -0800, David Fetter wrote:
> From the earlier discussion, it appears that there is a variety of
> opinions on what the COPY delimiter should be in pg_dump. This patch
> allows people to set it and the NULL string.
I'm still not convinced there is a reasonable use-case for this feature.
I can't recall: did the previous discussion conclude that we actually
want this functionality?
> *** src/bin/pg_dump/pg_dump.c 5 Mar 2006 15:58:50 -0000 1.433
> --- src/bin/pg_dump/pg_dump.c 6 Mar 2006 07:32:12 -0000
> ***************
> *** 114,119 ****
> --- 114,125 ----
> /* flag to turn on/off dollar quoting */
> static int disable_dollar_quoting = 0;
>
> + /* Things used when caller invokes COPY options. */
> + #define ARG_COPY_DELIMITER 2
> + #define ARG_COPY_NULL 3
> + char *copy_delimiter = "\t";
> + char *copy_null;
> +
The variables should be declared static.
> static void help(const char *progname);
> static NamespaceInfo *findNamespace(Oid nsoid, Oid objoid);
> ***************
> *** 181,186 ****
> --- 187,193 ----
> ExecStatusType expected);
>
>
> +
> int
> main(int argc, char **argv)
> {
> ***************
> *** 211,217 ****
> char *outputSuperuser = NULL;
>
> RestoreOptions *ropt;
> !
> static struct option long_options[] = {
> {"data-only", no_argument, NULL, 'a'},
> {"blobs", no_argument, NULL, 'b'},
> --- 218,224 ----
> char *outputSuperuser = NULL;
>
> RestoreOptions *ropt;
> !
> static struct option long_options[] = {
> {"data-only", no_argument, NULL, 'a'},
> {"blobs", no_argument, NULL, 'b'},
Please review patches and eliminate content-free hunks like these before
submitting.
> ***************
> *** 427,432 ****
> --- 464,479 ----
> }
> }
>
> + if (copy_null == NULL)
> + copy_null = malloc(3);
> + strcpy(copy_null, "\\N");
You're missing some braces.
> + if (strstr(copy_null, copy_delimiter))
> + {
> + fprintf(stderr, _("In %s, the NULL AS string cannot
> contain the COPY delimiter.\n"), progname);
> + exit(1);
> + }
I'm not sure as to whether you should be using write_msg() or fprintf()
here, but we should probably pick one and be consistent. Also ISTM we
should be to refactor the code to use exit_nicely() anyway, provided
that g_conn is initialized to NULL before we have connected to the DB.
> ***************
> *** 702,707 ****
> --- 749,756 ----
> " use SESSION
> AUTHORIZATION commands instead of\n"
> " OWNER TO commands
> \n"));
>
> + printf(_(" --copy-delimiter string to use as column
> DELIMITER in COPY statements\n"));
Capitalizing "DELIMITER" here is not good style, IMHO: it is just a
normal word.
> *** 844,849 ****
> --- 893,904 ----
> int ret;
> char *copybuf;
> const char *column_list;
> + char *local_copy_delimiter;
> + char *local_copy_null;
> + local_copy_delimiter = malloc(2*strlen(copy_delimiter)+1);
> + PQescapeString (local_copy_delimiter, copy_delimiter,
> 2*strlen(copy_delimiter)+1);
> + local_copy_null = malloc(2*strlen(copy_null)+1);
> + PQescapeString (local_copy_null, copy_null,
> 2*strlen(copy_null)+1);
Spacing: spaces around operands to mathematical operators, no spaces
before the parameter list to a function call.
You should also fix this compiler warning:
[...]/pg_dump.c:440: warning: format '%d' expects type 'int', but
argument 4 has type 'size_t'
-Neil
On Wed, Mar 08, 2006 at 11:03:00AM -0500, Tom Lane wrote: > David Fetter <david@fetter.org> writes: > > From the earlier discussion, it appears that there is a variety of > > opinions on what the COPY delimiter should be in pg_dump. This > > patch allows people to set it and the NULL string. > > Did anyone provide a convincing use case for this? I've had one so far, and it was enough to cause me to make a special patched version of pg_dump. To get some idea of how drastic that was, consider that I think it's generally bad practice to compile from source because it can take you too far off the "generally supported software" map. The case I had was making a database with a schema and initial data whose dump output gets checked into a source code management system. Those initial data sets, which can change--for example when the corresponding ISO codes do--may be in many different tables, so the easiest way to do this is to make the dump file as easy as possible to edit. > It's of zero value from the perspective of pg_dump itself; the only > possible argument is that it makes it easier for program-foo to > parse the output of pg_dump. But I don't see any programs around to > parse arbitrary SQL scripts, especially not the pretty-PG-specific > scripts that pg_dump emits. It's less about program-foo parsing than about multi-table data management, as above. However, I'm sure that there are people who will find other uses for it. > I think it much more likely that people needing this sort of thing would > be using something like "psql -c 'copy foo to stdout'", so as to get the > data without any added overhead. The one-table-at-a-time approach is quite error-prone for large numbers of tables and/or large data sets. > So this seems like mere creeping featurism to me. pg_dump has too > many switches already. I've been careful to see to it that only people who use the switches are affected by it. I am also volunteering to do ongoing maintenance of this feature. :) Cheers, D -- David Fetter david@fetter.org http://fetter.org/ phone: +1 415 235 3778 Remember to vote!
On Wed, Mar 08, 2006 at 11:10:04AM -0500, Neil Conway wrote: > On Wed, 2006-03-08 at 07:47 -0800, David Fetter wrote: > > From the earlier discussion, it appears that there is a variety of > > opinions on what the COPY delimiter should be in pg_dump. This patch > > allows people to set it and the NULL string. > > I'm still not convinced there is a reasonable use-case for this feature. > I can't recall: did the previous discussion conclude that we actually > want this functionality? The previous discussion showed that there is a wide diversity of opinions on what The Right Delimiter and The Right NULL String(TM) are. Thanks for the tips. :) I'll make a revised patch this evening, time permitting. Cheers, D -- David Fetter david@fetter.org http://fetter.org/ phone: +1 415 235 3778 Remember to vote!
David Fetter <david@fetter.org> writes:
> On Wed, Mar 08, 2006 at 11:03:00AM -0500, Tom Lane wrote:
>> Did anyone provide a convincing use case for this?
> I've had one so far, and it was enough to cause me to make a special
> patched version of pg_dump. To get some idea of how drastic that was,
> consider that I think it's generally bad practice to compile from
> source because it can take you too far off the "generally supported
> software" map. The case I had was making a database with a schema and
> initial data whose dump output gets checked into a source code
> management system.
So? Don't tell me your SCMS can't handle tabs.
regards, tom lane
On Wed, Mar 08, 2006 at 11:26:00AM -0500, Tom Lane wrote: > David Fetter <david@fetter.org> writes: > > On Wed, Mar 08, 2006 at 11:03:00AM -0500, Tom Lane wrote: > >> Did anyone provide a convincing use case for this? > > > I've had one so far, and it was enough to cause me to make a > > special patched version of pg_dump. To get some idea of how > > drastic that was, consider that I think it's generally bad > > practice to compile from source because it can take you too far > > off the "generally supported software" map. The case I had was > > making a database with a schema and initial data whose dump output > > gets checked into a source code management system. > > So? Don't tell me your SCMS can't handle tabs. Not everybody's editor/mailer/whatever does this right, and it makes things fragile. Another way to do this is to change the delimter to a printable character like '|', but that raises hackles, too. Cheers, D -- David Fetter david@fetter.org http://fetter.org/ phone: +1 415 235 3778 Remember to vote!
David Fetter <david@fetter.org> writes: > Not everybody's editor/mailer/whatever does this right, and it makes > things fragile. Another way to do this is to change the delimter to a > printable character like '|', but that raises hackles, too. Frankly if you're passing you data through an editor/mailer/whatever you don't trust then your setup is already fragile. At least if you're using tabs then you find out about these problems. Tiptoeing around the untrustworthy process just means that it'll fail randomly (and unpredictably) when other characters appear in the data that the software doesn't handle. There are certainly cases where you'll need to do this to interface with other (amateurish) software. But pg_dump isn't for that at all. Even COPY isn't a general purpose data formatter. To interface with other software not using a standard format you're going to have to pass the data through Perl or something like that anyways. -- greg
On Wed, 2006-03-08 at 08:20 -0800, David Fetter wrote: > The previous discussion showed that there is a wide diversity of > opinions on what The Right Delimiter and The Right NULL String(TM) > are. Barring a more convincing justification for why we need this feature, I'm inclined to side with Tom: pg_dump has enough obscure options as it is, and I can't imagine very many people needing this functionality. -Neil
On Wed, Mar 08, 2006 at 04:57:52PM -0500, Neil Conway wrote: > On Wed, 2006-03-08 at 08:20 -0800, David Fetter wrote: > > The previous discussion showed that there is a wide diversity of > > opinions on what The Right Delimiter and The Right NULL String(TM) > > are. > > Barring a more convincing justification for why we need this feature, > I'm inclined to side with Tom: pg_dump has enough obscure options as it > is, and I can't imagine very many people needing this functionality. Given all the different requests that come in for pg_dump and copy, maybe it makes sense for Someone Who Cares to start a pgFoundry project (or maybe extend the import/export project that's already there). -- Jim C. Nasby, Sr. Engineering Consultant jnasby@pervasive.com Pervasive Software http://pervasive.com work: 512-231-6117 vcard: http://jim.nasby.net/pervasive.vcf cell: 512-569-9461