Subject: aclchk.c refactor

From: Alvaro Herrera <[email-masked]>
Date: Mon, 21 Nov 2005 12:16:52 -0300

I intend to apply later today the attached patch in order to reduce some
code duplication in aclchk.c and clean a bit the API I just introduced
in the previous patch.  This reduces aclchk.c from 2377 lines to 2206.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Index: src/backend/catalog/aclchk.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/catalog/aclchk.c,v
retrieving revision 1.121
diff -c -r1.121 aclchk.c
*** src/backend/catalog/aclchk.c	21 Nov 2005 12:49:30 -0000	1.121
--- src/backend/catalog/aclchk.c	21 Nov 2005 15:05:19 -0000
***************
*** 42,69 ****
  #include "utils/syscache.h"

! static void ExecGrant_Relation(bool is_grant, List *objects, bool all_privs,
! 				   AclMode privileges, List *grantees, bool grant_option,
! 				   DropBehavior behavior);
! static void ExecGrant_Database(bool is_grant, List *objects, bool all_privs,
! 				   AclMode privileges, List *grantees, bool grant_option,
! 				   DropBehavior behavior);
! static void ExecGrant_Function(bool is_grant, List *objects, bool all_privs,
! 				   AclMode privileges, List *grantees, bool grant_option,
! 				   DropBehavior behavior);
! static void ExecGrant_Language(bool is_grant, List *objects, bool all_privs,
! 				   AclMode privileges, List *grantees, bool grant_option,
! 				   DropBehavior behavior);
! static void ExecGrant_Namespace(bool is_grant, List *objects, bool all_privs,
! 					AclMode privileges, List *grantees, bool grant_option,
! 					DropBehavior behavior);
! static void ExecGrant_Tablespace(bool is_grant, List *objects, bool all_privs,
! 					 AclMode privileges, List *grantees, bool grant_option,
! 					 DropBehavior behavior);
! static List *objectNamesToOids(GrantObjectType objtype, List *objnames);
  
  static AclMode string_to_privilege(const char *privname);
  static const char *privilege_to_string(AclMode privilege);

  #ifdef ACLDEBUG
--- 42,62 ----
  #include "utils/syscache.h"

! static void ExecGrant_Relation(InternalGrant *grantStmt);
! static void ExecGrant_Database(InternalGrant *grantStmt);
! static void ExecGrant_Function(InternalGrant *grantStmt);
! static void ExecGrant_Language(InternalGrant *grantStmt);
! static void ExecGrant_Namespace(InternalGrant *grantStmt);
! static void ExecGrant_Tablespace(InternalGrant *grantStmt);
  
+ static List *objectNamesToOids(GrantObjectType objtype, List *objnames);
  static AclMode string_to_privilege(const char *privname);
  static const char *privilege_to_string(AclMode privilege);
+ static AclMode restrict_and_check_grant(bool is_grant, AclMode avail_goptions,
+ 										bool all_privs, AclMode privileges,
+ 										Oid objectId, Oid grantorId,
+ 										AclMode whole_mask,
+ 										AclObjectKind objkind, char *objname);

  #ifdef ACLDEBUG
***************
*** 160,172 ****
  void
  ExecuteGrantStmt(GrantStmt *stmt)
  {
! 	List	   *objects;
! 	List	   *grantees = NIL;
! 	AclMode		privileges;
  	ListCell   *cell;
! 	bool		all_privs;
! 	AclMode all_privileges = (AclMode) 0;
! 	char	*errormsg = NULL;
  
  	/*
  	 * Convert the PrivGrantee list into an Oid list.  Note that at this point
--- 153,174 ----
  void
  ExecuteGrantStmt(GrantStmt *stmt)
  {
! 	InternalGrant istmt;
  	ListCell   *cell;
! 	char	   *errormsg;
! 	AclMode		all_privileges;
! 
! 	/*
! 	 * Turn the regular GrantStmt into the InternalGrant form.
! 	 */
! 	istmt.is_grant = stmt->is_grant;
! 	istmt.objtype = stmt->objtype;
! 	istmt.objects = objectNamesToOids(stmt->objtype, stmt->objects);
! 	/* all_privs to be filled below */
! 	/* privileges to be filled below */
! 	/* grantees to be filled below */
! 	istmt.grant_option = stmt->grant_option;
! 	istmt.behavior = stmt->behavior;
  
  	/*
  	 * Convert the PrivGrantee list into an Oid list.  Note that at this point
***************
*** 180,194 ****
  		PrivGrantee *grantee = (PrivGrantee *) lfirst(cell);
  
  		if (grantee->rolname == NULL)
! 			grantees = lappend_oid(grantees, ACL_ID_PUBLIC);
  		else
! 			grantees = lappend_oid(grantees,
! 								   get_roleid_checked(grantee->rolname));
  	}
  
  	/*
! 	 * Convert stmt->privileges, a textual list, into an AclMode bitmask
! 	 * appropiate for the given object class.
  	 */
  	switch (stmt->objtype)
  	{
--- 182,196 ----
  		PrivGrantee *grantee = (PrivGrantee *) lfirst(cell);
  
  		if (grantee->rolname == NULL)
! 			istmt.grantees = lappend_oid(istmt.grantees, ACL_ID_PUBLIC);
  		else
! 			istmt.grantees =
! 				lappend_oid(istmt.grantees,
! 							get_roleid_checked(grantee->rolname));
  	}
  
  	/*
! 	 * Convert stmt->privileges, a textual list, into an AclMode bitmask.
  	 */
  	switch (stmt->objtype)
  	{
***************
*** 217,235 ****
  			errormsg = _("invalid privilege type %s for tablespace");
  			break;
  		default:
  			elog(ERROR, "unrecognized GrantStmt.objtype: %d",
  				 (int) stmt->objtype);
  	}
  
  	if (stmt->privileges == NIL)
  	{
! 		all_privs = true;
! 		privileges = all_privileges;
  	}
  	else
  	{
! 		all_privs = false;
! 		privileges = ACL_NO_RIGHTS;
  		foreach(cell, stmt->privileges)
  		{
  			char	   *privname = strVal(lfirst(cell));
--- 219,244 ----
  			errormsg = _("invalid privilege type %s for tablespace");
  			break;
  		default:
+ 			/* keep compiler quiet */
+ 			all_privileges = ACL_NO_RIGHTS;
+ 			errormsg = NULL;
  			elog(ERROR, "unrecognized GrantStmt.objtype: %d",
  				 (int) stmt->objtype);
  	}
  
  	if (stmt->privileges == NIL)
  	{
! 		istmt.all_privs = true;
! 		/*
! 		 * will be turned into ACL_ALL_RIGHTS_* by the internal routines
! 		 * depending on the object type
! 		 */
! 		istmt.privileges = ACL_NO_RIGHTS;
  	}
  	else
  	{
! 		istmt.all_privs = false;
! 		istmt.privileges = ACL_NO_RIGHTS;
  		foreach(cell, stmt->privileges)
  		{
  			char	   *privname = strVal(lfirst(cell));
***************
*** 241,301 ****
  						 errmsg(errormsg,
  								privilege_to_string(priv))));
  
! 			privileges |= priv;
  		}
  	}
  
