Обсуждение: pg_dump versus rules, once again

I looked into the problem reported at
https://www.postgresql.org/message-id/b3690957-fd8c-6def-d3ec-e589887dd0f1%40codata.eu

It's easy to reproduce.  Given this simple database:

create table tt (f1 int primary key, f2 text);
create view vv as select * from tt group by f1;

pg_dump with the --clean option will generate

DROP RULE "_RETURN" ON public.vv;

which the backend rejects:

ERROR:  cannot drop rule _RETURN on view vv because view vv requires it
HINT:  You can drop view vv instead.

The reason for this is that because the view is dependent on tt's primary
key constraint (since it omits an otherwise-required "GROUP BY f2"),
pg_dump has a circular dependency to deal with: it wants to create the
view pre-data, but the view definition won't work until after the pkey has
been created post-data.

Our longtime solution to circularities involving views is to break the
view into CREATE TABLE and then CREATE RULE "_RETURN", exploiting a
horribly ancient backwards-compatibility hack in the backend that will
turn an empty table into a view if it gets a command to create an ON
SELECT rule for it.  That's fine until you add --clean to the mix, which
causes pg_dump to blindly emit a DROP RULE and then DROP TABLE.  Lose.

One way to fix this would be to add code to the backend so that
DROP RULE "_RETURN" converts the view back into a table, but ick.

We've talked before about replacing this _RETURN-rule business with
CREATE OR REPLACE VIEW, ie the idea would be for pg_dump to first emit
a dummy view with the right column names/types, say

CREATE VIEW vv AS SELECT null::int AS f1, null::text AS f2;

and then later when it's safe, emit CREATE OR REPLACE VIEW with the view's
real query.  The main point of this according to past discussion would be
to eliminate dump files' dependency on the _RETURN-rule implementation of
views, so that someday in the far future we could change that if we
wished.  However, if that were how pg_dump dealt with circular view
dependencies, then it would not take much more code to emit

CREATE OR REPLACE VIEW vv AS SELECT null::int AS f1, null::text AS f2;

as a substitute for the DROP RULE "_RETURN" step in a --clean script.
(Later, after we'd gotten rid of whatever was circularly depending on
that, we would emit DROP VIEW vv.)

So I'm thinking of going and doing this.  Any objections?

Although this is a bug fix, I'm not sure whether to consider
back-patching.  The case isn't that hard to work around -- either ignore
the error, or change your view to spell out its GROUP BY in full.
But it'd be annoying to hit this during pg_upgrade for instance.

CREATE OR REPLACE VIEW has existed since 7.3, so we're not creating much
of a portability problem at the server end if we make this change.
However, I notice that the kluge that was added to RestoreArchive() for
--if-exists will dump core (Assert failure or null pointer dereference)
if an archived dropStmt isn't what it expects.  I think that's broken
anyway, but it'd become actively broken as soon as we start handling views
this way, so we'd need to back-patch at least some change there.
Probably it's sufficient to teach that code to do nothing to statements
it doesn't recognize.

BTW, the ability to create a view that has this hazard has existed since
9.1.
        regards, tom lane

I wrote:
> We've talked before about replacing this _RETURN-rule business with
> CREATE OR REPLACE VIEW, ie the idea would be for pg_dump to first emit
> a dummy view with the right column names/types, say
> CREATE VIEW vv AS SELECT null::int AS f1, null::text AS f2;
> and then later when it's safe, emit CREATE OR REPLACE VIEW with the view's
> real query.

Here's a proposed patch for that.  It turns out there are some other
kluges that can be gotten rid of while we're at it: no longer need the
idea of reloptions being attached to a rule, for instance.

The changes in pg_backup_archiver.c would have to be back-patched
into all versions supporting --if-exists, so that they don't fail
on dump archives produced by patched versions.  We could possibly
put the rest only into HEAD; I remain of two minds about that.

            regards, tom lane

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index b938d79..b89bd99 100644
*** a/src/bin/pg_dump/pg_backup_archiver.c
--- b/src/bin/pg_dump/pg_backup_archiver.c
*************** RestoreArchive(Archive *AHX)
*** 521,527 ****
                           * knows how to do it, without depending on
                           * te->dropStmt; use that.  For other objects we need
                           * to parse the command.
