One less footgun: deprecating pg_dump -d - Mailing list pgsql-hackers

Attached is a patch to fix the annoying footgun that is pg_dump -d. Myself and
many others I know have all at one time or another done this:

psql -h localhost -U greg -d postgres

pg_dump -h localhost -U greg -d postgres > dumpfile

The latter command silently succeeds, but only through the combination of -d
being an option that takes no arguments, and the fact that 'postgres' is read by
pg_dump as a separate argument indicating which database to use. Thus, your dump
file now has INSERTs when you wanted to use COPY (as you want your database
restore to take 20 minutes, not three hours).

I thought about changing -d to actually indicate the database, as in psql, but
that brings about another problem: the command above will still silently work,
but produce a dump without INSERTs. While this is good for people who meant to
leave the -d out, it's not good for people (and scripts) that DID want the -d to
work as documented. Thus, changing it will silently break those scripts (until
they try to load the schema into a non-PG database...).

The solution I came up with is to use a new letter, -I, and to deprecate -d by
having it throw an exception when used. The choice of -I seems appropriate as a
shortcut for --inserts, and (as far as I can tell) does not conflict with any
other programs (e.g. psql). Doing so will require people to rewrite any scripts
that are using -d instead of --inserts, but it seems a small price to eliminate
this nasty footgun. As a bonus, their scripts will be easier to read, as -d was
confusing at best, and hardly mnemonic.

--
Greg Sabino Mullane greg@endpoint.com greg@turnstep.com
End Point Corporation
PGP Key: 0x14964AC8
Index: doc/src/sgml/ref/pg_dump.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/pg_dump.sgml,v
retrieving revision 1.112
diff -c -r1.112 pg_dump.sgml
*** doc/src/sgml/ref/pg_dump.sgml    4 Mar 2009 11:57:00 -0000    1.112
--- doc/src/sgml/ref/pg_dump.sgml    9 Mar 2009 15:02:47 -0000
***************
*** 176,182 ****
       </varlistentry>

       <varlistentry>
!       <term><option>-d</option></term>
        <term><option>--inserts</option></term>
        <listitem>
         <para>
--- 176,182 ----
       </varlistentry>

       <varlistentry>
!       <term><option>-I</option></term>
        <term><option>--inserts</option></term>
        <listitem>
         <para>
***************
*** 190,196 ****
          Note that
          the restore might fail altogether if you have rearranged column order.
          The <option>-D</option> option is safe against column order changes,
!         though even slower.
         </para>
        </listitem>
       </varlistentry>
--- 190,197 ----
          Note that
          the restore might fail altogether if you have rearranged column order.
          The <option>-D</option> option is safe against column order changes,
!         though even slower. (The <option>-d</option> has been deprectated, as it
!         was too similar to the same option as used by psql).
         </para>
        </listitem>
       </varlistentry>
Index: doc/src/sgml/ref/pg_dumpall.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/pg_dumpall.sgml,v
retrieving revision 1.78
diff -c -r1.78 pg_dumpall.sgml
*** doc/src/sgml/ref/pg_dumpall.sgml    4 Mar 2009 11:57:00 -0000    1.78
--- doc/src/sgml/ref/pg_dumpall.sgml    9 Mar 2009 15:02:47 -0000
***************
*** 99,105 ****
       </varlistentry>

       <varlistentry>
!       <term><option>-d</option></term>
        <term><option>--inserts</option></term>
        <listitem>
         <para>
--- 99,105 ----
       </varlistentry>

       <varlistentry>
!       <term><option>-I</option></term>
        <term><option>--inserts</option></term>
        <listitem>
         <para>
***************
*** 109,114 ****
--- 109,116 ----
          non-<productname>PostgreSQL</productname> databases.  Note that
          the restore might fail altogether if you have rearranged column order.
          The <option>-D</option> option is safer, though even slower.
+         (The <option>-d</option> has been deprectated, as it was too similar
+         to the same option as used by psql).
         </para>
        </listitem>
       </varlistentry>
Index: src/bin/pg_dump/pg_dump.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_dump.c,v
retrieving revision 1.528
diff -c -r1.528 pg_dump.c
*** src/bin/pg_dump/pg_dump.c    4 Mar 2009 11:57:00 -0000    1.528
--- src/bin/pg_dump/pg_dump.c    9 Mar 2009 15:02:48 -0000
***************
*** 246,256 ****
          {"create", no_argument, NULL, 'C'},
          {"file", required_argument, NULL, 'f'},
          {"format", required_argument, NULL, 'F'},
