Clean up pg_dump's handling of ownership for indexes (including

pkey/unique constraint indexes) and schemas.  Per report from
Michael Fuhr.

src/bin/pg_dump/pg_backup_archiver.c

@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *		$PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.100 2004/11/06 19:36:01 tgl Exp $
+ *		$PostgreSQL: pgsql/src/bin/pg_dump/pg_backup_archiver.c,v 1.101 2005/01/11 05:14:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -47,7 +47,7 @@ static char *modulename = gettext_noop("archiver");
 
 static ArchiveHandle *_allocAH(const char *FileSpec, const ArchiveFormat fmt,
 		 const int compression, ArchiveMode mode);
-static char *_getObjectFromDropStmt(const char *dropStmt, const char *type);
+static void _getObjectDescription(PQExpBuffer buf, TocEntry *te);
 static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isData, bool acl_pass);
 
 
@@ -2363,64 +2363,78 @@ _selectTablespace(ArchiveHandle *AH, const char *tablespace)
 	destroyPQExpBuffer(qry);
 }
 
-/**
+/*
- * Parses the dropStmt part of a TOC entry and returns
+ * Extract an object description for a TOC entry, and append it to buf.
- * a newly allocated string that is the object identifier
+ *
- * The caller must free the result.
+ * This is not quite as general as it may seem, since it really only
+ * handles constructing the right thing to put into ALTER ... OWNER TO.
+ *
+ * The whole thing is pretty grotty, but we are kind of stuck since the
+ * information used is all that's available in older dump files.
  */