! 	/* Turn the list of object names into an Oid list */
! 	objects = objectNamesToOids(stmt->objtype, stmt->objects);
! 
! 	ExecGrantStmt_oids(stmt->is_grant, stmt->objtype, objects, all_privs,
! 					   privileges, grantees, stmt->grant_option,
! 					   stmt->behavior);
  }
  
  /*
   * ExecGrantStmt_oids
   *
!  * "Internal" entrypoint for granting and revoking privileges.  The arguments
!  * it receives are lists of Oids or have been otherwise converted from text
!  * format to internal format.
   */
  void
! ExecGrantStmt_oids(bool is_grant, GrantObjectType objtype, List *objects,
! 				   bool all_privs, AclMode privileges, List *grantees,
! 				   bool grant_option, DropBehavior behavior)
  {
! 	switch (objtype)
  	{
  		case ACL_OBJECT_RELATION:
! 			ExecGrant_Relation(is_grant, objects, all_privs, privileges,
! 							   grantees, grant_option, behavior);
  			break;
  		case ACL_OBJECT_DATABASE:
! 			ExecGrant_Database(is_grant, objects, all_privs, privileges,
! 							   grantees, grant_option, behavior);
  			break;
  		case ACL_OBJECT_FUNCTION:
! 			ExecGrant_Function(is_grant, objects, all_privs, privileges,
! 							   grantees, grant_option, behavior);
  			break;
  		case ACL_OBJECT_LANGUAGE:
! 			ExecGrant_Language(is_grant, objects, all_privs, privileges,
! 							   grantees, grant_option, behavior);
  			break;
  		case ACL_OBJECT_NAMESPACE:
! 			ExecGrant_Namespace(is_grant, objects, all_privs,
! 								privileges, grantees, grant_option,
! 								behavior);
  			break;
  		case ACL_OBJECT_TABLESPACE:
! 			ExecGrant_Tablespace(is_grant, objects, all_privs,
! 								 privileges, grantees, grant_option,
! 								 behavior);
  			break;
  		default:
  			elog(ERROR, "unrecognized GrantStmt.objtype: %d",
! 				 (int) objtype);
  	}
  }
  
--- 250,293 ----
  						 errmsg(errormsg,
  								privilege_to_string(priv))));
  
! 			istmt.privileges |= priv;
  		}
  	}
  
! 	ExecGrantStmt_oids(&istmt);
  }
  
  /*
   * ExecGrantStmt_oids
   *
!  * "Internal" entrypoint for granting and revoking privileges.
   */
  void
! ExecGrantStmt_oids(InternalGrant *istmt)
  {
! 	switch (istmt->objtype)
  	{
  		case ACL_OBJECT_RELATION:
! 			ExecGrant_Relation(istmt);
  			break;
  		case ACL_OBJECT_DATABASE:
! 			ExecGrant_Database(istmt);
  			break;
  		case ACL_OBJECT_FUNCTION:
! 			ExecGrant_Function(istmt);
  			break;
  		case ACL_OBJECT_LANGUAGE:
! 			ExecGrant_Language(istmt);
  			break;
  		case ACL_OBJECT_NAMESPACE:
! 			ExecGrant_Namespace(istmt);
  			break;
  		case ACL_OBJECT_TABLESPACE:
! 			ExecGrant_Tablespace(istmt);
  			break;
  		default:
  			elog(ERROR, "unrecognized GrantStmt.objtype: %d",
! 				 (int) istmt->objtype);
  	}
  }
  
***************
*** 443,462 ****
  	return objects;
  }
  
  static void
! ExecGrant_Relation(bool is_grant, List *objects, bool all_privs,
! 				   AclMode privileges, List *grantees, bool grant_option,
! 				   DropBehavior behavior)
  {
  	Relation	relation;
  	ListCell   *cell;
  
! 	if (all_privs && privileges == ACL_NO_RIGHTS)
! 		privileges = ACL_ALL_RIGHTS_RELATION;
  
  	relation = heap_open(RelationRelationId, RowExclusiveLock);
  
! 	foreach (cell, objects)
  	{
  		Oid			relOid = lfirst_oid(cell);
  		Datum		aclDatum;
--- 435,513 ----
  	return objects;
  }
  
+ /*
+  * Restrict the privileges to what we can actually grant, and emit
+  * the standards-mandated warning and error messages.
+  */
+ static AclMode
+ restrict_and_check_grant(bool is_grant, AclMode avail_goptions, bool all_privs,
+ 						 AclMode privileges, Oid objectId, Oid grantorId,
+ 						 AclMode whole_mask, AclObjectKind objkind,
+ 						 char *objname)
+ {
+ 	AclMode	this_privileges;
+ 
+ 	/*
+ 	 * If we found no grant options, consider whether to issue a hard
+ 	 * error.  Per spec, having any privilege at all on the object will
+ 	 * get you by here.
+ 	 */
+ 	if (avail_goptions == ACL_NO_RIGHTS)
+ 	{
+ 		if (pg_class_aclmask(objectId,
+ 							 grantorId,
+ 							 whole_mask | ACL_GRANT_OPTION_FOR(whole_mask),
+ 							 ACLMASK_ANY) == ACL_NO_RIGHTS)
+ 			aclcheck_error(ACLCHECK_NO_PRIV, objkind, objname);
+ 	}
+ 
+ 	/*
+ 	 * Restrict the operation to what we can actually grant or revoke, and
+ 	 * issue a warning if appropriate.	(For REVOKE this isn't quite what
+ 	 * the spec says to do: the spec seems to want a warning only if no
+ 	 * privilege bits actually change in the ACL. In practice that
+ 	 * behavior seems much too noisy, as well as inconsistent with the
+ 	 * GRANT case.)
+ 	 */
+ 	this_privileges = privileges & ACL_OPTION_TO_PRIVS(avail_goptions);
+ 	if (is_grant)
+ 	{
+ 		if (this_privileges == 0)
+ 			ereport(WARNING,
+ 					(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
+ 					 errmsg("no privileges were granted")));
+ 		else if (!all_privs && this_privileges != privileges)
+ 			ereport(WARNING,
+ 					(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
+ 					 errmsg("not all privileges were granted")));
+ 	}
+ 	else
+ 	{
+ 		if (this_privileges == 0)
+ 			ereport(WARNING,
+ 					(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
+ 					 errmsg("no privileges could be revoked")));
+ 		else if (!all_privs && this_privileges != privileges)
+ 			ereport(WARNING,
+ 					(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
+ 					 errmsg("not all privileges could be revoked")));
+ 	}
+ 
+ 	return this_privileges;
+ }
+ 
  static void
! ExecGrant_Relation(InternalGrant *istmt)
  {
  	Relation	relation;
  	ListCell   *cell;
  
! 	if (istmt->all_privs && istmt->privileges == ACL_NO_RIGHTS)
! 		istmt->privileges = ACL_ALL_RIGHTS_RELATION;
  
  	relation = heap_open(RelationRelationId, RowExclusiveLock);
  
! 	foreach (cell, istmt->objects)
  	{
  		Oid			relOid = lfirst_oid(cell);
  		Datum		aclDatum;
***************
*** 511,566 ****
  			old_acl = DatumGetAclPCopy(aclDatum);
  
  		/* Determine ID to do the grant as, and available grant options */
! 		select_best_grantor(GetUserId(), privileges,
  							old_acl, ownerId,
  							&grantorId, &avail_goptions);
  
  		/*
! 		 * If we found no grant options, consider whether to issue a hard
! 		 * error.  Per spec, having any privilege at all on the object will
! 		 * get you by here.
! 		 */
! 		if (avail_goptions == ACL_NO_RIGHTS)
! 		{
! 			if (pg_class_aclmask(relOid,
! 								 grantorId,
! 								 ACL_ALL_RIGHTS_RELATION | ACL_GRANT_OPTION_FOR(ACL_ALL_RIGHTS_RELATION),
! 								 ACLMASK_ANY) == ACL_NO_RIGHTS)
! 				aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
! 							   NameStr(pg_class_tuple->relname));
! 		}
! 
! 		/*
! 		 * Restrict the operation to what we can actually grant or revoke, and
! 		 * issue a warning if appropriate.	(For REVOKE this isn't quite what
! 		 * the spec says to do: the spec seems to want a warning only if no
! 		 * privilege bits actually change in the ACL. In practice that
! 		 * behavior seems much too noisy, as well as inconsistent with the
! 		 * GRANT case.)
  		 */
! 		this_privileges = privileges & ACL_OPTION_TO_PRIVS(avail_goptions);
! 		if (is_grant)
! 		{
! 			if (this_privileges == 0)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! 						 errmsg("no privileges were granted")));
! 			else if (!all_privs && this_privileges != privileges)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! 						 errmsg("not all privileges were granted")));
! 		}
! 		else
! 		{
! 			if (this_privileges == 0)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! 						 errmsg("no privileges could be revoked")));
! 			else if (!all_privs && this_privileges != privileges)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! 						 errmsg("not all privileges could be revoked")));
! 		}
  
  		/*
  		 * Generate new ACL.
--- 562,581 ----
  			old_acl = DatumGetAclPCopy(aclDatum);
  
  		/* Determine ID to do the grant as, and available grant options */
