Add switches for DELIMITER and NULL in pg_dump COPY

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