-                          *
                           */
                          if (strncmp(te->desc, "BLOB", 4) == 0)
                          {
--- 521,526 ----
*************** RestoreArchive(Archive *AHX)
*** 529,538 ****
                          }
                          else
                          {
-                             char        buffer[40];
-                             char       *mark;
                              char       *dropStmt = pg_strdup(te->dropStmt);
!                             char       *dropStmtPtr = dropStmt;
                              PQExpBuffer ftStmt = createPQExpBuffer();

                              /*
--- 528,535 ----
                          }
                          else
                          {
                              char       *dropStmt = pg_strdup(te->dropStmt);
!                             char       *dropStmtOrig = dropStmt;
                              PQExpBuffer ftStmt = createPQExpBuffer();

                              /*
*************** RestoreArchive(Archive *AHX)
*** 549,566 ****
                              /*
                               * ALTER TABLE..ALTER COLUMN..DROP DEFAULT does
                               * not support the IF EXISTS clause, and therefore
!                              * we simply emit the original command for such
!                              * objects. For other objects, we need to extract
!                              * the first part of the DROP which includes the
!                              * object type. Most of the time this matches
                               * te->desc, so search for that; however for the
                               * different kinds of CONSTRAINTs, we know to
                               * search for hardcoded "DROP CONSTRAINT" instead.
                               */
!                             if (strcmp(te->desc, "DEFAULT") == 0)
                                  appendPQExpBufferStr(ftStmt, dropStmt);
                              else
                              {
                                  if (strcmp(te->desc, "CONSTRAINT") == 0 ||
                                   strcmp(te->desc, "CHECK CONSTRAINT") == 0 ||
                                      strcmp(te->desc, "FK CONSTRAINT") == 0)
--- 546,573 ----
                              /*
                               * ALTER TABLE..ALTER COLUMN..DROP DEFAULT does
                               * not support the IF EXISTS clause, and therefore
!                              * we simply emit the original command for DEFAULT
!                              * objects (modulo the adjustment made above).
!                              *
!                              * If we used CREATE OR REPLACE VIEW as a means of
!                              * quasi-dropping an ON SELECT rule, that should
!                              * be emitted unchanged as well.
!                              *
!                              * For other object types, we need to extract the
!                              * first part of the DROP which includes the
!                              * object type.  Most of the time this matches
                               * te->desc, so search for that; however for the
                               * different kinds of CONSTRAINTs, we know to
                               * search for hardcoded "DROP CONSTRAINT" instead.
                               */
!                             if (strcmp(te->desc, "DEFAULT") == 0 ||
!                                 strncmp(dropStmt, "CREATE OR REPLACE VIEW", 22) == 0)
                                  appendPQExpBufferStr(ftStmt, dropStmt);
                              else
                              {
+                                 char        buffer[40];
+                                 char       *mark;
+
                                  if (strcmp(te->desc, "CONSTRAINT") == 0 ||
                                   strcmp(te->desc, "CHECK CONSTRAINT") == 0 ||
                                      strcmp(te->desc, "FK CONSTRAINT") == 0)
*************** RestoreArchive(Archive *AHX)
*** 570,588 ****
                                               te->desc);

                                  mark = strstr(dropStmt, buffer);
-                                 Assert(mark != NULL);

!                                 *mark = '\0';
!                                 appendPQExpBuffer(ftStmt, "%s%s IF EXISTS%s",
!                                                   dropStmt, buffer,
!                                                   mark + strlen(buffer));
                              }

                              ahprintf(AH, "%s", ftStmt->data);

                              destroyPQExpBuffer(ftStmt);
!
!                             pg_free(dropStmtPtr);
                          }
                      }
                  }
--- 577,604 ----
                                               te->desc);

                                  mark = strstr(dropStmt, buffer);

!                                 if (mark)
!                                 {
!                                     *mark = '\0';
!                                     appendPQExpBuffer(ftStmt, "%s%s IF EXISTS%s",
!                                                       dropStmt, buffer,
!                                                       mark + strlen(buffer));
!                                 }
!                                 else
!                                 {
!                                     /* complain and emit unmodified command */
!                                     write_msg(modulename,
!                                               "WARNING: could not find where to insert IF EXISTS in statement
\"%s\"\n",
!                                               dropStmtOrig);
!                                     appendPQExpBufferStr(ftStmt, dropStmt);
!                                 }
                              }

                              ahprintf(AH, "%s", ftStmt->data);

                              destroyPQExpBuffer(ftStmt);