! 		select_best_grantor(GetUserId(), istmt->privileges,
  							old_acl, ownerId,
  							&grantorId, &avail_goptions);
  
  		/*
! 		 * Restrict the privileges to what we can actually grant, and emit
! 		 * the standards-mandated warning and error messages.
  		 */
! 		this_privileges =
! 			restrict_and_check_grant(istmt->is_grant, avail_goptions,
! 									 istmt->all_privs, istmt->privileges,
! 									 relOid, grantorId,
! 									 ACL_ALL_RIGHTS_RELATION, ACL_KIND_CLASS,
! 									 NameStr(pg_class_tuple->relname));
  
  		/*
  		 * Generate new ACL.
***************
*** 570,578 ****
  		 */
  		noldmembers = aclmembers(old_acl, &oldmembers);
  
! 		new_acl = merge_acl_with_grant(old_acl, is_grant,
! 									   grant_option, behavior,
! 									   grantees, this_privileges,
  									   grantorId, ownerId);
  
  		nnewmembers = aclmembers(new_acl, &newmembers);
--- 585,593 ----
  		 */
  		noldmembers = aclmembers(old_acl, &oldmembers);
  
! 		new_acl = merge_acl_with_grant(old_acl, istmt->is_grant,
! 									   istmt->grant_option, istmt->behavior,
! 									   istmt->grantees, this_privileges,
  									   grantorId, ownerId);
  
  		nnewmembers = aclmembers(new_acl, &newmembers);
***************
*** 594,600 ****
  
  		/* Update the shared dependency ACL info */
  		updateAclDependencies(RelationRelationId, relOid,
! 							  ownerId, is_grant,
  							  noldmembers, oldmembers,
  							  nnewmembers, newmembers);
  
--- 609,615 ----
  
  		/* Update the shared dependency ACL info */
  		updateAclDependencies(RelationRelationId, relOid,
! 							  ownerId, istmt->is_grant,
  							  noldmembers, oldmembers,
  							  nnewmembers, newmembers);
  
***************
*** 610,628 ****
  }
  
  static void
! ExecGrant_Database(bool is_grant, List *objects, bool all_privs,
! 				   AclMode privileges, List *grantees, bool grant_option,
! 				   DropBehavior behavior)
  {
  	Relation	relation;
  	ListCell   *cell;
  
! 	if (all_privs && privileges == ACL_NO_RIGHTS)
! 		privileges = ACL_ALL_RIGHTS_DATABASE;
  
  	relation = heap_open(DatabaseRelationId, RowExclusiveLock);
  
! 	foreach (cell, objects)
  	{
  		Oid			datId = lfirst_oid(cell);
  		Form_pg_database pg_database_tuple;
--- 625,641 ----
  }
  
  static void
! ExecGrant_Database(InternalGrant *istmt)
  {
  	Relation	relation;
  	ListCell   *cell;
  
! 	if (istmt->all_privs && istmt->privileges == ACL_NO_RIGHTS)
! 		istmt->privileges = ACL_ALL_RIGHTS_DATABASE;
  
  	relation = heap_open(DatabaseRelationId, RowExclusiveLock);
  
! 	foreach (cell, istmt->objects)
  	{
  		Oid			datId = lfirst_oid(cell);
  		Form_pg_database pg_database_tuple;
***************
*** 674,729 ****
  			old_acl = DatumGetAclPCopy(aclDatum);
  
  		/* Determine ID to do the grant as, and available grant options */
! 		select_best_grantor(GetUserId(), privileges,
  							old_acl, ownerId,
  							&grantorId, &avail_goptions);
  
  		/*
! 		 * If we found no grant options, consider whether to issue a hard
! 		 * error.  Per spec, having any privilege at all on the object will
! 		 * get you by here.
! 		 */
! 		if (avail_goptions == ACL_NO_RIGHTS)
! 		{
! 			if (pg_database_aclmask(HeapTupleGetOid(tuple),
! 									grantorId,
! 									ACL_ALL_RIGHTS_DATABASE | ACL_GRANT_OPTION_FOR(ACL_ALL_RIGHTS_DATABASE),
! 									ACLMASK_ANY) == ACL_NO_RIGHTS)
! 				aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_DATABASE,
! 							   NameStr(pg_database_tuple->datname));
! 		}
! 
! 		/*
! 		 * Restrict the operation to what we can actually grant or revoke, and
! 		 * issue a warning if appropriate.	(For REVOKE this isn't quite what
! 		 * the spec says to do: the spec seems to want a warning only if no
! 		 * privilege bits actually change in the ACL. In practice that
! 		 * behavior seems much too noisy, as well as inconsistent with the
! 		 * GRANT case.)
  		 */
! 		this_privileges = privileges & ACL_OPTION_TO_PRIVS(avail_goptions);
! 		if (is_grant)
! 		{
! 			if (this_privileges == 0)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! 						 errmsg("no privileges were granted")));
! 			else if (!all_privs && this_privileges != privileges)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! 						 errmsg("not all privileges were granted")));
! 		}
! 		else
! 		{
! 			if (this_privileges == 0)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! 						 errmsg("no privileges could be revoked")));
! 			else if (!all_privs && this_privileges != privileges)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! 						 errmsg("not all privileges could be revoked")));
! 		}
  
  		/*
  		 * Generate new ACL.
--- 687,706 ----
  			old_acl = DatumGetAclPCopy(aclDatum);
  
  		/* Determine ID to do the grant as, and available grant options */