-static char *
+static void
-_getObjectFromDropStmt(const char *dropStmt, const char *type)
+_getObjectDescription(PQExpBuffer buf, TocEntry *te)
 {
-	/* Chop "DROP" off the front and make a copy */
+	const char *type = te->desc;
-	char	   *first = strdup(dropStmt + 5);
+
-	char	   *last = first + strlen(first) - 1;		/* Points to the last
+	/* Use ALTER TABLE for views and sequences */
-														 * real char in extract */
+	if (strcmp(type, "VIEW") == 0 ||
-	char	   *buf = NULL;
+		strcmp(type, "SEQUENCE") == 0)
+		type = "TABLE";
+
+	/* We assume CONSTRAINTs are always pkey/unique indexes */
+	if (strcmp(type, "CONSTRAINT") == 0)
+		type = "INDEX";
+
+	/* objects named by a schema and name */
+	if (strcmp(type, "CONVERSION") == 0 ||
+		strcmp(type, "DOMAIN") == 0 ||
+		strcmp(type, "INDEX") == 0 ||
+		strcmp(type, "TABLE") == 0 ||
+		strcmp(type, "TYPE") == 0)
+	{
+		appendPQExpBuffer(buf, "%s %s", type, fmtId(te->namespace));
+		appendPQExpBuffer(buf, ".%s", fmtId(te->tag));
+		return;
+	}
+
+	/* objects named by just a name */
+	if (strcmp(type, "DATABASE") == 0 ||
+		strcmp(type, "SCHEMA") == 0)
+	{
+		appendPQExpBuffer(buf, "%s %s", type, fmtId(te->tag));
+		return;
+	}
 
 	/*
-	 * Loop from the end of the string until last char is no longer '\n'
+	 * These object types require additional decoration.  Fortunately,
-	 * or ';'
+	 * the information needed is exactly what's in the DROP command.
 	 */
+	if (strcmp(type, "AGGREGATE") == 0 ||
+		strcmp(type, "FUNCTION") == 0 ||
+		strcmp(type, "OPERATOR") == 0 ||
+		strcmp(type, "OPERATOR CLASS") == 0)
+	{
+		/* Chop "DROP " off the front and make a modifiable copy */
+		char	   *first = strdup(te->dropStmt + 5);
+		char	   *last;
+
+		/* point to last character in string */
+		last = first + strlen(first) - 1;
+
+		/* Strip off any ';' or '\n' at the end */
 		while (last >= first && (*last == '\n' || *last == ';'))
 			last--;
-
-	/* Insert end of string one place after last */
 		*(last + 1) = '\0';
 
-	/*
+		appendPQExpBufferStr(buf, first);
-	 * Take off CASCADE if necessary.  Only TYPEs seem to have this, but
-	 * may as well check for all
-	 */
-	if ((last - first) >= 8)
-	{
-		if (strcmp(last - 7, " CASCADE") == 0)
-			last -= 8;
-	}
 
-	/* Insert end of string one place after last */
-	*(last + 1) = '\0';
-
-	/* Special case VIEWs and SEQUENCEs.  They must use ALTER TABLE. */
-	if (strcmp(type, "VIEW") == 0 && (last - first) >= 5)
-	{
-		int			len = 6 + strlen(first + 5) + 1;
-
-		buf = malloc(len);
-		snprintf(buf, len, "TABLE %s", first + 5);
 		free(first);
+		return;
 	}
-	else if (strcmp(type, "SEQUENCE") == 0 && (last - first) >= 9)
-	{
-		int			len = 6 + strlen(first + 9) + 1;
 
-		buf = malloc(len);
+	write_msg(modulename, "WARNING: don't know how to set owner for object type %s\n",
-		snprintf(buf, len, "TABLE %s", first + 9);
+			  type);
-		free(first);
-	}
-	else
-		buf = first;
-
-	return buf;
 }
 
 static void
@@ -2497,13 +2511,14 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isDat
 	/*
 	 * Actually print the definition.
 	 *
-	 * Really crude hack for suppressing AUTHORIZATION clause of CREATE
+	 * Really crude hack for suppressing AUTHORIZATION clause that old
-	 * SCHEMA when --no-owner mode is selected.  This is ugly, but I see
+	 * pg_dump versions put into CREATE SCHEMA.  We have to do this when
+	 * --no-owner mode is selected.  This is ugly, but I see
 	 * no other good way ...
 	 */
-	if (AH->ropt && AH->ropt->noOwner && strcmp(te->desc, "SCHEMA") == 0)
+	if (ropt->noOwner && strcmp(te->desc, "SCHEMA") == 0)
 	{
-		ahprintf(AH, "CREATE SCHEMA %s;\n\n\n", te->tag);
+		ahprintf(AH, "CREATE SCHEMA %s;\n\n\n", fmtId(te->tag));
 	}
 	else
 	{
@@ -2513,29 +2528,51 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt, bool isDat
 
 	/*
 	 * If we aren't using SET SESSION AUTH to determine ownership, we must
-	 * instead issue an ALTER OWNER command.  Ugly, since we have to cons
+	 * instead issue an ALTER OWNER command.  We assume that anything without
-	 * one up based on the dropStmt.  We don't need this for schemas
+	 * a DROP command is not a separately ownable object.  All the categories
-	 * (since we use CREATE SCHEMA AUTHORIZATION instead), nor for some
+	 * with DROP commands must appear in one list or the other.
-	 * other object types.
 	 */
 	if (!ropt->noOwner && !ropt->use_setsessauth &&
-		strlen(te->owner) > 0 && strlen(te->dropStmt) > 0 &&
+		strlen(te->owner) > 0 && strlen(te->dropStmt) > 0)
-		(strcmp(te->desc, "AGGREGATE") == 0 ||
+	{
+		if (strcmp(te->desc, "AGGREGATE") == 0 ||
+			strcmp(te->desc, "CONSTRAINT") == 0 ||
 			strcmp(te->desc, "CONVERSION") == 0 ||
 			strcmp(te->desc, "DATABASE") == 0 ||
 			strcmp(te->desc, "DOMAIN") == 0 ||
 			strcmp(te->desc, "FUNCTION") == 0 ||
+			strcmp(te->desc, "INDEX") == 0 ||
 			strcmp(te->desc, "OPERATOR") == 0 ||
 			strcmp(te->desc, "OPERATOR CLASS") == 0 ||
+			strcmp(te->desc, "SCHEMA") == 0 ||
 			strcmp(te->desc, "TABLE") == 0 ||
 			strcmp(te->desc, "TYPE") == 0 ||
 			strcmp(te->desc, "VIEW") == 0 ||
-		 strcmp(te->desc, "SEQUENCE") == 0))
+			strcmp(te->desc, "SEQUENCE") == 0)
 		{
-		char	   *temp = _getObjectFromDropStmt(te->dropStmt, te->desc);
+			PQExpBuffer temp = createPQExpBuffer();
 
-		ahprintf(AH, "ALTER %s OWNER TO %s;\n\n", temp, fmtId(te->owner));
+			appendPQExpBuffer(temp, "ALTER ");
-		free(temp);
+			_getObjectDescription(temp, te);
+			appendPQExpBuffer(temp, " OWNER TO %s;", fmtId(te->owner));
+			ahprintf(AH, "%s\n\n", temp->data);
+			destroyPQExpBuffer(temp);
+		}
+		else if (strcmp(te->desc, "CAST") == 0 ||
+				 strcmp(te->desc, "CHECK CONSTRAINT") == 0 ||
+				 strcmp(te->desc, "DEFAULT") == 0 ||
+				 strcmp(te->desc, "FK CONSTRAINT") == 0 ||
+				 strcmp(te->desc, "PROCEDURAL LANGUAGE") == 0 ||
+				 strcmp(te->desc, "RULE") == 0 ||
+				 strcmp(te->desc, "TRIGGER") == 0)
+		{
+			/* these object types don't have separate owners */
+		}
+		else
+		{
+			write_msg(modulename, "WARNING: don't know how to set owner for object type %s\n",
+					  te->desc);
+		}
 	}
 
 	/*

src/bin/pg_dump/pg_dump.c

@@ -12,7 +12,7 @@
  *	by PostgreSQL
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/bin/pg_dump/pg_dump.c,v 1.398 2005/01/04 22:27:46 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/bin/pg_dump/pg_dump.c,v 1.399 2005/01/11 05:14:13 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -4428,23 +4428,14 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo)
 
 	qnspname = strdup(fmtId(nspinfo->dobj.name));
 
-	/*
-	 * Note that ownership is shown in the AUTHORIZATION clause, while the
-	 * archive entry is listed with empty owner (causing it to be emitted
-	 * with SET SESSION AUTHORIZATION DEFAULT). This seems the best way of
-	 * dealing with schemas owned by users without CREATE SCHEMA
-	 * privilege.  Further hacking has to be applied for --no-owner mode,
-	 * though!
-	 */
 	appendPQExpBuffer(delq, "DROP SCHEMA %s;\n", qnspname);
 
-	appendPQExpBuffer(q, "CREATE SCHEMA %s AUTHORIZATION %s;\n",
+	appendPQExpBuffer(q, "CREATE SCHEMA %s;\n", qnspname);
-					  qnspname, fmtId(nspinfo->usename));
 
 	ArchiveEntry(fout, nspinfo->dobj.catId, nspinfo->dobj.dumpId,
 				 nspinfo->dobj.name,
 				 NULL, NULL, 
-				 strcmp(nspinfo->dobj.name, "public") == 0 ? nspinfo->usename : "",
+				 nspinfo->usename,
 				 false, "SCHEMA", q->data, delq->data, NULL,
 				 nspinfo->dobj.dependencies, nspinfo->dobj.nDeps,
 				 NULL, NULL);