!                             pg_free(dropStmtOrig);
                          }
                      }
                  }
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index ee1f673..9f59f53 100644
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** getTables(Archive *fout, int *numTables)
*** 5484,5490 ****
              tblinfo[i].dobj.dump &= ~DUMP_COMPONENT_ACL;

          tblinfo[i].interesting = tblinfo[i].dobj.dump ? true : false;
!
          tblinfo[i].postponed_def = false;        /* might get set during sort */

          /*
--- 5486,5492 ----
              tblinfo[i].dobj.dump &= ~DUMP_COMPONENT_ACL;

          tblinfo[i].interesting = tblinfo[i].dobj.dump ? true : false;
!         tblinfo[i].dummy_view = false;    /* might get set during sort */
          tblinfo[i].postponed_def = false;        /* might get set during sort */

          /*
*************** getRules(Archive *fout, int *numRules)
*** 6205,6220 ****
          }
          else
              ruleinfo[i].separate = true;
-
-         /*
-          * If we're forced to break a dependency loop by dumping a view as a
-          * table and separate _RETURN rule, we'll move the view's reloptions
-          * to the rule.  (This is necessary because tables and views have
-          * different valid reloptions, so we can't apply the options until the
-          * backend knows it's a view.)  Otherwise the rule's reloptions stay
-          * NULL.
-          */
-         ruleinfo[i].reloptions = NULL;
      }

      PQclear(res);
--- 6207,6212 ----
*************** createViewAsClause(Archive *fout, TableI
*** 13976,13981 ****
--- 13968,14021 ----
  }

  /*
+  * Create a dummy AS clause for a view.  This is used when the real view
+  * definition has to be postponed because of circular dependencies.
+  * We must duplicate the view's external properties -- column names and types
+  * (including collation) -- so that it works for subsequent references.
+  *
+  * This returns a new buffer which must be freed by the caller.
+  */
+ static PQExpBuffer
+ createDummyViewAsClause(Archive *fout, TableInfo *tbinfo)
+ {
+     PQExpBuffer result = createPQExpBuffer();
+     int            j;
+
+     appendPQExpBufferStr(result, "SELECT");
+
+     for (j = 0; j < tbinfo->numatts; j++)
+     {
+         if (j > 0)
+             appendPQExpBufferChar(result, ',');
+         appendPQExpBufferStr(result, "\n    ");
+
+         appendPQExpBuffer(result, "NULL::%s", tbinfo->atttypnames[j]);
+
+         /*
+          * Must add collation if not default for the type, because CREATE OR
+          * REPLACE VIEW won't change it
+          */
+         if (OidIsValid(tbinfo->attcollation[j]))
+         {
+             CollInfo   *coll;
+
+             coll = findCollationByOid(tbinfo->attcollation[j]);
+             if (coll)
+             {
+                 /* always schema-qualify, don't try to be smart */
+                 appendPQExpBuffer(result, " COLLATE %s.",
+                                   fmtId(coll->dobj.namespace->dobj.name));
+                 appendPQExpBufferStr(result, fmtId(coll->dobj.name));
+             }
+         }
+
+         appendPQExpBuffer(result, " AS %s", fmtId(tbinfo->attnames[j]));
+     }
+
+     return result;
+ }
+
+ /*
   * dumpTableSchema
   *      write the declaration (not data) of one user-defined table or view
   */