! 		select_best_grantor(GetUserId(), istmt->privileges,
  							old_acl, ownerId,
  							&grantorId, &avail_goptions);
  
  		/*
! 		 * Restrict the privileges to what we can actually grant, and emit
! 		 * the standards-mandated warning and error messages.
  		 */
! 		this_privileges =
! 			restrict_and_check_grant(istmt->is_grant, avail_goptions,
! 									 istmt->all_privs, istmt->privileges,
! 									 datId, grantorId, ACL_ALL_RIGHTS_DATABASE,
! 									 ACL_KIND_DATABASE,
! 									 NameStr(pg_database_tuple->datname));
  
  		/*
  		 * Generate new ACL.
***************
*** 733,741 ****
  		 */
  		noldmembers = aclmembers(old_acl, &oldmembers);
  
! 		new_acl = merge_acl_with_grant(old_acl, is_grant,
! 									   grant_option, behavior,
! 									   grantees, this_privileges,
  									   grantorId, ownerId);
  
  		nnewmembers = aclmembers(new_acl, &newmembers);
--- 710,718 ----
  		 */
  		noldmembers = aclmembers(old_acl, &oldmembers);
  
! 		new_acl = merge_acl_with_grant(old_acl, istmt->is_grant,
! 									   istmt->grant_option, istmt->behavior,
! 									   istmt->grantees, this_privileges,
  									   grantorId, ownerId);
  
  		nnewmembers = aclmembers(new_acl, &newmembers);
***************
*** 758,764 ****
  
  		/* Update the shared dependency ACL info */
  		updateAclDependencies(DatabaseRelationId, HeapTupleGetOid(tuple),
! 							  ownerId, is_grant,
  							  noldmembers, oldmembers,
  							  nnewmembers, newmembers);
  
--- 735,741 ----
  
  		/* Update the shared dependency ACL info */
  		updateAclDependencies(DatabaseRelationId, HeapTupleGetOid(tuple),
! 							  ownerId, istmt->is_grant,
  							  noldmembers, oldmembers,
  							  nnewmembers, newmembers);
  
***************
*** 774,792 ****
  }
  
  static void
! ExecGrant_Function(bool is_grant, List *objects, bool all_privs,
! 				   AclMode privileges, List *grantees, bool grant_option,
! 				   DropBehavior behavior)
  {
  	Relation	relation;
  	ListCell   *cell;
  
! 	if (all_privs && privileges == ACL_NO_RIGHTS)
! 		privileges = ACL_ALL_RIGHTS_FUNCTION;
  
  	relation = heap_open(ProcedureRelationId, RowExclusiveLock);
  
! 	foreach (cell, objects)
  	{
  		Oid			funcId = lfirst_oid(cell);
  		Form_pg_proc pg_proc_tuple;
--- 751,767 ----
  }
  
  static void
! ExecGrant_Function(InternalGrant *istmt)
  {
  	Relation	relation;
  	ListCell   *cell;
  
! 	if (istmt->all_privs && istmt->privileges == ACL_NO_RIGHTS)
! 		istmt->privileges = ACL_ALL_RIGHTS_FUNCTION;
  
  	relation = heap_open(ProcedureRelationId, RowExclusiveLock);
  
! 	foreach (cell, istmt->objects)
  	{
  		Oid			funcId = lfirst_oid(cell);
  		Form_pg_proc pg_proc_tuple;
***************
*** 829,884 ****
  			old_acl = DatumGetAclPCopy(aclDatum);
  
  		/* Determine ID to do the grant as, and available grant options */
! 		select_best_grantor(GetUserId(), privileges,
  							old_acl, ownerId,
  							&grantorId, &avail_goptions);
  
  		/*
! 		 * If we found no grant options, consider whether to issue a hard
! 		 * error.  Per spec, having any privilege at all on the object will
! 		 * get you by here.
  		 */
! 		if (avail_goptions == ACL_NO_RIGHTS)
! 		{
! 			if (pg_proc_aclmask(funcId,
! 								grantorId,
! 								ACL_ALL_RIGHTS_FUNCTION | ACL_GRANT_OPTION_FOR(ACL_ALL_RIGHTS_FUNCTION),
! 								ACLMASK_ANY) == ACL_NO_RIGHTS)
! 				aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_PROC,
! 							   NameStr(pg_proc_tuple->proname));
! 		}
! 
! 		/*
! 		 * Restrict the operation to what we can actually grant or revoke, and
! 		 * issue a warning if appropriate.	(For REVOKE this isn't quite what
! 		 * the spec says to do: the spec seems to want a warning only if no
! 		 * privilege bits actually change in the ACL. In practice that
! 		 * behavior seems much too noisy, as well as inconsistent with the
! 		 * GRANT case.)
! 		 */
! 		this_privileges = privileges & ACL_OPTION_TO_PRIVS(avail_goptions);
! 		if (is_grant)
! 		{
! 			if (this_privileges == 0)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! 						 errmsg("no privileges were granted")));
! 			else if (!all_privs && this_privileges != privileges)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! 						 errmsg("not all privileges were granted")));
! 		}
! 		else
! 		{
! 			if (this_privileges == 0)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! 						 errmsg("no privileges could be revoked")));
! 			else if (!all_privs && this_privileges != privileges)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! 						 errmsg("not all privileges could be revoked")));
! 		}
  
  		/*
  		 * Generate new ACL.
--- 804,824 ----
  			old_acl = DatumGetAclPCopy(aclDatum);
  
  		/* Determine ID to do the grant as, and available grant options */
! 		select_best_grantor(GetUserId(), istmt->privileges,
  							old_acl, ownerId,
  							&grantorId, &avail_goptions);
  
  		/*
! 		 * Restrict the privileges to what we can actually grant, and emit
! 		 * the standards-mandated warning and error messages.
  		 */
! 		this_privileges =
! 			restrict_and_check_grant(istmt->is_grant, avail_goptions,
! 									 istmt->all_privs, istmt->privileges,
! 									 funcId, grantorId,
! 									 ACL_ALL_RIGHTS_FUNCTION,
! 									 ACL_KIND_PROC,
! 									 NameStr(pg_proc_tuple->proname));
  
  		/*
  		 * Generate new ACL.
***************
*** 888,896 ****
  		 */
  		noldmembers = aclmembers(old_acl, &oldmembers);
  
