Patch for gripe about pg_dump -C not dumping database privileges

Started by Tom Laneover 20 years ago1 messagespatches
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

Attached is a proposed patch for bug #2085. It's pretty grotty because
it introduces a new TOC tag type into the pg_dump format, which makes it
incompatible with existing pg_restores; so I think we probably couldn't
back-patch this, it'd have to be treated as a new feature for 8.2.
I don't see any other way to do it, though, because we want pg_restore
to emit the information under different conditions than those under
which it emits regular object ACLs (namely, only if -C mode).

Comments, better ideas? Should we just leave well enough alone?
IMHO #2085 is a definition disagreement, not an outright bug.

regards, tom lane

*** src/bin/pg_dump/pg_backup_archiver.c.orig	Tue Nov 22 16:06:41 2005
--- src/bin/pg_dump/pg_backup_archiver.c	Fri Dec  2 17:01:46 2005
***************
*** 1877,1882 ****
--- 1877,1885 ----
  	if (!ropt->create && strcmp(te->desc, "DATABASE") == 0)
  		return 0;
+ 	if ((!ropt->create || ropt->aclsSkip) &&
+ 		strcmp(te->desc, "DATABASE ACL") == 0)
+ 		return 0;
  	/* Check if tablename only is wanted */
  	if (ropt->selTypes)
*** src/bin/pg_dump/pg_dump.c.orig	Tue Nov 22 16:06:41 2005
--- src/bin/pg_dump/pg_dump.c	Fri Dec  2 16:54:48 2005
***************
*** 1168,1179 ****
--- 1168,1181 ----
  				i_oid,
  				i_dba,
  				i_encoding,
+ 				i_datacl,
  				i_tablespace;
  	CatalogId	dbCatId;
  	DumpId		dbDumpId;
  	const char *datname,
  			   *dba,
  			   *encoding,
+ 			   *datacl,
  			   *tablespace;
  	datname = PQdb(g_conn);
***************
*** 1190,1206 ****
--- 1192,1222 ----
  		appendPQExpBuffer(dbQry, "SELECT tableoid, oid, "
  						  "(%s datdba) as dba, "
  						  "pg_encoding_to_char(encoding) as encoding, "
+ 						  "datacl, "
  						  "(SELECT spcname FROM pg_tablespace t WHERE t.oid = dattablespace) as tablespace "
  						  "FROM pg_database "
  						  "WHERE datname = ",
  						  username_subquery);
  		appendStringLiteral(dbQry, datname, true);
  	}
+ 	else if (g_fout->remoteVersion >= 70300)
+ 	{
+ 		appendPQExpBuffer(dbQry, "SELECT tableoid, oid, "
+ 						  "(%s datdba) as dba, "
+ 						  "pg_encoding_to_char(encoding) as encoding, "
+ 						  "datacl, "
+ 						  "NULL as tablespace "
+ 						  "FROM pg_database "
+ 						  "WHERE datname = ",
+ 						  username_subquery);
+ 		appendStringLiteral(dbQry, datname, true);
+ 	}
  	else if (g_fout->remoteVersion >= 70100)
  	{
  		appendPQExpBuffer(dbQry, "SELECT tableoid, oid, "
  						  "(%s datdba) as dba, "
  						  "pg_encoding_to_char(encoding) as encoding, "
+ 						  "NULL as datacl, "
  						  "NULL as tablespace "
  						  "FROM pg_database "
  						  "WHERE datname = ",
***************
*** 1214,1219 ****
--- 1230,1236 ----
  						  "oid, "
  						  "(%s datdba) as dba, "
  						  "pg_encoding_to_char(encoding) as encoding, "
+ 						  "NULL as datacl, "
  						  "NULL as tablespace "
  						  "FROM pg_database "
  						  "WHERE datname = ",
***************
*** 1244,1255 ****
--- 1261,1274 ----
  	i_oid = PQfnumber(res, "oid");
  	i_dba = PQfnumber(res, "dba");
  	i_encoding = PQfnumber(res, "encoding");
+ 	i_datacl = PQfnumber(res, "datacl");
  	i_tablespace = PQfnumber(res, "tablespace");

dbCatId.tableoid = atooid(PQgetvalue(res, 0, i_tableoid));
dbCatId.oid = atooid(PQgetvalue(res, 0, i_oid));
dba = PQgetvalue(res, 0, i_dba);
encoding = PQgetvalue(res, 0, i_encoding);
+ datacl = PQgetvalue(res, 0, i_datacl);
tablespace = PQgetvalue(res, 0, i_tablespace);

  	appendPQExpBuffer(creaQry, "CREATE DATABASE %s WITH TEMPLATE = template0",
***************
*** 1292,1297 ****
--- 1311,1320 ----
  	dumpComment(AH, dbQry->data, NULL, "",
  				dbCatId, 0, dbDumpId);
+ 	dumpACL(AH, dbCatId, dbDumpId, "DATABASE",
+ 			fmtId(datname), datname, NULL,
+ 			dba, datacl);
+ 
  	PQclear(res);
  	destroyPQExpBuffer(dbQry);
***************
*** 6689,6695 ****
   *
   * 'objCatId' is the catalog ID of the underlying object.
   * 'objDumpId' is the dump ID of the underlying object.
!  * 'type' must be TABLE, FUNCTION, LANGUAGE, or SCHEMA.
   * 'name' is the formatted name of the object.	Must be quoted etc. already.
   * 'tag' is the tag for the archive entry (typ. unquoted name of object).
   * 'nspname' is the namespace the object is in (NULL if none).
--- 6712,6718 ----
   *
   * 'objCatId' is the catalog ID of the underlying object.
   * 'objDumpId' is the dump ID of the underlying object.
!  * 'type' must be TABLE, FUNCTION, LANGUAGE, SCHEMA, DATABASE, or TABLESPACE.
   * 'name' is the formatted name of the object.	Must be quoted etc. already.
   * 'tag' is the tag for the archive entry (typ. unquoted name of object).
   * 'nspname' is the namespace the object is in (NULL if none).
***************
*** 6705,6710 ****
--- 6728,6734 ----
  		const char *acls)
  {
  	PQExpBuffer sql;
+ 	const char *acltag;

/* Do nothing if ACL dump is not enabled */
if (dataOnly || aclsSkip)
***************
*** 6720,6732 ****
}

if (sql->len > 0)
ArchiveEntry(fout, nilCatalogId, createDumpId(),
tag, nspname,
NULL,
owner ? owner : "",
! false, "ACL", sql->data, "", NULL,
&(objDumpId), 1,
NULL, NULL);

  	destroyPQExpBuffer(sql);
  }
--- 6744,6763 ----
  	}

if (sql->len > 0)
+ {
+ if (strcmp(type, "DATABASE") == 0)
+ acltag = "DATABASE ACL";
+ else
+ acltag = "ACL";
+
ArchiveEntry(fout, nilCatalogId, createDumpId(),
tag, nspname,
NULL,
owner ? owner : "",
! false, acltag, sql->data, "", NULL,
&(objDumpId), 1,
NULL, NULL);
+ }

destroyPQExpBuffer(sql);
}