*************** dumpTableSchema(Archive *fout, TableInfo
*** 14008,14013 ****
--- 14048,14057 ----
      {
          PQExpBuffer result;

+         /*
+          * Note: keep this code in sync with the is_view case in dumpRule()
+          */
+
          reltypename = "VIEW";

          /*
*************** dumpTableSchema(Archive *fout, TableInfo
*** 14024,14040 ****
                                               tbinfo->dobj.catId.oid, false);

          appendPQExpBuffer(q, "CREATE VIEW %s", fmtId(tbinfo->dobj.name));
!         if (nonemptyReloptions(tbinfo->reloptions))
          {
!             appendPQExpBufferStr(q, " WITH (");
!             appendReloptionsArrayAH(q, tbinfo->reloptions, "", fout);
!             appendPQExpBufferChar(q, ')');
          }
-         result = createViewAsClause(fout, tbinfo);
          appendPQExpBuffer(q, " AS\n%s", result->data);
          destroyPQExpBuffer(result);

!         if (tbinfo->checkoption != NULL)
              appendPQExpBuffer(q, "\n  WITH %s CHECK OPTION", tbinfo->checkoption);
          appendPQExpBufferStr(q, ";\n");

--- 14068,14089 ----
                                               tbinfo->dobj.catId.oid, false);

          appendPQExpBuffer(q, "CREATE VIEW %s", fmtId(tbinfo->dobj.name));
!         if (tbinfo->dummy_view)
!             result = createDummyViewAsClause(fout, tbinfo);
!         else
          {
!             if (nonemptyReloptions(tbinfo->reloptions))
!             {
!                 appendPQExpBufferStr(q, " WITH (");
!                 appendReloptionsArrayAH(q, tbinfo->reloptions, "", fout);
!                 appendPQExpBufferChar(q, ')');
!             }
!             result = createViewAsClause(fout, tbinfo);
          }
          appendPQExpBuffer(q, " AS\n%s", result->data);
          destroyPQExpBuffer(result);

!         if (tbinfo->checkoption != NULL && !tbinfo->dummy_view)
              appendPQExpBuffer(q, "\n  WITH %s CHECK OPTION", tbinfo->checkoption);
          appendPQExpBufferStr(q, ";\n");

*************** dumpRule(Archive *fout, RuleInfo *rinfo)
*** 15646,15651 ****
--- 15695,15701 ----
  {
      DumpOptions *dopt = fout->dopt;
      TableInfo  *tbinfo = rinfo->ruletable;
+     bool        is_view;
      PQExpBuffer query;
      PQExpBuffer cmd;
      PQExpBuffer delcmd;
*************** dumpRule(Archive *fout, RuleInfo *rinfo)
*** 15665,15670 ****
--- 15715,15726 ----
          return;

      /*
+      * If it's an ON SELECT rule, we want to print it as a view definition,
+      * instead of a rule.
+      */
+     is_view = (rinfo->ev_type == '1' && rinfo->is_instead);
+
+     /*
       * Make sure we are in proper schema.
       */
      selectSourceSchema(fout, tbinfo->dobj.namespace->dobj.name);
*************** dumpRule(Archive *fout, RuleInfo *rinfo)
*** 15674,15693 ****
      delcmd = createPQExpBuffer();
      labelq = createPQExpBuffer();

!     appendPQExpBuffer(query,
!       "SELECT pg_catalog.pg_get_ruledef('%u'::pg_catalog.oid) AS definition",
!                       rinfo->dobj.catId.oid);
!
!     res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
!
!     if (PQntuples(res) != 1)
      {
!         write_msg(NULL, "query to get rule \"%s\" for table \"%s\" failed: wrong number of rows returned\n",
!                   rinfo->dobj.name, tbinfo->dobj.name);
!         exit_nicely(1);
      }

!     printfPQExpBuffer(cmd, "%s\n", PQgetvalue(res, 0, 0));

      /*
       * Add the command to alter the rules replication firing semantics if it
--- 15730,15779 ----
      delcmd = createPQExpBuffer();
      labelq = createPQExpBuffer();

!     if (is_view)
      {
!         PQExpBuffer result;
!
!         /*
!          * We need OR REPLACE here because we'll be replacing a dummy view.
!          * Otherwise this should look largely like the regular view dump code.
!          */
!         appendPQExpBuffer(cmd, "CREATE OR REPLACE VIEW %s",
!                           fmtId(tbinfo->dobj.name));
!         if (nonemptyReloptions(tbinfo->reloptions))
!         {
!             appendPQExpBufferStr(cmd, " WITH (");
!             appendReloptionsArrayAH(cmd, tbinfo->reloptions, "", fout);
!             appendPQExpBufferChar(cmd, ')');
!         }
!         result = createViewAsClause(fout, tbinfo);
!         appendPQExpBuffer(cmd, " AS\n%s", result->data);
!         destroyPQExpBuffer(result);
!         if (tbinfo->checkoption != NULL)
!             appendPQExpBuffer(cmd, "\n  WITH %s CHECK OPTION",
!                               tbinfo->checkoption);
!         appendPQExpBufferStr(cmd, ";\n");
      }
+     else
+     {
+         /* In the rule case, just print pg_get_ruledef's result verbatim */
+         appendPQExpBuffer(query,
+                     "SELECT pg_catalog.pg_get_ruledef('%u'::pg_catalog.oid)",
+                           rinfo->dobj.catId.oid);

!         res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
!
!         if (PQntuples(res) != 1)
!         {
!             write_msg(NULL, "query to get rule \"%s\" for table \"%s\" failed: wrong number of rows returned\n",
!                       rinfo->dobj.name, tbinfo->dobj.name);
!             exit_nicely(1);
!         }
!
!         printfPQExpBuffer(cmd, "%s\n", PQgetvalue(res, 0, 0));
!
!         PQclear(res);
!     }

      /*
       * Add the command to alter the rules replication firing semantics if it
*************** dumpRule(Archive *fout, RuleInfo *rinfo)
*** 15714,15738 ****
      }

      /*
!      * Apply view's reloptions when its ON SELECT rule is separate.
       */
!     if (nonemptyReloptions(rinfo->reloptions))
      {
!         appendPQExpBuffer(cmd, "ALTER VIEW %s SET (",
                            fmtId(tbinfo->dobj.name));
-         appendReloptionsArrayAH(cmd, rinfo->reloptions, "", fout);
-         appendPQExpBufferStr(cmd, ");\n");
      }
-
-     /*
-      * DROP must be fully qualified in case same name appears in pg_catalog
-      */
-     appendPQExpBuffer(delcmd, "DROP RULE %s ",
-                       fmtId(rinfo->dobj.name));
-     appendPQExpBuffer(delcmd, "ON %s.",
-                       fmtId(tbinfo->dobj.namespace->dobj.name));
-     appendPQExpBuffer(delcmd, "%s;\n",
-                       fmtId(tbinfo->dobj.name));

      appendPQExpBuffer(labelq, "RULE %s",
                        fmtId(rinfo->dobj.name));
--- 15800,15833 ----
      }

      /*
!      * DROP must be fully qualified in case same name appears in pg_catalog
       */