! 		new_acl = merge_acl_with_grant(old_acl, is_grant,
! 									   grant_option, behavior,
! 									   grantees, this_privileges,
  									   grantorId, ownerId);
  
  		nnewmembers = aclmembers(new_acl, &newmembers);
--- 828,836 ----
  		 */
  		noldmembers = aclmembers(old_acl, &oldmembers);
  
! 		new_acl = merge_acl_with_grant(old_acl, istmt->is_grant,
! 									   istmt->grant_option, istmt->behavior,
! 									   istmt->grantees, this_privileges,
  									   grantorId, ownerId);
  
  		nnewmembers = aclmembers(new_acl, &newmembers);
***************
*** 913,919 ****
  
  		/* Update the shared dependency ACL info */
  		updateAclDependencies(ProcedureRelationId, funcId,	
! 							  ownerId, is_grant,
  							  noldmembers, oldmembers,
  							  nnewmembers, newmembers);
  
--- 853,859 ----
  
  		/* Update the shared dependency ACL info */
  		updateAclDependencies(ProcedureRelationId, funcId,	
! 							  ownerId, istmt->is_grant,
  							  noldmembers, oldmembers,
  							  nnewmembers, newmembers);
  
***************
*** 929,949 ****
  }
  
  static void
! ExecGrant_Language(bool is_grant, List *objects, bool all_privs,
! 				   AclMode privileges, List *grantees, bool grant_option,
! 				   DropBehavior behavior)
  {
  	Relation	relation;
  	ListCell   *cell;
  
! 	if (all_privs && privileges == ACL_NO_RIGHTS)
! 		privileges = ACL_ALL_RIGHTS_LANGUAGE;
  
  	relation = heap_open(LanguageRelationId, RowExclusiveLock);
  
! 	foreach (cell, objects)
  	{
! 		Oid			langid = lfirst_oid(cell);
  		Form_pg_language pg_language_tuple;
  		Datum		aclDatum;
  		bool		isNull;
--- 869,887 ----
  }
  
  static void
! ExecGrant_Language(InternalGrant *istmt)
  {
  	Relation	relation;
  	ListCell   *cell;
  
! 	if (istmt->all_privs && istmt->privileges == ACL_NO_RIGHTS)
! 		istmt->privileges = ACL_ALL_RIGHTS_LANGUAGE;
  
  	relation = heap_open(LanguageRelationId, RowExclusiveLock);
  
! 	foreach (cell, istmt->objects)
  	{
! 		Oid			langId = lfirst_oid(cell);
  		Form_pg_language pg_language_tuple;
  		Datum		aclDatum;
  		bool		isNull;
***************
*** 964,973 ****
  		Oid		   *newmembers;
  
  		tuple = SearchSysCache(LANGOID,
! 							   ObjectIdGetDatum(langid),
  							   0, 0, 0);
  		if (!HeapTupleIsValid(tuple))
! 			elog(ERROR, "cache lookup failed for language %u", langid);
  
  		pg_language_tuple = (Form_pg_language) GETSTRUCT(tuple);
  
--- 902,911 ----
  		Oid		   *newmembers;
  
  		tuple = SearchSysCache(LANGOID,
! 							   ObjectIdGetDatum(langId),
  							   0, 0, 0);
  		if (!HeapTupleIsValid(tuple))
! 			elog(ERROR, "cache lookup failed for language %u", langId);
  
  		pg_language_tuple = (Form_pg_language) GETSTRUCT(tuple);
  
***************
*** 994,1049 ****
  			old_acl = DatumGetAclPCopy(aclDatum);
  
  		/* Determine ID to do the grant as, and available grant options */
! 		select_best_grantor(GetUserId(), privileges,
  							old_acl, ownerId,
  							&grantorId, &avail_goptions);
  
  		/*
! 		 * If we found no grant options, consider whether to issue a hard
! 		 * error.  Per spec, having any privilege at all on the object will
! 		 * get you by here.
  		 */
! 		if (avail_goptions == ACL_NO_RIGHTS)
! 		{
! 			if (pg_language_aclmask(HeapTupleGetOid(tuple),
! 									grantorId,
! 									ACL_ALL_RIGHTS_LANGUAGE | ACL_GRANT_OPTION_FOR(ACL_ALL_RIGHTS_LANGUAGE),
! 									ACLMASK_ANY) == ACL_NO_RIGHTS)
! 				aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_LANGUAGE,
! 							   NameStr(pg_language_tuple->lanname));
! 		}
! 
! 		/*
! 		 * Restrict the operation to what we can actually grant or revoke, and
! 		 * issue a warning if appropriate.	(For REVOKE this isn't quite what
! 		 * the spec says to do: the spec seems to want a warning only if no
! 		 * privilege bits actually change in the ACL. In practice that
! 		 * behavior seems much too noisy, as well as inconsistent with the
! 		 * GRANT case.)
! 		 */
! 		this_privileges = privileges & ACL_OPTION_TO_PRIVS(avail_goptions);
! 		if (is_grant)
! 		{
! 			if (this_privileges == 0)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! 						 errmsg("no privileges were granted")));
! 			else if (!all_privs && this_privileges != privileges)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! 						 errmsg("not all privileges were granted")));
! 		}
! 		else
! 		{
! 			if (this_privileges == 0)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! 						 errmsg("no privileges could be revoked")));
! 			else if (!all_privs && this_privileges != privileges)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! 						 errmsg("not all privileges could be revoked")));
! 		}
  
  		/*
  		 * Generate new ACL.
--- 932,952 ----
  			old_acl = DatumGetAclPCopy(aclDatum);
  
  		/* Determine ID to do the grant as, and available grant options */
! 		select_best_grantor(GetUserId(), istmt->privileges,
  							old_acl, ownerId,
  							&grantorId, &avail_goptions);
  
  		/*
! 		 * Restrict the privileges to what we can actually grant, and emit
! 		 * the standards-mandated warning and error messages.
  		 */
! 		this_privileges =
! 			restrict_and_check_grant(istmt->is_grant, avail_goptions,
! 									 istmt->all_privs, istmt->privileges,
! 									 langId, grantorId,
! 									 ACL_ALL_RIGHTS_LANGUAGE,
! 									 ACL_KIND_LANGUAGE,
! 									 NameStr(pg_language_tuple->lanname));
  
  		/*
  		 * Generate new ACL.
***************
*** 1053,1061 ****
  		 */
  		noldmembers = aclmembers(old_acl, &oldmembers);
  
! 		new_acl = merge_acl_with_grant(old_acl, is_grant,
! 									   grant_option, behavior,
! 									   grantees, this_privileges,
  									   grantorId, ownerId);
  
  		nnewmembers = aclmembers(new_acl, &newmembers);
--- 956,964 ----
  		 */
  		noldmembers = aclmembers(old_acl, &oldmembers);
  
