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);
  }
  
  /*
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to