Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Teach pg_dump to quote reloption values safely.
authorTom Lane <tgl@sss.pgh.pa.us>
Sun, 3 Jan 2016 00:04:45 +0000 (19:04 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Sun, 3 Jan 2016 00:04:45 +0000 (19:04 -0500)
Commit c7e27becd2e6eb93 fixed this on the backend side, but we neglected
the fact that several code paths in pg_dump were printing reloptions
values that had not gotten massaged by ruleutils.  Apply essentially the
same quoting logic in those places, too.

src/bin/pg_dump/pg_dump.c
src/bin/pg_dump/pg_dump.h

index 02d1221b455144b62a0a921d43ecb5bcf45ec029..57a3eb450d3229b57df4f3ed072fc053f1200000 100644 (file)
@@ -245,6 +245,9 @@ static void binary_upgrade_extension_member(PQExpBuffer upgrade_buffer,
                                const char *objlabel);
 static const char *getAttrName(int attrnum, TableInfo *tblInfo);
 static const char *fmtCopyColumnList(const TableInfo *ti);
+static bool nonemptyReloptions(const char *reloptions);
+static void fmtReloptionsArray(Archive *fout, PQExpBuffer buffer,
+                  const char *reloptions, const char *prefix);
 static void do_sql_command(PGconn *conn, const char *query);
 static void check_sql_result(PGresult *res, PGconn *conn, const char *query,
                 ExecStatusType expected);
@@ -3956,8 +3959,8 @@ getTables(int *numTables)
                          "d.refobjid AS owning_tab, "
                          "d.refobjsubid AS owning_col, "
                          "(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
-                       "array_to_string(c.reloptions, ', ') AS reloptions, "
-                         "array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
+                         "c.reloptions AS reloptions, "
+                         "tc.reloptions AS toast_reloptions "
                          "FROM pg_class c "
                          "LEFT JOIN pg_depend d ON "
                          "(c.relkind = '%c' AND "
@@ -3992,8 +3995,8 @@ getTables(int *numTables)
                          "d.refobjid AS owning_tab, "
                          "d.refobjsubid AS owning_col, "
                          "(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
-                       "array_to_string(c.reloptions, ', ') AS reloptions, "
-                         "array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
+                         "c.reloptions AS reloptions, "
+                         "tc.reloptions AS toast_reloptions "
                          "FROM pg_class c "
                          "LEFT JOIN pg_depend d ON "
                          "(c.relkind = '%c' AND "
@@ -4027,8 +4030,8 @@ getTables(int *numTables)
                          "d.refobjid AS owning_tab, "
                          "d.refobjsubid AS owning_col, "
                          "(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
-                       "array_to_string(c.reloptions, ', ') AS reloptions, "
-                         "array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
+                         "c.reloptions AS reloptions, "
+                         "tc.reloptions AS toast_reloptions "
                          "FROM pg_class c "
                          "LEFT JOIN pg_depend d ON "
                          "(c.relkind = '%c' AND "
@@ -4062,7 +4065,7 @@ getTables(int *numTables)
                          "d.refobjid AS owning_tab, "
                          "d.refobjsubid AS owning_col, "
                          "(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
-                       "array_to_string(c.reloptions, ', ') AS reloptions, "
+                         "c.reloptions AS reloptions, "
                          "NULL AS toast_reloptions "
                          "FROM pg_class c "
                          "LEFT JOIN pg_depend d ON "
@@ -4497,7 +4500,7 @@ getIndexes(TableInfo tblinfo[], int numTables)
                i_conoid,
                i_condef,
                i_tablespace,
-               i_options;
+               i_indreloptions;
    int         ntups;
 
    for (i = 0; i < numTables; i++)
@@ -4545,7 +4548,7 @@ getIndexes(TableInfo tblinfo[], int numTables)
                              "c.oid AS conoid, "
                  "pg_catalog.pg_get_constraintdef(c.oid, false) AS condef, "
                              "(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, "
-                           "array_to_string(t.reloptions, ', ') AS options "
+                             "t.reloptions AS indreloptions "
                              "FROM pg_catalog.pg_index i "
                      "JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
                              "LEFT JOIN pg_catalog.pg_constraint c "
@@ -4571,7 +4574,7 @@ getIndexes(TableInfo tblinfo[], int numTables)
                              "c.oid AS conoid, "
                              "null AS condef, "
                              "(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, "
-                           "array_to_string(t.reloptions, ', ') AS options "
+                             "t.reloptions AS indreloptions "
                              "FROM pg_catalog.pg_index i "
                      "JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
                              "LEFT JOIN pg_catalog.pg_depend d "
@@ -4600,7 +4603,7 @@ getIndexes(TableInfo tblinfo[], int numTables)
                              "c.oid AS conoid, "
                              "null AS condef, "
                              "(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, "
-                             "null AS options "
+                             "null AS indreloptions "
                              "FROM pg_catalog.pg_index i "
                      "JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
                              "LEFT JOIN pg_catalog.pg_depend d "
@@ -4628,7 +4631,7 @@ getIndexes(TableInfo tblinfo[], int numTables)
                              "c.oid AS conoid, "
                              "null AS condef, "
                              "NULL AS tablespace, "
-                             "null AS options "
+                             "null AS indreloptions "
                              "FROM pg_catalog.pg_index i "
                      "JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
                              "LEFT JOIN pg_catalog.pg_depend d "
@@ -4659,7 +4662,7 @@ getIndexes(TableInfo tblinfo[], int numTables)
                              "t.oid AS conoid, "
                              "null AS condef, "
                              "NULL AS tablespace, "
-                             "null AS options "
+                             "null AS indreloptions "
                              "FROM pg_index i, pg_class t "
                              "WHERE t.oid = i.indexrelid "
                              "AND i.indrelid = '%u'::oid "
@@ -4685,7 +4688,7 @@ getIndexes(TableInfo tblinfo[], int numTables)
                              "t.oid AS conoid, "
                              "null AS condef, "
                              "NULL AS tablespace, "
-                             "null AS options "
+                             "null AS indreloptions "
                              "FROM pg_index i, pg_class t "
                              "WHERE t.oid = i.indexrelid "
                              "AND i.indrelid = '%u'::oid "
@@ -4713,7 +4716,7 @@ getIndexes(TableInfo tblinfo[], int numTables)
        i_conoid = PQfnumber(res, "conoid");
        i_condef = PQfnumber(res, "condef");
        i_tablespace = PQfnumber(res, "tablespace");
-       i_options = PQfnumber(res, "options");
+       i_indreloptions = PQfnumber(res, "indreloptions");
 
        indxinfo = (IndxInfo *) malloc(ntups * sizeof(IndxInfo));
        constrinfo = (ConstraintInfo *) malloc(ntups * sizeof(ConstraintInfo));
@@ -4732,7 +4735,7 @@ getIndexes(TableInfo tblinfo[], int numTables)
            indxinfo[j].indexdef = strdup(PQgetvalue(res, j, i_indexdef));
            indxinfo[j].indnkeys = atoi(PQgetvalue(res, j, i_indnkeys));
            indxinfo[j].tablespace = strdup(PQgetvalue(res, j, i_tablespace));
-           indxinfo[j].options = strdup(PQgetvalue(res, j, i_options));
+           indxinfo[j].indreloptions = strdup(PQgetvalue(res, j, i_indreloptions));
 
            /*
             * In pre-7.4 releases, indkeys may contain more entries than
@@ -12448,23 +12451,24 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
        if (tbinfo->relkind == RELKIND_FOREIGN_TABLE)
            appendPQExpBuffer(q, "\nSERVER %s", fmtId(srvname));
 
-       if ((tbinfo->reloptions && strlen(tbinfo->reloptions) > 0) ||
-         (tbinfo->toast_reloptions && strlen(tbinfo->toast_reloptions) > 0))
+       if (nonemptyReloptions(tbinfo->reloptions) ||
+           nonemptyReloptions(tbinfo->toast_reloptions))
        {
            bool        addcomma = false;
 
-           appendPQExpBuffer(q, "\nWITH (");
-           if (tbinfo->reloptions && strlen(tbinfo->reloptions) > 0)
+           appendPQExpBufferStr(q, "\nWITH (");
+           if (nonemptyReloptions(tbinfo->reloptions))
            {
                addcomma = true;
-               appendPQExpBuffer(q, "%s", tbinfo->reloptions);
+               fmtReloptionsArray(fout, q, tbinfo->reloptions, "");
            }
-           if (tbinfo->toast_reloptions && strlen(tbinfo->toast_reloptions) > 0)
+           if (nonemptyReloptions(tbinfo->toast_reloptions))
            {
-               appendPQExpBuffer(q, "%s%s", addcomma ? ", " : "",
-                                 tbinfo->toast_reloptions);
+               if (addcomma)
+                   appendPQExpBufferStr(q, ", ");
+               fmtReloptionsArray(fout, q, tbinfo->toast_reloptions, "toast.");
            }
-           appendPQExpBuffer(q, ")");
+           appendPQExpBufferChar(q, ')');
        }
 
        /* Dump generic options if any */
@@ -12959,8 +12963,12 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
 
            appendPQExpBuffer(q, ")");
 
-           if (indxinfo->options && strlen(indxinfo->options) > 0)
-               appendPQExpBuffer(q, " WITH (%s)", indxinfo->options);
+           if (nonemptyReloptions(indxinfo->indreloptions))
+           {
+               appendPQExpBufferStr(q, " WITH (");
+               fmtReloptionsArray(fout, q, indxinfo->indreloptions, "");
+               appendPQExpBufferChar(q, ')');
+           }
 
            if (coninfo->condeferrable)
            {
@@ -14572,6 +14580,83 @@ fmtCopyColumnList(const TableInfo *ti)
    return q->data;
 }
 
+/*
+ * Check if a reloptions array is nonempty.
+ */
+static bool
+nonemptyReloptions(const char *reloptions)
+{
+   /* Don't want to print it if it's just "{}" */
+   return (reloptions != NULL && strlen(reloptions) > 2);
+}
+
+/*
+ * Format a reloptions array and append it to the given buffer.
+ *
+ * "prefix" is prepended to the option names; typically it's "" or "toast.".
+ *
+ * Note: this logic should generally match the backend's flatten_reloptions()
+ * (in adt/ruleutils.c).
+ */
+static void
+fmtReloptionsArray(Archive *fout, PQExpBuffer buffer, const char *reloptions,
+                  const char *prefix)
+{
+   char      **options;
+   int         noptions;
+   int         i;
+
+   if (!parsePGArray(reloptions, &options, &noptions))
+   {
+       write_msg(NULL, "WARNING: could not parse reloptions array\n");
+       if (options)
+           free(options);
+       return;
+   }
+
+   for (i = 0; i < noptions; i++)
+   {
+       char       *option = options[i];
+       char       *name;
+       char       *separator;
+       char       *value;
+
+       /*
+        * Each array element should have the form name=value.  If the "=" is
+        * missing for some reason, treat it like an empty value.
+        */
+       name = option;
+       separator = strchr(option, '=');
+       if (separator)
+       {
+           *separator = '\0';
+           value = separator + 1;
+       }
+       else
+           value = "";
+
+       if (i > 0)
+           appendPQExpBufferStr(buffer, ", ");
+       appendPQExpBuffer(buffer, "%s%s=", prefix, fmtId(name));
+
+       /*
+        * In general we need to quote the value; but to avoid unnecessary
+        * clutter, do not quote if it is an identifier that would not need
+        * quoting.  (We could also allow numbers, but that is a bit trickier
+        * than it looks --- for example, are leading zeroes significant?  We
+        * don't want to assume very much here about what custom reloptions
+        * might mean.)
+        */
+       if (strcmp(fmtId(value), value) == 0)
+           appendPQExpBufferStr(buffer, value);
+       else
+           appendStringLiteralAH(buffer, value, fout);
+   }
+
+   if (options)
+       free(options);
+}
+
 /*
  * Convenience subroutine to execute a SQL command and check for
  * COMMAND_OK status.
index 90296ca7384d6abae25e5636e3fb64dcadd87b3a..be139c8aea50d2b2ccbd9c8369bfe45beabb4b14 100644 (file)
@@ -313,7 +313,7 @@ typedef struct _indxInfo
    TableInfo  *indextable;     /* link to table the index is for */
    char       *indexdef;
    char       *tablespace;     /* tablespace in which index is stored */
-   char       *options;        /* options specified by WITH (...) */
+   char       *indreloptions;  /* options specified by WITH (...) */
    int         indnkeys;
    Oid        *indkeys;
    bool        indisclustered;