Thread: Add switches for DELIMITER and NULL in pg_dump COPY

Add switches for DELIMITER and NULL in pg_dump COPY

From
David Fetter
Date:
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

Re: [HACKERS] Add switches for DELIMITER and NULL in pg_dump COPY

From
Tom Lane
Date:
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

Re: Add switches for DELIMITER and NULL in pg_dump COPY

From
Neil Conway
Date:
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



Re: [HACKERS] Add switches for DELIMITER and NULL in pg_dump COPY

From
David Fetter
Date:
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!

Re: Add switches for DELIMITER and NULL in pg_dump COPY

From
David Fetter
Date:
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!

Re: [HACKERS] Add switches for DELIMITER and NULL in pg_dump COPY

From
Tom Lane
Date:
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

Re: [HACKERS] Add switches for DELIMITER and NULL in pg_dump COPY

From
David Fetter
Date:
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!

Re: [HACKERS] Add switches for DELIMITER and NULL in pg_dump COPY

From
Greg Stark
Date:
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

Re: Add switches for DELIMITER and NULL in pg_dump COPY

From
Neil Conway
Date:
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



Re: [HACKERS] Add switches for DELIMITER and NULL in pg_dump COPY

From
"Jim C. Nasby"
Date:
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