!     if (is_view)
      {
!         /*
!          * We can't DROP a view's ON SELECT rule.  Instead, use CREATE OR
!          * REPLACE VIEW to replace the rule with something with minimal
!          * dependencies.
!          */
!         PQExpBuffer result;
!
!         appendPQExpBuffer(delcmd, "CREATE OR REPLACE VIEW %s.",
!                           fmtId(tbinfo->dobj.namespace->dobj.name));
!         appendPQExpBuffer(delcmd, "%s",
!                           fmtId(tbinfo->dobj.name));
!         result = createDummyViewAsClause(fout, tbinfo);
!         appendPQExpBuffer(delcmd, " AS\n%s;\n", result->data);
!         destroyPQExpBuffer(result);
!     }
!     else
!     {
!         appendPQExpBuffer(delcmd, "DROP RULE %s ",
!                           fmtId(rinfo->dobj.name));
!         appendPQExpBuffer(delcmd, "ON %s.",
!                           fmtId(tbinfo->dobj.namespace->dobj.name));
!         appendPQExpBuffer(delcmd, "%s;\n",
                            fmtId(tbinfo->dobj.name));
      }

      appendPQExpBuffer(labelq, "RULE %s",
                        fmtId(rinfo->dobj.name));
*************** dumpRule(Archive *fout, RuleInfo *rinfo)
*** 15759,15766 ****
                      tbinfo->rolname,
                      rinfo->dobj.catId, 0, rinfo->dobj.dumpId);

-     PQclear(res);
-
      free(tag);
      destroyPQExpBuffer(query);
      destroyPQExpBuffer(cmd);
--- 15854,15859 ----
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 642c4d5..f3e5977 100644
*** a/src/bin/pg_dump/pg_dump.h
--- b/src/bin/pg_dump/pg_dump.h
*************** typedef struct _tableInfo
*** 287,292 ****
--- 287,293 ----
      int            relpages;        /* table's size in pages (from pg_class) */

      bool        interesting;    /* true if need to collect more data */
+     bool        dummy_view;        /* view's real definition must be postponed */
      bool        postponed_def;    /* matview must be postponed into post-data */

      /*
*************** typedef struct _ruleInfo
*** 364,371 ****
      char        ev_enabled;
      bool        separate;        /* TRUE if must dump as separate item */
      /* separate is always true for non-ON SELECT rules */
