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 6cc28336e14aa4b3d5030447a939518ed4dbddcb..56c0528ee3fece89c3cb3f6091687800f3c303e1 100644 (file)
@@ -256,6 +256,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, PQExpBuffer buffer);
+static bool nonemptyReloptions(const char *reloptions);
+static void fmtReloptionsArray(Archive *fout, PQExpBuffer buffer,
+                  const char *reloptions, const char *prefix);
 static char *get_synchronized_snapshot(Archive *fout);
 static PGresult *ExecuteSqlQueryForSingleRow(Archive *fout, char *query);
 static void setupDumpWorker(Archive *AHX, DumpOptions *dopt, RestoreOptions *ropt);
@@ -4604,10 +4607,10 @@ getTables(Archive *fout, DumpOptions *dopt, 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(array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded'), ', ') AS reloptions, "
+                         "array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded') AS reloptions, "
                          "CASE WHEN 'check_option=local' = ANY (c.reloptions) THEN 'LOCAL'::text "
                          "WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption, "
-                         "array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
+                         "tc.reloptions AS toast_reloptions "
                          "FROM pg_class c "
                          "LEFT JOIN pg_depend d ON "
                          "(c.relkind = '%c' AND "
@@ -4646,10 +4649,10 @@ getTables(Archive *fout, DumpOptions *dopt, 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(array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded'), ', ') AS reloptions, "
+                         "array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded') AS reloptions, "
                          "CASE WHEN 'check_option=local' = ANY (c.reloptions) THEN 'LOCAL'::text "
                          "WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption, "
-                         "array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
+                         "tc.reloptions AS toast_reloptions "
                          "FROM pg_class c "
                          "LEFT JOIN pg_depend d ON "
                          "(c.relkind = '%c' AND "
@@ -4688,10 +4691,10 @@ getTables(Archive *fout, DumpOptions *dopt, 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(array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded'), ', ') AS reloptions, "
+                         "array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded') AS reloptions, "
                          "CASE WHEN 'check_option=local' = ANY (c.reloptions) THEN 'LOCAL'::text "
                          "WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption, "
-                         "array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
+                         "tc.reloptions AS toast_reloptions "
                          "FROM pg_class c "
                          "LEFT JOIN pg_depend d ON "
                          "(c.relkind = '%c' AND "
@@ -4730,8 +4733,8 @@ getTables(Archive *fout, DumpOptions *dopt, 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 "
@@ -4770,8 +4773,8 @@ getTables(Archive *fout, DumpOptions *dopt, 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 "
@@ -4809,8 +4812,8 @@ getTables(Archive *fout, DumpOptions *dopt, 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 "
@@ -4848,7 +4851,7 @@ getTables(Archive *fout, DumpOptions *dopt, 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 "
@@ -5321,7 +5324,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
                i_conoid,
                i_condef,
                i_tablespace,
-               i_options,
+               i_indreloptions,
                i_relpages;
    int         ntups;
 
@@ -5379,7 +5382,7 @@ getIndexes(Archive *fout, 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 "
@@ -5410,7 +5413,7 @@ getIndexes(Archive *fout, 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 "
@@ -5437,7 +5440,7 @@ getIndexes(Archive *fout, 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 "
@@ -5467,7 +5470,7 @@ getIndexes(Archive *fout, 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 "
@@ -5496,7 +5499,7 @@ getIndexes(Archive *fout, 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 "
@@ -5528,7 +5531,7 @@ getIndexes(Archive *fout, 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 "
@@ -5555,7 +5558,7 @@ getIndexes(Archive *fout, 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 "
@@ -5584,7 +5587,7 @@ getIndexes(Archive *fout, 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 *) pg_malloc(ntups * sizeof(IndxInfo));
        constrinfo = (ConstraintInfo *) pg_malloc(ntups * sizeof(ConstraintInfo));
@@ -5603,7 +5606,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
            indxinfo[j].indexdef = pg_strdup(PQgetvalue(res, j, i_indexdef));
            indxinfo[j].indnkeys = atoi(PQgetvalue(res, j, i_indnkeys));
            indxinfo[j].tablespace = pg_strdup(PQgetvalue(res, j, i_tablespace));
-           indxinfo[j].options = pg_strdup(PQgetvalue(res, j, i_options));
+           indxinfo[j].indreloptions = pg_strdup(PQgetvalue(res, j, i_indreloptions));
 
            /*
             * In pre-7.4 releases, indkeys may contain more entries than
@@ -10255,7 +10258,8 @@ dumpFunc(Archive *fout, DumpOptions *dopt, FuncInfo *finfo)
                     "pg_catalog.pg_get_function_result(oid) AS funcresult, "
                          "array_to_string(protrftypes, ' ') AS protrftypes, "
                          "proiswindow, provolatile, proisstrict, prosecdef, "
-                         "proleakproof, proconfig, procost, prorows, proparallel, "
+                         "proleakproof, proconfig, procost, prorows, "
+                         "proparallel, "
                          "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
                          "FROM pg_catalog.pg_proc "
                          "WHERE oid = '%u'::pg_catalog.oid",
@@ -13869,8 +13873,12 @@ dumpTableSchema(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo)
                                             tbinfo->dobj.catId.oid, false);
 
        appendPQExpBuffer(q, "CREATE VIEW %s", fmtId(tbinfo->dobj.name));
-       if (tbinfo->reloptions && strlen(tbinfo->reloptions) > 0)
-           appendPQExpBuffer(q, " WITH (%s)", tbinfo->reloptions);
+       if (nonemptyReloptions(tbinfo->reloptions))
+       {
+           appendPQExpBufferStr(q, " WITH (");
+           fmtReloptionsArray(fout, q, tbinfo->reloptions, "");
+           appendPQExpBufferChar(q, ')');
+       }
        result = createViewAsClause(fout, tbinfo);
        appendPQExpBuffer(q, " AS\n%s", result->data);
        destroyPQExpBuffer(result);
@@ -14114,21 +14122,22 @@ dumpTableSchema(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo)
                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;
 
            appendPQExpBufferStr(q, "\nWITH (");
-           if (tbinfo->reloptions && strlen(tbinfo->reloptions) > 0)
+           if (nonemptyReloptions(tbinfo->reloptions))
            {
                addcomma = true;
-               appendPQExpBufferStr(q, 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.");
            }
            appendPQExpBufferChar(q, ')');
        }
@@ -14710,8 +14719,12 @@ dumpConstraint(Archive *fout, DumpOptions *dopt, ConstraintInfo *coninfo)
 
            appendPQExpBufferChar(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)
            {
@@ -15571,11 +15584,12 @@ dumpRule(Archive *fout, DumpOptions *dopt, RuleInfo *rinfo)
    /*
     * Apply view's reloptions when its ON SELECT rule is separate.
     */
-   if (rinfo->reloptions && strlen(rinfo->reloptions) > 0)
+   if (nonemptyReloptions(rinfo->reloptions))
    {
-       appendPQExpBuffer(cmd, "ALTER VIEW %s SET (%s);\n",
-                         fmtId(tbinfo->dobj.name),
-                         rinfo->reloptions);
+       appendPQExpBuffer(cmd, "ALTER VIEW %s SET (",
+                         fmtId(tbinfo->dobj.name));
+       fmtReloptionsArray(fout, cmd, rinfo->reloptions, "");
+       appendPQExpBufferStr(cmd, ");\n");
    }
 
    /*
@@ -16448,6 +16462,83 @@ fmtCopyColumnList(const TableInfo *ti, PQExpBuffer buffer)
    return buffer->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);
+}
+
 /*
  * Execute an SQL query and verify that we got exactly one row back.
  */
index 03756ba100a63c646168f58f9efd4865f51d999b..ba37c4c6e8e77e8b5b0c23033c07228baaae0ef6 100644 (file)
@@ -205,7 +205,7 @@ typedef struct _tableInfo
    char        relreplident;   /* replica identifier */
    char       *reltablespace;  /* relation tablespace */
    char       *reloptions;     /* options specified by WITH (...) */
-   char       *checkoption;    /* WITH CHECK OPTION */
+   char       *checkoption;    /* WITH CHECK OPTION, if any */
    char       *toast_reloptions;       /* WITH options for the TOAST table */
    bool        hasindex;       /* does it have any indexes? */
    bool        hasrules;       /* does it have any rules? */
@@ -282,7 +282,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;