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 b8eaa2a5404405e837ab84b2df8da89938077e67..19e7e9ba8551207f3b7e5464a3fa4e06b6b02e01 100644 (file)
@@ -263,6 +263,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, RestoreOptions *ropt);
@@ -4336,10 +4339,10 @@ getTables(Archive *fout, 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 "
@@ -4376,10 +4379,10 @@ getTables(Archive *fout, 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 "
@@ -4416,8 +4419,8 @@ getTables(Archive *fout, 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 "
@@ -4454,8 +4457,8 @@ getTables(Archive *fout, 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 "
@@ -4491,8 +4494,8 @@ getTables(Archive *fout, 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 "
@@ -4528,7 +4531,7 @@ getTables(Archive *fout, 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 "
@@ -4987,7 +4990,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
                                i_conoid,
                                i_condef,
                                i_tablespace,
-                               i_options,
+                               i_indreloptions,
                                i_relpages;
        int                     ntups;
 
@@ -5044,7 +5047,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 "
@@ -5075,7 +5078,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 "
@@ -5102,7 +5105,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 "
@@ -5132,7 +5135,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 "
@@ -5161,7 +5164,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 "
@@ -5193,7 +5196,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 "
@@ -5220,7 +5223,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 "
@@ -5249,7 +5252,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));
@@ -5268,7 +5271,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
@@ -13201,8 +13204,12 @@ dumpTableSchema(Archive *fout, 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);
@@ -13446,21 +13453,22 @@ dumpTableSchema(Archive *fout, 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, ')');
                }
@@ -14034,8 +14042,12 @@ dumpConstraint(Archive *fout, 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)
                        {
@@ -14895,11 +14907,12 @@ dumpRule(Archive *fout, 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");
        }
 
        /*
@@ -15769,6 +15782,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 7df99bd6c8ddd19922c804895a7ee079796f9e62..d1c94e80ea1a5e603ebfdf3782f0f1303ae39dc0 100644 (file)
@@ -239,7 +239,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? */
@@ -314,7 +314,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;