-     char       *reloptions;        /* options specified by WITH (...) */
-     /* reloptions is only set if we need to dump the options with the rule */
  } RuleInfo;

  typedef struct _triggerInfo
--- 365,370 ----
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 5b96334..9966423 100644
*** a/src/bin/pg_dump/pg_dump_sort.c
--- b/src/bin/pg_dump/pg_dump_sort.c
*************** repairViewRuleLoop(DumpableObject *viewo
*** 786,791 ****
--- 786,792 ----
  {
      /* remove rule's dependency on view */
      removeObjectDependency(ruleobj, viewobj->dumpId);
+     /* flags on the two objects are already set correctly for this case */
  }

  /*
*************** repairViewRuleMultiLoop(DumpableObject *
*** 805,831 ****
  {
      TableInfo  *viewinfo = (TableInfo *) viewobj;
      RuleInfo   *ruleinfo = (RuleInfo *) ruleobj;
-     int            i;

      /* remove view's dependency on rule */
      removeObjectDependency(viewobj, ruleobj->dumpId);
!     /* pretend view is a plain table and dump it that way */
!     viewinfo->relkind = 'r';    /* RELKIND_RELATION */
      /* mark rule as needing its own dump */
      ruleinfo->separate = true;
-     /* move any reloptions from view to rule */
-     if (viewinfo->reloptions)
-     {
-         ruleinfo->reloptions = viewinfo->reloptions;
-         viewinfo->reloptions = NULL;
-     }
      /* put back rule's dependency on view */
      addObjectDependency(ruleobj, viewobj->dumpId);
      /* now that rule is separate, it must be post-data */
      addObjectDependency(ruleobj, postDataBoundId);
-     /* also, any triggers on the view must be dumped after the rule */
-     for (i = 0; i < viewinfo->numTriggers; i++)
-         addObjectDependency(&(viewinfo->triggers[i].dobj), ruleobj->dumpId);
  }

  /*
--- 806,822 ----
  {
      TableInfo  *viewinfo = (TableInfo *) viewobj;
      RuleInfo   *ruleinfo = (RuleInfo *) ruleobj;

      /* remove view's dependency on rule */
      removeObjectDependency(viewobj, ruleobj->dumpId);
!     /* mark view to be printed with a dummy definition */
!     viewinfo->dummy_view = true;
      /* mark rule as needing its own dump */
      ruleinfo->separate = true;
      /* put back rule's dependency on view */
      addObjectDependency(ruleobj, viewobj->dumpId);
      /* now that rule is separate, it must be post-data */
      addObjectDependency(ruleobj, postDataBoundId);
  }

  /*

On Wed, Nov 16, 2016 at 10:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> The changes in pg_backup_archiver.c would have to be back-patched
> into all versions supporting --if-exists, so that they don't fail
> on dump archives produced by patched versions.

Even if you patch future minor releases, past minor releases are still
going to exist out there in the wild for a long, long time.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Nov 16, 2016 at 10:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The changes in pg_backup_archiver.c would have to be back-patched
>> into all versions supporting --if-exists, so that they don't fail
>> on dump archives produced by patched versions.

> Even if you patch future minor releases, past minor releases are still
> going to exist out there in the wild for a long, long time.

Yeah, but it would only matter if you try to use pg_restore --clean --if-exists
with an archive file that happens to contain a view that has this issue.
Such cases would previously have failed anyway, because of precisely
the bug at issue ... and there aren't very many of them, or we'd have
noticed the problem before.  So I don't feel *too* bad about this,
I just want to make sure we have a solution available.
        regards, tom lane

On Wed, Nov 16, 2016 at 10:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Wed, Nov 16, 2016 at 10:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> The changes in pg_backup_archiver.c would have to be back-patched
>>> into all versions supporting --if-exists, so that they don't fail
>>> on dump archives produced by patched versions.
>
>> Even if you patch future minor releases, past minor releases are still
>> going to exist out there in the wild for a long, long time.
>
> Yeah, but it would only matter if you try to use pg_restore --clean --if-exists
> with an archive file that happens to contain a view that has this issue.
> Such cases would previously have failed anyway, because of precisely
> the bug at issue ... and there aren't very many of them, or we'd have
> noticed the problem before.  So I don't feel *too* bad about this,
> I just want to make sure we have a solution available.

Right, OK.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company