-         {"inserts", no_argument, NULL, 'd'},
          {"attribute-inserts", no_argument, NULL, 'D'},
          {"column-inserts", no_argument, NULL, 'D'},
          {"host", required_argument, NULL, 'h'},
          {"ignore-version", no_argument, NULL, 'i'},
          {"no-reconnect", no_argument, NULL, 'R'},
          {"oids", no_argument, NULL, 'o'},
          {"no-owner", no_argument, NULL, 'O'},
--- 246,256 ----
          {"create", no_argument, NULL, 'C'},
          {"file", required_argument, NULL, 'f'},
          {"format", required_argument, NULL, 'F'},
          {"attribute-inserts", no_argument, NULL, 'D'},
          {"column-inserts", no_argument, NULL, 'D'},
          {"host", required_argument, NULL, 'h'},
          {"ignore-version", no_argument, NULL, 'i'},
+         {"inserts", no_argument, NULL, 'I'},
          {"no-reconnect", no_argument, NULL, 'R'},
          {"oids", no_argument, NULL, 'o'},
          {"no-owner", no_argument, NULL, 'O'},
***************
*** 316,322 ****
--- 316,326 ----
          }
      }

+ <<<<<<< pg_dump.c
+     while ((c = getopt_long(argc, argv, "abcCdDE:f:F:h:iIn:N:oOp:RsS:t:T:U:vWxX:Z:",
+ =======
      while ((c = getopt_long(argc, argv, "abcCdDE:f:F:h:in:N:oOp:RsS:t:T:U:vwWxX:Z:",
+ >>>>>>> 1.528
                              long_options, &optindex)) != -1)
      {
          switch (c)
***************
*** 337,345 ****
                  outputCreate = 1;
                  break;

!             case 'd':            /* dump data as proper insert strings */
!                 dumpInserts = true;
!                 break;

              case 'D':            /* dump data as proper insert strings with
                                   * attr names */
--- 341,351 ----
                  outputCreate = 1;
                  break;

!             case 'd':            /* deprecated flag: warn the user and exit */
!                 fprintf(stderr,
!                         _("%s: invalid option, -d has been deprecated, use --inserts instead\n"),
!                         progname);
!                 exit(1);

              case 'D':            /* dump data as proper insert strings with
                                   * attr names */
***************
*** 367,372 ****
--- 373,382 ----
                  /* ignored, deprecated option */
                  break;

+             case 'I':            /* dump data as proper insert strings */
+                 dumpInserts = true;
+                 break;
+
              case 'n':            /* include schema(s) */
                  simple_string_list_append(&schema_include_patterns, optarg);
                  include_everything = false;
***************
*** 498,504 ****

      if (dumpInserts == true && oids == true)
      {
!         write_msg(NULL, "options -d/-D/--inserts/--column-inserts and -o/--oids cannot be used together\n");
          write_msg(NULL, "(The INSERT command cannot set OIDs.)\n");
          exit(1);
      }
--- 508,514 ----

      if (dumpInserts == true && oids == true)
      {
!         write_msg(NULL, "options -I/-D/--inserts/--column-inserts and -o/--oids cannot be used together\n");
          write_msg(NULL, "(The INSERT command cannot set OIDs.)\n");
          exit(1);
      }
***************
*** 815,823 ****
      printf(_("  -b, --blobs                 include large objects in dump\n"));
      printf(_("  -c, --clean                 clean (drop) database objects before recreating\n"));
      printf(_("  -C, --create                include commands to create database in dump\n"));
-     printf(_("  -d, --inserts               dump data as INSERT commands, rather than COPY\n"));
      printf(_("  -D, --column-inserts        dump data as INSERT commands with column names\n"));
      printf(_("  -E, --encoding=ENCODING     dump the data in encoding ENCODING\n"));
      printf(_("  -n, --schema=SCHEMA         dump the named schema(s) only\n"));
      printf(_("  -N, --exclude-schema=SCHEMA do NOT dump the named schema(s)\n"));
      printf(_("  -o, --oids                  include OIDs in dump\n"));
--- 825,833 ----
      printf(_("  -b, --blobs                 include large objects in dump\n"));
      printf(_("  -c, --clean                 clean (drop) database objects before recreating\n"));
      printf(_("  -C, --create                include commands to create database in dump\n"));
      printf(_("  -D, --column-inserts        dump data as INSERT commands with column names\n"));
      printf(_("  -E, --encoding=ENCODING     dump the data in encoding ENCODING\n"));
+     printf(_("  -I, --inserts               dump data as INSERT commands, rather than COPY\n"));
      printf(_("  -n, --schema=SCHEMA         dump the named schema(s) only\n"));
      printf(_("  -N, --exclude-schema=SCHEMA do NOT dump the named schema(s)\n"));
      printf(_("  -o, --oids                  include OIDs in dump\n"));
Index: src/bin/pg_dump/pg_dumpall.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_dumpall.c,v
retrieving revision 1.119
diff -c -r1.119 pg_dumpall.c
*** src/bin/pg_dump/pg_dumpall.c    4 Mar 2009 11:57:00 -0000    1.119
--- src/bin/pg_dump/pg_dumpall.c    9 Mar 2009 15:02:48 -0000
***************
*** 97,103 ****
          {"binary-upgrade", no_argument, &binary_upgrade, 1},    /* not documented */
          {"data-only", no_argument, NULL, 'a'},
          {"clean", no_argument, NULL, 'c'},
!         {"inserts", no_argument, NULL, 'd'},
          {"attribute-inserts", no_argument, NULL, 'D'},
          {"column-inserts", no_argument, NULL, 'D'},
          {"file", required_argument, NULL, 'f'},
--- 97,103 ----
          {"binary-upgrade", no_argument, &binary_upgrade, 1},    /* not documented */
          {"data-only", no_argument, NULL, 'a'},
          {"clean", no_argument, NULL, 'c'},
!         {"inserts", no_argument, NULL, 'I'},
          {"attribute-inserts", no_argument, NULL, 'D'},
          {"column-inserts", no_argument, NULL, 'D'},
          {"file", required_argument, NULL, 'f'},
***************
*** 178,184 ****
--- 178,188 ----

      pgdumpopts = createPQExpBuffer();

+ <<<<<<< pg_dumpall.c
+     while ((c = getopt_long(argc, argv, "acdDf:gh:iIl:oOp:rsS:tU:vWxX:", long_options, &optindex)) != -1)
+ =======
      while ((c = getopt_long(argc, argv, "acdDf:gh:il:oOp:rsS:tU:vwWxX:", long_options, &optindex)) != -1)
+ >>>>>>> 1.119
      {
          switch (c)
          {
***************
*** 191,198 ****
                  output_clean = true;
                  break;

!             case 'd':
              case 'D':
                  appendPQExpBuffer(pgdumpopts, " -%c", c);
                  break;

--- 195,208 ----
                  output_clean = true;
                  break;

!             case 'd':          /* deprecated flag */
!                 fprintf(stderr,
!                         _("%s: invalid option, -d has been deprecated, use --inserts instead\n"),
!                         progname);
!                 exit(1);
!
              case 'D':
+             case 'I':
                  appendPQExpBuffer(pgdumpopts, " -%c", c);
                  break;

***************
*** 511,519 ****
      printf(_("\nOptions controlling the output content:\n"));
      printf(_("  -a, --data-only             dump only the data, not the schema\n"));
      printf(_("  -c, --clean                 clean (drop) databases before recreating\n"));
-     printf(_("  -d, --inserts               dump data as INSERT commands, rather than COPY\n"));
      printf(_("  -D, --column-inserts        dump data as INSERT commands with column names\n"));
      printf(_("  -g, --globals-only          dump only global objects, no databases\n"));
      printf(_("  -o, --oids                  include OIDs in dump\n"));
      printf(_("  -O, --no-owner              skip restoration of object ownership\n"));
      printf(_("  -r, --roles-only            dump only roles, no databases or tablespaces\n"));
--- 521,529 ----
      printf(_("\nOptions controlling the output content:\n"));
      printf(_("  -a, --data-only             dump only the data, not the schema\n"));
      printf(_("  -c, --clean                 clean (drop) databases before recreating\n"));
      printf(_("  -D, --column-inserts        dump data as INSERT commands with column names\n"));
      printf(_("  -g, --globals-only          dump only global objects, no databases\n"));
+     printf(_("  -I, --inserts               dump data as INSERT, rather than COPY, commands\n"));
      printf(_("  -o, --oids                  include OIDs in dump\n"));
      printf(_("  -O, --no-owner              skip restoration of object ownership\n"));
      printf(_("  -r, --roles-only            dump only roles, no databases or tablespaces\n"));