! 		new_acl = merge_acl_with_grant(old_acl, istmt->is_grant,
! 									   istmt->grant_option, istmt->behavior,
! 									   istmt->grantees, this_privileges,
  									   grantorId, ownerId);
  
  		nnewmembers = aclmembers(new_acl, &newmembers);
***************
*** 1078,1084 ****
  
  		/* Update the shared dependency ACL info */
  		updateAclDependencies(LanguageRelationId, HeapTupleGetOid(tuple),
! 							  ownerId, is_grant,
  							  noldmembers, oldmembers,
  							  nnewmembers, newmembers);
  
--- 981,987 ----
  
  		/* Update the shared dependency ACL info */
  		updateAclDependencies(LanguageRelationId, HeapTupleGetOid(tuple),
! 							  ownerId, istmt->is_grant,
  							  noldmembers, oldmembers,
  							  nnewmembers, newmembers);
  
***************
*** 1094,1112 ****
  }
  
  static void
! ExecGrant_Namespace(bool is_grant, List *objects, bool all_privs,
!  					AclMode privileges, List *grantees, bool grant_option,
!  					DropBehavior behavior)
  {
  	Relation	relation;
  	ListCell   *cell;
  
! 	if (all_privs && privileges == ACL_NO_RIGHTS)
! 		privileges = ACL_ALL_RIGHTS_NAMESPACE;
  
  	relation = heap_open(NamespaceRelationId, RowExclusiveLock);
  
! 	foreach(cell, objects)
  	{
  		Oid			nspid = lfirst_oid(cell);
  		Form_pg_namespace pg_namespace_tuple;
--- 997,1013 ----
  }
  
  static void
! ExecGrant_Namespace(InternalGrant *istmt)
  {
  	Relation	relation;
  	ListCell   *cell;
  
! 	if (istmt->all_privs && istmt->privileges == ACL_NO_RIGHTS)
! 		istmt->privileges = ACL_ALL_RIGHTS_NAMESPACE;
  
  	relation = heap_open(NamespaceRelationId, RowExclusiveLock);
  
! 	foreach(cell, istmt->objects)
  	{
  		Oid			nspid = lfirst_oid(cell);
  		Form_pg_namespace pg_namespace_tuple;
***************
*** 1150,1205 ****
  			old_acl = DatumGetAclPCopy(aclDatum);
  
  		/* Determine ID to do the grant as, and available grant options */
! 		select_best_grantor(GetUserId(), privileges,
  							old_acl, ownerId,
  							&grantorId, &avail_goptions);
  
  		/*
! 		 * If we found no grant options, consider whether to issue a hard
! 		 * error.  Per spec, having any privilege at all on the object will
! 		 * get you by here.
  		 */
! 		if (avail_goptions == ACL_NO_RIGHTS)
! 		{
! 			if (pg_namespace_aclmask(HeapTupleGetOid(tuple),
! 									 grantorId,
! 									 ACL_ALL_RIGHTS_NAMESPACE | ACL_GRANT_OPTION_FOR(ACL_ALL_RIGHTS_NAMESPACE),
! 									 ACLMASK_ANY) == ACL_NO_RIGHTS)
! 				aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_NAMESPACE,
! 							   NameStr(pg_namespace_tuple->nspname));
! 		}
! 
! 		/*
! 		 * Restrict the operation to what we can actually grant or revoke, and
! 		 * issue a warning if appropriate.	(For REVOKE this isn't quite what
! 		 * the spec says to do: the spec seems to want a warning only if no
! 		 * privilege bits actually change in the ACL. In practice that
! 		 * behavior seems much too noisy, as well as inconsistent with the
! 		 * GRANT case.)
! 		 */
! 		this_privileges = privileges & ACL_OPTION_TO_PRIVS(avail_goptions);
! 		if (is_grant)
! 		{
! 			if (this_privileges == 0)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! 						 errmsg("no privileges were granted")));
! 			else if (!all_privs && this_privileges != privileges)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! 						 errmsg("not all privileges were granted")));
! 		}
! 		else
! 		{
! 			if (this_privileges == 0)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! 						 errmsg("no privileges could be revoked")));
! 			else if (!all_privs && this_privileges != privileges)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! 						 errmsg("not all privileges could be revoked")));
! 		}
  
  		/*
  		 * Generate new ACL.
--- 1051,1071 ----
  			old_acl = DatumGetAclPCopy(aclDatum);
  
  		/* Determine ID to do the grant as, and available grant options */
! 		select_best_grantor(GetUserId(), istmt->privileges,
  							old_acl, ownerId,
  							&grantorId, &avail_goptions);
  
  		/*
! 		 * Restrict the privileges to what we can actually grant, and emit
! 		 * the standards-mandated warning and error messages.
  		 */
! 		this_privileges =
! 			restrict_and_check_grant(istmt->is_grant, avail_goptions,
! 									 istmt->all_privs, istmt->privileges,
! 									 nspid, grantorId,
! 									 ACL_ALL_RIGHTS_NAMESPACE,
! 									 ACL_KIND_NAMESPACE,
! 									 NameStr(pg_namespace_tuple->nspname));
  
  		/*
  		 * Generate new ACL.
***************
*** 1209,1217 ****
  		 */
  		noldmembers = aclmembers(old_acl, &oldmembers);
  
! 		new_acl = merge_acl_with_grant(old_acl, is_grant,
! 									   grant_option, behavior,
! 									   grantees, this_privileges,
  									   grantorId, ownerId);
  
  		nnewmembers = aclmembers(new_acl, &newmembers);
--- 1075,1083 ----
  		 */
  		noldmembers = aclmembers(old_acl, &oldmembers);
  
! 		new_acl = merge_acl_with_grant(old_acl, istmt->is_grant,
! 									   istmt->grant_option, istmt->behavior,
! 									   istmt->grantees, this_privileges,
  									   grantorId, ownerId);
  
  		nnewmembers = aclmembers(new_acl, &newmembers);
***************
*** 1234,1240 ****
  
  		/* Update the shared dependency ACL info */
  		updateAclDependencies(NamespaceRelationId, HeapTupleGetOid(tuple),
! 							  ownerId, is_grant,
  							  noldmembers, oldmembers,
  							  nnewmembers, newmembers);
  
--- 1100,1106 ----
  
  		/* Update the shared dependency ACL info */
  		updateAclDependencies(NamespaceRelationId, HeapTupleGetOid(tuple),
! 							  ownerId, istmt->is_grant,
  							  noldmembers, oldmembers,
  							  nnewmembers, newmembers);
  
***************
*** 1250,1268 ****
  }
  
  static void
! ExecGrant_Tablespace(bool is_grant, List *objects, bool all_privs,
! 					 AclMode privileges, List *grantees, bool grant_option,
! 					 DropBehavior behavior)
  {
  	Relation	relation;
  	ListCell   *cell;
  
! 	if (all_privs && privileges == ACL_NO_RIGHTS)
! 		privileges = ACL_ALL_RIGHTS_TABLESPACE;
  
  	relation = heap_open(TableSpaceRelationId, RowExclusiveLock);
  
! 	foreach(cell, objects)
  	{
  		Oid			tblId = lfirst_oid(cell);
  		Form_pg_tablespace pg_tablespace_tuple;
--- 1116,1132 ----
  }
  
  static void
! ExecGrant_Tablespace(InternalGrant *istmt)
  {
  	Relation	relation;
  	ListCell   *cell;
  
! 	if (istmt->all_privs && istmt->privileges == ACL_NO_RIGHTS)
! 		istmt->privileges = ACL_ALL_RIGHTS_TABLESPACE;
  
  	relation = heap_open(TableSpaceRelationId, RowExclusiveLock);
  
! 	foreach(cell, istmt->objects)
  	{
  		Oid			tblId = lfirst_oid(cell);
  		Form_pg_tablespace pg_tablespace_tuple;
***************
*** 1312,1367 ****
  			old_acl = DatumGetAclPCopy(aclDatum);
  
  		/* Determine ID to do the grant as, and available grant options */
! 		select_best_grantor(GetUserId(), privileges,
  							old_acl, ownerId,
  							&grantorId, &avail_goptions);
  
  		/*
! 		 * If we found no grant options, consider whether to issue a hard
! 		 * error.  Per spec, having any privilege at all on the object will
! 		 * get you by here.
! 		 */
! 		if (avail_goptions == ACL_NO_RIGHTS)
! 		{
! 			if (pg_tablespace_aclmask(HeapTupleGetOid(tuple),
! 									  grantorId,
! 									  ACL_ALL_RIGHTS_TABLESPACE | ACL_GRANT_OPTION_FOR(ACL_ALL_RIGHTS_TABLESPACE),
! 									  ACLMASK_ANY) == ACL_NO_RIGHTS)
! 				aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_TABLESPACE,
! 							   NameStr(pg_tablespace_tuple->spcname));
! 		}
! 
! 		/*
! 		 * Restrict the operation to what we can actually grant or revoke, and
! 		 * issue a warning if appropriate.	(For REVOKE this isn't quite what
! 		 * the spec says to do: the spec seems to want a warning only if no
! 		 * privilege bits actually change in the ACL. In practice that
! 		 * behavior seems much too noisy, as well as inconsistent with the
! 		 * GRANT case.)
  		 */
! 		this_privileges = privileges & ACL_OPTION_TO_PRIVS(avail_goptions);
! 		if (is_grant)
! 		{
! 			if (this_privileges == 0)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! 						 errmsg("no privileges were granted")));
! 			else if (!all_privs && this_privileges != privileges)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED),
! 						 errmsg("not all privileges were granted")));
! 		}
! 		else
! 		{
! 			if (this_privileges == 0)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! 						 errmsg("no privileges could be revoked")));
! 			else if (!all_privs && this_privileges != privileges)
! 				ereport(WARNING,
! 						(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
! 						 errmsg("not all privileges could be revoked")));
! 		}
  
  		/*
  		 * Generate new ACL.
--- 1176,1196 ----
  			old_acl = DatumGetAclPCopy(aclDatum);
  
  		/* Determine ID to do the grant as, and available grant options */
! 		select_best_grantor(GetUserId(), istmt->privileges,
  							old_acl, ownerId,
  							&grantorId, &avail_goptions);
  
  		/*
! 		 * Restrict the privileges to what we can actually grant, and emit
! 		 * the standards-mandated warning and error messages.
  		 */
! 		this_privileges =
! 			restrict_and_check_grant(istmt->is_grant, avail_goptions,
! 									 istmt->all_privs, istmt->privileges,
! 									 tblId, grantorId,
! 									 ACL_ALL_RIGHTS_TABLESPACE,
! 									 ACL_KIND_TABLESPACE,
! 									 NameStr(pg_tablespace_tuple->spcname));
  
  		/*
  		 * Generate new ACL.
***************
*** 1371,1379 ****
  		 */
  		noldmembers = aclmembers(old_acl, &oldmembers);
  
! 		new_acl = merge_acl_with_grant(old_acl, is_grant,
! 									   grant_option, behavior,
! 									   grantees, this_privileges,
  									   grantorId, ownerId);
  
  		nnewmembers = aclmembers(new_acl, &newmembers);
--- 1200,1208 ----
  		 */
  		noldmembers = aclmembers(old_acl, &oldmembers);
  
! 		new_acl = merge_acl_with_grant(old_acl, istmt->is_grant,
! 									   istmt->grant_option, istmt->behavior,
! 									   istmt->grantees, this_privileges,
  									   grantorId, ownerId);
  
  		nnewmembers = aclmembers(new_acl, &newmembers);
***************
*** 1396,1402 ****
  
  		/* Update the shared dependency ACL info */
  		updateAclDependencies(TableSpaceRelationId, tblId,
! 							  ownerId, is_grant,
  							  noldmembers, oldmembers,
  							  nnewmembers, newmembers);
  
--- 1225,1231 ----
  
  		/* Update the shared dependency ACL info */
  		updateAclDependencies(TableSpaceRelationId, tblId,
! 							  ownerId, istmt->is_grant,
  							  noldmembers, oldmembers,
  							  nnewmembers, newmembers);
  
Index: src/backend/catalog/pg_shdepend.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/catalog/pg_shdepend.c,v
retrieving revision 1.4
diff -c -r1.4 pg_shdepend.c
*** src/backend/catalog/pg_shdepend.c	21 Nov 2005 12:49:30 -0000	1.4
--- src/backend/catalog/pg_shdepend.c	21 Nov 2005 15:08:16 -0000
***************
*** 1122,1127 ****
--- 1122,1128 ----
  			{
  				ObjectAddress	obj;
  				GrantObjectType	objtype;
+ 				InternalGrant	istmt;
  
  				/* Shouldn't happen */
  				case SHARED_DEPENDENCY_PIN:
***************
*** 1132,1153 ****
  					switch (sdepForm->classid)
  					{
  						case RelationRelationId:
! 							objtype = ACL_OBJECT_RELATION;
  							break;
  						case DatabaseRelationId:
! 							objtype = ACL_OBJECT_DATABASE;
  							break;
  						case ProcedureRelationId:
! 							objtype = ACL_OBJECT_FUNCTION;
  							break;
  						case LanguageRelationId:
! 							objtype = ACL_OBJECT_LANGUAGE;
  							break;
  						case NamespaceRelationId:
! 							objtype = ACL_OBJECT_NAMESPACE;
  							break;
  						case TableSpaceRelationId:
! 							objtype = ACL_OBJECT_TABLESPACE;
  							break;
  						default:
  							elog(ERROR, "unexpected object type %d",
--- 1133,1154 ----
  					switch (sdepForm->classid)
  					{
  						case RelationRelationId:
! 							istmt.objtype = ACL_OBJECT_RELATION;
  							break;
  						case DatabaseRelationId:
! 							istmt.objtype = ACL_OBJECT_DATABASE;
  							break;
  						case ProcedureRelationId:
! 							istmt.objtype = ACL_OBJECT_FUNCTION;
  							break;
  						case LanguageRelationId:
! 							istmt.objtype = ACL_OBJECT_LANGUAGE;
  							break;
  						case NamespaceRelationId:
! 							istmt.objtype = ACL_OBJECT_NAMESPACE;
  							break;
  						case TableSpaceRelationId:
! 							istmt.objtype = ACL_OBJECT_TABLESPACE;
  							break;
  						default:
  							elog(ERROR, "unexpected object type %d",
***************
*** 1156,1166 ****
  							objtype = (GrantObjectType) 0;
  							break;
  					}
  
! 					ExecGrantStmt_oids(false, objtype,
! 									   list_make1_oid(sdepForm->objid), true,
! 									   ACL_NO_RIGHTS, list_make1_oid(roleid),
! 									   false, DROP_CASCADE);
  					break;
  				case SHARED_DEPENDENCY_OWNER:
  					/*
--- 1157,1171 ----
  							objtype = (GrantObjectType) 0;
  							break;
  					}
+ 					istmt.is_grant = false;
+ 					istmt.objects = list_make1_oid(sdepForm->objid);
+ 					istmt.all_privs = true;
+ 					istmt.privileges = ACL_NO_RIGHTS;
+ 					istmt.grantees = list_make1_oid(roleid);
+ 					istmt.grant_option = false;
+ 					istmt.behavior = DROP_CASCADE;
  
! 					ExecGrantStmt_oids(&istmt);
  					break;
  				case SHARED_DEPENDENCY_OWNER:
  					/*
Index: src/include/utils/acl.h
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/include/utils/acl.h,v
retrieving revision 1.89
diff -c -r1.89 acl.h
*** src/include/utils/acl.h	21 Nov 2005 12:49:33 -0000	1.89
--- src/include/utils/acl.h	21 Nov 2005 14:32:59 -0000
***************
*** 182,187 ****
--- 182,207 ----
  } AclObjectKind;
  
  /*
+  * The information about one Grant/Revoke statement, in internal format: object
+  * and grantees names have been turned into Oids, the privilege list is an
+  * AclMode bitmask.  If 'privileges' is ACL_NO_RIGHTS (the 0 value) and
+  * all_privs is true, it will be internally turned into the right kind of
+  * ACL_ALL_RIGHTS_*, depending on the object type (NB - this will modify the
+  * InternalGrant struct!)
+  */
+ typedef struct
+ {
+ 	bool    is_grant;
+ 	GrantObjectType objtype;
+ 	List   *objects;
+ 	bool    all_privs;
+ 	AclMode privileges;
+ 	List   *grantees;
+ 	bool    grant_option;
+ 	DropBehavior behavior;
+ } InternalGrant;
+ 
+ /*
   * routines used internally
   */
  extern Acl *acldefault(GrantObjectType objtype, Oid ownerId);
***************
*** 221,229 ****
   * prototypes for functions in aclchk.c
   */
  extern void ExecuteGrantStmt(GrantStmt *stmt);
! extern void ExecGrantStmt_oids(bool is_grant, GrantObjectType objtype,
! 				   List *objects, bool all_privs, AclMode privileges,
! 				   List *grantees, bool grant_option, DropBehavior behavior);
  
  extern AclMode pg_class_aclmask(Oid table_oid, Oid roleid,
  				 AclMode mask, AclMaskHow how);
--- 241,247 ----
   * prototypes for functions in aclchk.c
   */
  extern void ExecuteGrantStmt(GrantStmt *stmt);
! extern void ExecGrantStmt_oids(InternalGrant *istmt);
  
  extern AclMode pg_class_aclmask(Oid table_oid, Oid roleid,
  				 AclMode mask, AclMaskHow how);

---

From: Alvaro Herrera <[email-masked]>
Date: Mon, 21 Nov 2005 13:34:41 -0300

> in the previous patch.  This reduces aclchk.c from 2377 lines to 2206.

Of course, it would be much better if the proposed patch actually
worked, which it doesn't because there's a function call that I didn't
generalize: the code is calling pg_class_aclmask(), which of course only
works for relations.

Now I noticed that there are multiple functions pg_class_aclmask,
pg_database_aclmask, pg_language_aclmask, etc.  Is there any objection
to making the exported routine expose the object type as an AclKind
parameter instead of having one function for each object type?
So instead of having 

extern AclMode pg_class_aclmask(Oid table_oid, Oid roleid,
                 AclMode mask, AclMaskHow how);
extern AclMode pg_database_aclmask(Oid db_oid, Oid roleid,
                    AclMode mask, AclMaskHow how);
extern AclMode pg_proc_aclmask(Oid proc_oid, Oid roleid,
                AclMode mask, AclMaskHow how);
extern AclMode pg_language_aclmask(Oid lang_oid, Oid roleid,
                    AclMode mask, AclMaskHow how);
extern AclMode pg_namespace_aclmask(Oid nsp_oid, Oid roleid,
                     AclMode mask, AclMaskHow how);
extern AclMode pg_tablespace_aclmask(Oid spc_oid, Oid roleid,
                      AclMode mask, AclMaskHow how);

We would have 

extern AclMode pg_aclmask(AclKind objkind, Oid obj_oid, Oid roleid,
				AclMode mask, AclMaskHow how);

And this would call the appropiate static function.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

---

From: Tom Lane <[email-masked]>
Date: Mon, 21 Nov 2005 11:57:40 -0500

> parameter instead of having one function for each object type?

How about "in addition to" instead of "instead"?  I see no reason to
impose extra notation and a level of indirection on the places that know
perfectly well which object type they are dealing with.

			regards, tom lane

---

From: Alvaro Herrera <[email-masked]>
Date: Thu, 1 Dec 2005 10:36:29 -0300

> in the previous patch.  This reduces aclchk.c from 2377 lines to 2206.

I applied this patch yesterday, but I did not receive the commit
message.  However I do see it in the online archives.

Is there something fishy going on here, or just the mail was lost on its
way here?

---

From: Tom Lane <[email-masked]>
Date: Thu, 01 Dec 2005 10:28:34 -0500

> Is there something fishy going on here, or just the mail was lost on its

[ checks mail logs... ]  Hm, I never got a copy of this either:
http://archives.postgresql.org/pgsql-committers/2005-11/msg00609.php

There's definitely something flaky about pgsql-committers since the
server move.  I still haven't seen anything about Michael's ecpg commits
of yesterday, either.

			regards, tom lane

---

From: "Marc G. Fournier" <[email-masked]>
Date: Thu, 1 Dec 2005 15:09:07 -0400 (AST)

Everything should be good now ... had to change Michael and Niel over, and 
just approved through their commits also ...

> server move.  I still haven't seen anything about Michael's ecpg commits

----
Marc G. Fournier           Hub.Org Networking Services (http://www.hub.org)
Email: [email-masked]           Yahoo!: yscrappy              ICQ: 7615664