Align GRANT/REVOKE behavior more closely with the SQL spec, per discussion

of bug report #1150.  Also, arrange that the object owner's irrevocable
grant-option permissions are handled implicitly by the system rather than
being listed in the ACL as self-granted rights (which was wrong anyway).
I did not take the further step of showing these permissions in an
explicit 'granted by _SYSTEM' ACL entry, as that seemed more likely to
bollix up existing clients than to do anything really useful.  It's still
a possible future direction, though.

5 files changed, 692 insertions, 311 deletions

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 3aaabf1b990..de74a422b78 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/aclchk.c,v 1.102 2004/05/28 16:37:11 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/aclchk.c,v 1.103 2004/06/01 21:49:22 tgl Exp $
  *
  * NOTES
  *	  See acl.h.
@@ -48,9 +48,6 @@ static void ExecuteGrantStmt_Namespace(GrantStmt *stmt);
 
 static const char *privilege_to_string(AclMode privilege);
 
-static AclMode aclmask(Acl *acl, AclId userid,
-					   AclMode mask, AclMaskHow how);
-
 
 #ifdef ACLDEBUG
 static
@@ -126,15 +123,12 @@ merge_acl_with_grant(Acl *old_acl, bool is_grant,
 		AclItem		aclitem;
 		uint32		idtype;
 		Acl		   *newer_acl;
-		bool		grantee_is_owner = false;
 
 		if (grantee->username)
 		{
 			aclitem.ai_grantee = get_usesysid(grantee->username);
 
 			idtype = ACL_IDTYPE_UID;
-
-			grantee_is_owner = (aclitem.ai_grantee == owner_uid);
 		}
 		else if (grantee->groupname)
 		{
@@ -161,19 +155,21 @@ merge_acl_with_grant(Acl *old_acl, bool is_grant,
 					(errcode(ERRCODE_INVALID_GRANT_OPERATION),
 					 errmsg("grant options can only be granted to individual users")));
 
-		if (!is_grant && grant_option && grantee_is_owner)
-			ereport(ERROR,
-					(errcode(ERRCODE_INVALID_GRANT_OPERATION),
-					 errmsg("cannot revoke grant options from owner")));
-
 		aclitem.ai_grantor = grantor_uid;
 
+		/*
+		 * The asymmetry in the conditions here comes from the spec.  In
+		 * GRANT, the grant_option flag signals WITH GRANT OPTION, which means
+		 * to grant both the basic privilege and its grant option.  But in
+		 * REVOKE, plain revoke revokes both the basic privilege and its
+		 * grant option, while REVOKE GRANT OPTION revokes only the option.
+		 */
 		ACLITEM_SET_PRIVS_IDTYPE(aclitem,
 								 (is_grant || !grant_option) ? privileges : ACL_NO_RIGHTS,
-								 (grant_option || (!is_grant && !grantee_is_owner)) ? privileges : ACL_NO_RIGHTS,
+								 (!is_grant || grant_option) ? privileges : ACL_NO_RIGHTS,
 								 idtype);
 
-		newer_acl = aclinsert3(new_acl, &aclitem, modechg, behavior);
+		newer_acl = aclupdate(new_acl, &aclitem, modechg, owner_uid, behavior);
 
 		/* avoid memory leak when there are many grantees */
 		pfree(new_acl);
@@ -221,12 +217,17 @@ static void
 ExecuteGrantStmt_Relation(GrantStmt *stmt)
 {
 	AclMode		privileges;
+	bool		all_privs;
 	ListCell   *i;
 
 	if (linitial_int(stmt->privileges) == ACL_ALL_RIGHTS)
+	{
+		all_privs = true;
 		privileges = ACL_ALL_RIGHTS_RELATION;
+	}
 	else
 	{
+		all_privs = false;
 		privileges = ACL_NO_RIGHTS;
 		foreach(i, stmt->privileges)
 		{
@@ -250,6 +251,8 @@ ExecuteGrantStmt_Relation(GrantStmt *stmt)
 		Form_pg_class pg_class_tuple;
 		Datum		aclDatum;
 		bool		isNull;
+		AclMode		my_goptions;
+		AclMode		this_privileges;
 		Acl		   *old_acl;
 		Acl		   *new_acl;
 		AclId		grantorId;
@@ -269,15 +272,6 @@ ExecuteGrantStmt_Relation(GrantStmt *stmt)
 			elog(ERROR, "cache lookup failed for relation %u", relOid);
 		pg_class_tuple = (Form_pg_class) GETSTRUCT(tuple);
 
-		ownerId = pg_class_tuple->relowner;
-		grantorId = select_grantor(ownerId);
-
-		if (stmt->is_grant
-			&& !pg_class_ownercheck(relOid, GetUserId())
-			&& pg_class_aclcheck(relOid, GetUserId(),
-								 ACL_GRANT_OPTION_FOR(privileges)) != ACLCHECK_OK)
-			aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, relvar->relname);
-
 		/* Not sensible to grant on an index */
 		if (pg_class_tuple->relkind == RELKIND_INDEX)
 			ereport(ERROR,
@@ -285,6 +279,69 @@ ExecuteGrantStmt_Relation(GrantStmt *stmt)
 					 errmsg("\"%s\" is an index",
 							relvar->relname)));
 
+		/* Composite types aren't tables either */
+		if (pg_class_tuple->relkind == RELKIND_COMPOSITE_TYPE)
+			ereport(ERROR,
+					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+					 errmsg("\"%s\" is a composite type",
+							relvar->relname)));
+
+		ownerId = pg_class_tuple->relowner;
+		grantorId = select_grantor(ownerId);
+
+		/*
+		 * Must be owner or have some privilege on the object (per spec,
+		 * any privilege will get you by here).  The owner is always
+		 * treated as having all grant options.
+		 */
+		if (pg_class_ownercheck(relOid, GetUserId()))
+			my_goptions = ACL_ALL_RIGHTS_RELATION;
+		else
+		{
+			AclMode		my_rights;
+
+			my_rights = pg_class_aclmask(relOid,
+										 GetUserId(),
+										 ACL_ALL_RIGHTS_RELATION | ACL_GRANT_OPTION_FOR(ACL_ALL_RIGHTS_RELATION),
+										 ACLMASK_ALL);
+			if (my_rights == ACL_NO_RIGHTS)
+				aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS,
+							   relvar->relname);
+			my_goptions = ACL_OPTION_TO_PRIVS(my_rights);
+		}
+
+		/*
+		 * 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 & my_goptions;
+		if (stmt->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")));
+		}
+
 		/*
 		 * If there's no ACL, substitute the proper default.
 		 */
@@ -298,7 +355,7 @@ ExecuteGrantStmt_Relation(GrantStmt *stmt)
 
 		new_acl = merge_acl_with_grant(old_acl, stmt->is_grant,
 									   stmt->grant_option, stmt->behavior,
-									   stmt->grantees, privileges,
+									   stmt->grantees, this_privileges,
 									   grantorId, ownerId);
 
 		/* finished building new ACL value, now insert it */
@@ -328,12 +385,17 @@ static void
 ExecuteGrantStmt_Database(GrantStmt *stmt)
 {
 	AclMode		privileges;
+	bool		all_privs;
 	ListCell   *i;
 
 	if (linitial_int(stmt->privileges) == ACL_ALL_RIGHTS)
+	{
+		all_privs = true;
 		privileges = ACL_ALL_RIGHTS_DATABASE;
+	}
 	else
 	{
+		all_privs = false;
 		privileges = ACL_NO_RIGHTS;
 		foreach(i, stmt->privileges)
 		{
@@ -358,6 +420,8 @@ ExecuteGrantStmt_Database(GrantStmt *stmt)
 		Form_pg_database pg_database_tuple;
 		Datum		aclDatum;
 		bool		isNull;
+		AclMode		my_goptions;
+		AclMode		this_privileges;
 		Acl		   *old_acl;
 		Acl		   *new_acl;
 		AclId		grantorId;
@@ -383,12 +447,58 @@ ExecuteGrantStmt_Database(GrantStmt *stmt)
 		ownerId = pg_database_tuple->datdba;
 		grantorId = select_grantor(ownerId);
 
-		if (stmt->is_grant
-			&& !pg_database_ownercheck(HeapTupleGetOid(tuple), GetUserId())
-			&& pg_database_aclcheck(HeapTupleGetOid(tuple), GetUserId(),
-									ACL_GRANT_OPTION_FOR(privileges)) != ACLCHECK_OK)
-			aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_DATABASE,
-						   NameStr(pg_database_tuple->datname));
+		/*
+		 * Must be owner or have some privilege on the object (per spec,
+		 * any privilege will get you by here).  The owner is always
+		 * treated as having all grant options.
+		 */
+		if (pg_database_ownercheck(HeapTupleGetOid(tuple), GetUserId()))
+			my_goptions = ACL_ALL_RIGHTS_DATABASE;
+		else
+		{
+			AclMode		my_rights;
+
+			my_rights = pg_database_aclmask(HeapTupleGetOid(tuple),
+											GetUserId(),
+											ACL_ALL_RIGHTS_DATABASE | ACL_GRANT_OPTION_FOR(ACL_ALL_RIGHTS_DATABASE),
+											ACLMASK_ALL);
+			if (my_rights == ACL_NO_RIGHTS)
+				aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_DATABASE,
+							   NameStr(pg_database_tuple->datname));
+			my_goptions = ACL_OPTION_TO_PRIVS(my_rights);
+		}
+
+		/*
+		 * 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 & my_goptions;
+		if (stmt->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")));
+		}
 
 		/*
 		 * If there's no ACL, substitute the proper default.
@@ -403,7 +513,7 @@ ExecuteGrantStmt_Database(GrantStmt *stmt)
 
 		new_acl = merge_acl_with_grant(old_acl, stmt->is_grant,
 									   stmt->grant_option, stmt->behavior,
-									   stmt->grantees, privileges,
+									   stmt->grantees, this_privileges,
 									   grantorId, ownerId);
 
 		/* finished building new ACL value, now insert it */
@@ -433,12 +543,17 @@ static void
 ExecuteGrantStmt_Function(GrantStmt *stmt)
 {
 	AclMode		privileges;
+	bool		all_privs;
 	ListCell   *i;
 
 	if (linitial_int(stmt->privileges) == ACL_ALL_RIGHTS)
+	{
+		all_privs = true;
 		privileges = ACL_ALL_RIGHTS_FUNCTION;
+	}
 	else
 	{
+		all_privs = false;
 		privileges = ACL_NO_RIGHTS;
 		foreach(i, stmt->privileges)
 		{
@@ -462,6 +577,8 @@ ExecuteGrantStmt_Function(GrantStmt *stmt)
 		Form_pg_proc pg_proc_tuple;
 		Datum		aclDatum;
 		bool		isNull;
+		AclMode		my_goptions;
+		AclMode		this_privileges;
 		Acl		   *old_acl;
 		Acl		   *new_acl;
 		AclId		grantorId;
@@ -484,12 +601,58 @@ ExecuteGrantStmt_Function(GrantStmt *stmt)
 		ownerId = pg_proc_tuple->proowner;
 		grantorId = select_grantor(ownerId);
 
-		if (stmt->is_grant
-			&& !pg_proc_ownercheck(oid, GetUserId())
-			&& pg_proc_aclcheck(oid, GetUserId(),
-								ACL_GRANT_OPTION_FOR(privileges)) != ACLCHECK_OK)
-			aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_PROC,
-						   NameStr(pg_proc_tuple->proname));
+		/*
+		 * Must be owner or have some privilege on the object (per spec,
+		 * any privilege will get you by here).  The owner is always
+		 * treated as having all grant options.
+		 */
+		if (pg_proc_ownercheck(oid, GetUserId()))
+			my_goptions = ACL_ALL_RIGHTS_FUNCTION;
+		else
+		{
+			AclMode		my_rights;
+
+			my_rights = pg_proc_aclmask(oid,
+										GetUserId(),
+										ACL_ALL_RIGHTS_FUNCTION | ACL_GRANT_OPTION_FOR(ACL_ALL_RIGHTS_FUNCTION),
+										ACLMASK_ALL);
+			if (my_rights == ACL_NO_RIGHTS)
+				aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_PROC,
+							   NameStr(pg_proc_tuple->proname));
+			my_goptions = ACL_OPTION_TO_PRIVS(my_rights);
+		}
+
+		/*
+		 * 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 & my_goptions;
+		if (stmt->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")));
+		}
 
 		/*
 		 * If there's no ACL, substitute the proper default.
@@ -504,7 +667,7 @@ ExecuteGrantStmt_Function(GrantStmt *stmt)
 
 		new_acl = merge_acl_with_grant(old_acl, stmt->is_grant,
 									   stmt->grant_option, stmt->behavior,
-									   stmt->grantees, privileges,
+									   stmt->grantees, this_privileges,
 									   grantorId, ownerId);
 
 		/* finished building new ACL value, now insert it */
@@ -534,12 +697,17 @@ static void
 ExecuteGrantStmt_Language(GrantStmt *stmt)
 {
 	AclMode		privileges;
+	bool		all_privs;
 	ListCell   *i;
 
 	if (linitial_int(stmt->privileges) == ACL_ALL_RIGHTS)
+	{
+		all_privs = true;
 		privileges = ACL_ALL_RIGHTS_LANGUAGE;
+	}
 	else
 	{
+		all_privs = false;
 		privileges = ACL_NO_RIGHTS;
 		foreach(i, stmt->privileges)
 		{
@@ -562,6 +730,8 @@ ExecuteGrantStmt_Language(GrantStmt *stmt)
 		Form_pg_language pg_language_tuple;
 		Datum		aclDatum;
 		bool		isNull;
+		AclMode		my_goptions;
+		AclMode		this_privileges;
 		Acl		   *old_acl;
 		Acl		   *new_acl;
 		AclId		grantorId;
@@ -581,6 +751,11 @@ ExecuteGrantStmt_Language(GrantStmt *stmt)
 					 errmsg("language \"%s\" does not exist", langname)));
 		pg_language_tuple = (Form_pg_language) GETSTRUCT(tuple);
 
+		if (!pg_language_tuple->lanpltrusted)
+			ereport(ERROR,
+					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+					 errmsg("language \"%s\" is not trusted", langname)));
+
 		/*
 		 * Note: for now, languages are treated as owned by the bootstrap
 		 * user.  We should add an owner column to pg_language instead.
@@ -588,17 +763,58 @@ ExecuteGrantStmt_Language(GrantStmt *stmt)
 		ownerId = BOOTSTRAP_USESYSID;
 		grantorId = select_grantor(ownerId);
 
-		if (stmt->is_grant
-			&& !superuser()		/* XXX no ownercheck() available */
-			&& pg_language_aclcheck(HeapTupleGetOid(tuple), GetUserId(),
-									ACL_GRANT_OPTION_FOR(privileges)) != ACLCHECK_OK)
-			aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_LANGUAGE,
-						   NameStr(pg_language_tuple->lanname));
+		/*
+		 * Must be owner or have some privilege on the object (per spec,
+		 * any privilege will get you by here).  The owner is always
+		 * treated as having all grant options.
+		 */
+		if (superuser())		/* XXX no ownercheck() available */
+			my_goptions = ACL_ALL_RIGHTS_LANGUAGE;
+		else
+		{
+			AclMode		my_rights;
+
+			my_rights = pg_language_aclmask(HeapTupleGetOid(tuple),
+											GetUserId(),
+											ACL_ALL_RIGHTS_LANGUAGE | ACL_GRANT_OPTION_FOR(ACL_ALL_RIGHTS_LANGUAGE),
+											ACLMASK_ALL);
+			if (my_rights == ACL_NO_RIGHTS)
+				aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_LANGUAGE,
+							   NameStr(pg_language_tuple->lanname));
+			my_goptions = ACL_OPTION_TO_PRIVS(my_rights);
+		}
 
-		if (!pg_language_tuple->lanpltrusted)
-			ereport(ERROR,
-					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-					 errmsg("language \"%s\" is not trusted", langname)));
+		/*
+		 * 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 & my_goptions;
+		if (stmt->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")));
+		}
 
 		/*
 		 * If there's no ACL, substitute the proper default.
@@ -613,7 +829,7 @@ ExecuteGrantStmt_Language(GrantStmt *stmt)
 
 		new_acl = merge_acl_with_grant(old_acl, stmt->is_grant,
 									   stmt->grant_option, stmt->behavior,
-									   stmt->grantees, privileges,
+									   stmt->grantees, this_privileges,
 									   grantorId, ownerId);
 
 		/* finished building new ACL value, now insert it */
@@ -643,12 +859,17 @@ static void
 ExecuteGrantStmt_Namespace(GrantStmt *stmt)
 {
 	AclMode		privileges;
+	bool		all_privs;
 	ListCell   *i;
 
 	if (linitial_int(stmt->privileges) == ACL_ALL_RIGHTS)
+	{
+		all_privs = true;
 		privileges = ACL_ALL_RIGHTS_NAMESPACE;
+	}
 	else
 	{
+		all_privs = false;
 		privileges = ACL_NO_RIGHTS;
 		foreach(i, stmt->privileges)
 		{
@@ -671,6 +892,8 @@ ExecuteGrantStmt_Namespace(GrantStmt *stmt)
 		Form_pg_namespace pg_namespace_tuple;
 		Datum		aclDatum;
 		bool		isNull;
+		AclMode		my_goptions;
+		AclMode		this_privileges;
 		Acl		   *old_acl;
 		Acl		   *new_acl;
 		AclId		grantorId;
@@ -693,12 +916,58 @@ ExecuteGrantStmt_Namespace(GrantStmt *stmt)
 		ownerId = pg_namespace_tuple->nspowner;
 		grantorId = select_grantor(ownerId);
 
-		if (stmt->is_grant
-			&& !pg_namespace_ownercheck(HeapTupleGetOid(tuple), GetUserId())
-			&& pg_namespace_aclcheck(HeapTupleGetOid(tuple), GetUserId(),
-									 ACL_GRANT_OPTION_FOR(privileges)) != ACLCHECK_OK)
-			aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_NAMESPACE,
-						   nspname);
+		/*
+		 * Must be owner or have some privilege on the object (per spec,
+		 * any privilege will get you by here).  The owner is always
+		 * treated as having all grant options.
+		 */
+		if (pg_namespace_ownercheck(HeapTupleGetOid(tuple), GetUserId()))
+			my_goptions = ACL_ALL_RIGHTS_NAMESPACE;
+		else
+		{
+			AclMode		my_rights;
+
+			my_rights = pg_namespace_aclmask(HeapTupleGetOid(tuple),
+											 GetUserId(),
+											 ACL_ALL_RIGHTS_NAMESPACE | ACL_GRANT_OPTION_FOR(ACL_ALL_RIGHTS_NAMESPACE),
+											 ACLMASK_ALL);
+			if (my_rights == ACL_NO_RIGHTS)
+				aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_NAMESPACE,
+							   nspname);
+			my_goptions = ACL_OPTION_TO_PRIVS(my_rights);
+		}
+
+		/*
+		 * 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 & my_goptions;
+		if (stmt->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")));
+		}
 
 		/*
 		 * If there's no ACL, substitute the proper default.
@@ -714,7 +983,7 @@ ExecuteGrantStmt_Namespace(GrantStmt *stmt)
 
 		new_acl = merge_acl_with_grant(old_acl, stmt->is_grant,
 									   stmt->grant_option, stmt->behavior,
-									   stmt->grantees, privileges,
+									   stmt->grantees, this_privileges,
 									   grantorId, ownerId);
 
 		/* finished building new ACL value, now insert it */
@@ -816,147 +1085,6 @@ get_groname(AclId grosysid)
 	return name;
 }
 
-/*
- * Is user a member of group?
- */
-static bool
-in_group(AclId uid, AclId gid)
-{
-	bool		result = false;
-	HeapTuple	tuple;
-	Datum		att;
-	bool		isNull;
-	IdList	   *glist;
-	AclId	   *aidp;
-	int			i,
-				num;
-
-	tuple = SearchSysCache(GROSYSID,
-						   ObjectIdGetDatum(gid),
-						   0, 0, 0);
-	if (HeapTupleIsValid(tuple))
-	{
-		att = SysCacheGetAttr(GROSYSID,
-							  tuple,
-							  Anum_pg_group_grolist,
-							  &isNull);
-		if (!isNull)
-		{
-			/* be sure the IdList is not toasted */
-			glist = DatumGetIdListP(att);
-			/* scan it */
-			num = IDLIST_NUM(glist);
-			aidp = IDLIST_DAT(glist);
-			for (i = 0; i < num; ++i)
-			{
-				if (aidp[i] == uid)
-				{
-					result = true;
-					break;
-				}
-			}
-			/* if IdList was toasted, free detoasted copy */
-			if ((Pointer) glist != DatumGetPointer(att))
-				pfree(glist);
-		}
-		ReleaseSysCache(tuple);
-	}
-	else
-		ereport(WARNING,
-				(errcode(ERRCODE_UNDEFINED_OBJECT),
-				 errmsg("group with ID %u does not exist", gid)));
-	return result;
-}
-
-
-/*
- * aclmask --- compute bitmask of all privileges held by userid.
- *
- * When 'how' = ACLMASK_ALL, this simply returns the privilege bits
- * held by the given userid according to the given ACL list, ANDed
- * with 'mask'.  (The point of passing 'mask' is to let the routine
- * exit early if all privileges of interest have been found.)
- *
- * When 'how' = ACLMASK_ANY, returns as soon as any bit in the mask
- * is known true.  (This lets us exit soonest in cases where the
- * caller is only going to test for zero or nonzero result.)
- *
- * Usage patterns:
- *
- * To see if any of a set of privileges are held:
- *		if (aclmask(acl, userid, privs, ACLMASK_ANY) != 0)
- *
- * To see if all of a set of privileges are held:
- *		if (aclmask(acl, userid, privs, ACLMASK_ALL) == privs)
- *
- * To determine exactly which of a set of privileges are held:
- *		heldprivs = aclmask(acl, userid, privs, ACLMASK_ALL);
- */
-static AclMode
-aclmask(Acl *acl, AclId userid, AclMode mask, AclMaskHow how)
-{
-	AclMode		result;
-	AclMode		remaining;
-	AclItem    *aidat;
-	int			i,
-				num;
-
-	/*
-	 * Null ACL should not happen, since caller should have inserted
-	 * appropriate default
-	 */
-	if (acl == NULL)
-		elog(ERROR, "null ACL");
-
-	/* Quick exit for mask == 0 */
-	if (mask == 0)
-		return 0;
-
-	num = ACL_NUM(acl);
-	aidat = ACL_DAT(acl);
-
-	result = 0;
-
-	/*
-	 * Check privileges granted directly to user or to public
-	 */
-	for (i = 0; i < num; i++)
-	{
-		AclItem	   *aidata = &aidat[i];
-
-		if (ACLITEM_GET_IDTYPE(*aidata) == ACL_IDTYPE_WORLD
-			|| (ACLITEM_GET_IDTYPE(*aidata) == ACL_IDTYPE_UID
-				&& aidata->ai_grantee == userid))
-		{
-			result |= (aidata->ai_privs & mask);
-			if ((how == ACLMASK_ALL) ? (result == mask) : (result != 0))
-				return result;
-		}
-	}
-
-	/*
-	 * Check privileges granted via groups.  We do this in a separate
-	 * pass to minimize expensive lookups in pg_group.
-	 */
-	remaining = (mask & ~result);
-	for (i = 0; i < num; i++)
-	{
-		AclItem	   *aidata = &aidat[i];
-
-		if (ACLITEM_GET_IDTYPE(*aidata) == ACL_IDTYPE_GID
-			&& (aidata->ai_privs & remaining)
-			&& in_group(userid, aidata->ai_grantee))
-		{
-			result |= (aidata->ai_privs & mask);
-			if ((how == ACLMASK_ALL) ? (result == mask) : (result != 0))
-				return result;
-			remaining = (mask & ~result);
-		}
-	}
-
-	return result;
-}
-
 
 /*
  * Standardized reporting of aclcheck permissions failures.
@@ -1058,6 +1186,7 @@ pg_class_aclmask(Oid table_oid, AclId userid,
 	Datum		aclDatum;
 	bool		isNull;
 	Acl		   *acl;
+	AclId		ownerId;
 
 	/*
 	 * Validate userid, find out if he is superuser, also get usecatupd
@@ -1125,13 +1254,13 @@ pg_class_aclmask(Oid table_oid, AclId userid,
 	/*
 	 * Normal case: get the relation's ACL from pg_class
 	 */
+	ownerId = classForm->relowner;
+
 	aclDatum = SysCacheGetAttr(RELOID, tuple, Anum_pg_class_relacl,
 							   &isNull);
 	if (isNull)
 	{
 		/* No ACL, so build default ACL */
-		AclId		ownerId = classForm->relowner;
-
 		acl = acldefault(ACL_OBJECT_RELATION, ownerId);
 		aclDatum = (Datum) 0;
 	}
@@ -1141,7 +1270,7 @@ pg_class_aclmask(Oid table_oid, AclId userid,
 		acl = DatumGetAclP(aclDatum);
 	}
 
-	result = aclmask(acl, userid, mask, how);
+	result = aclmask(acl, userid, ownerId, mask, how);
 
 	/* if we have a detoasted copy, free it */
 	if (acl && (Pointer) acl != DatumGetPointer(aclDatum))
@@ -1167,6 +1296,7 @@ pg_database_aclmask(Oid db_oid, AclId userid,
 	Datum		aclDatum;
 	bool		isNull;
 	Acl		   *acl;
+	AclId		ownerId;
 
 	/* Superusers bypass all permission checking. */
 	if (superuser_arg(userid))
@@ -1189,15 +1319,14 @@ pg_database_aclmask(Oid db_oid, AclId userid,
 				(errcode(ERRCODE_UNDEFINED_DATABASE),
 				 errmsg("database with OID %u does not exist", db_oid)));
 
+	ownerId = ((Form_pg_database) GETSTRUCT(tuple))->datdba;
+
 	aclDatum = heap_getattr(tuple, Anum_pg_database_datacl,
 							RelationGetDescr(pg_database), &isNull);
 
 	if (isNull)
 	{
 		/* No ACL, so build default ACL */
-		AclId		ownerId;
-
-		ownerId = ((Form_pg_database) GETSTRUCT(tuple))->datdba;
 		acl = acldefault(ACL_OBJECT_DATABASE, ownerId);
 		aclDatum = (Datum) 0;
 	}
@@ -1207,7 +1336,7 @@ pg_database_aclmask(Oid db_oid, AclId userid,
 		acl = DatumGetAclP(aclDatum);
 	}
 
-	result = aclmask(acl, userid, mask, how);
+	result = aclmask(acl, userid, ownerId, mask, how);
 
 	/* if we have a detoasted copy, free it */
 	if (acl && (Pointer) acl != DatumGetPointer(aclDatum))
@@ -1231,6 +1360,7 @@ pg_proc_aclmask(Oid proc_oid, AclId userid,
 	Datum		aclDatum;
 	bool		isNull;
 	Acl		   *acl;
+	AclId		ownerId;
 
 	/* Superusers bypass all permission checking. */
 	if (superuser_arg(userid))
@@ -1247,14 +1377,13 @@ pg_proc_aclmask(Oid proc_oid, AclId userid,
 				(errcode(ERRCODE_UNDEFINED_FUNCTION),
 			   errmsg("function with OID %u does not exist", proc_oid)));
 
+	ownerId = ((Form_pg_proc) GETSTRUCT(tuple))->proowner;
+
 	aclDatum = SysCacheGetAttr(PROCOID, tuple, Anum_pg_proc_proacl,
 							   &isNull);
 	if (isNull)
 	{
 		/* No ACL, so build default ACL */
-		AclId		ownerId;
-
-		ownerId = ((Form_pg_proc) GETSTRUCT(tuple))->proowner;
 		acl = acldefault(ACL_OBJECT_FUNCTION, ownerId);
 		aclDatum = (Datum) 0;
 	}
@@ -1264,7 +1393,7 @@ pg_proc_aclmask(Oid proc_oid, AclId userid,
 		acl = DatumGetAclP(aclDatum);
 	}
 
-	result = aclmask(acl, userid, mask, how);
+	result = aclmask(acl, userid, ownerId, mask, how);
 
 	/* if we have a detoasted copy, free it */
 	if (acl && (Pointer) acl != DatumGetPointer(aclDatum))
@@ -1287,6 +1416,7 @@ pg_language_aclmask(Oid lang_oid, AclId userid,
 	Datum		aclDatum;
 	bool		isNull;
 	Acl		   *acl;
+	AclId		ownerId;
 
 	/* Superusers bypass all permission checking. */
 	if (superuser_arg(userid))
@@ -1303,13 +1433,15 @@ pg_language_aclmask(Oid lang_oid, AclId userid,
 				(errcode(ERRCODE_UNDEFINED_OBJECT),
 			   errmsg("language with OID %u does not exist", lang_oid)));
 
+	/* XXX pg_language should have an owner column, but doesn't */
+	ownerId = BOOTSTRAP_USESYSID;
+
 	aclDatum = SysCacheGetAttr(LANGOID, tuple, Anum_pg_language_lanacl,
 							   &isNull);
 	if (isNull)
 	{
 		/* No ACL, so build default ACL */
-		/* XXX pg_language should have an owner column, but doesn't */
-		acl = acldefault(ACL_OBJECT_LANGUAGE, BOOTSTRAP_USESYSID);
+		acl = acldefault(ACL_OBJECT_LANGUAGE, ownerId);
 		aclDatum = (Datum) 0;
 	}
 	else
@@ -1318,7 +1450,7 @@ pg_language_aclmask(Oid lang_oid, AclId userid,
 		acl = DatumGetAclP(aclDatum);
 	}
 
-	result = aclmask(acl, userid, mask, how);
+	result = aclmask(acl, userid, ownerId, mask, how);
 
 	/* if we have a detoasted copy, free it */
 	if (acl && (Pointer) acl != DatumGetPointer(aclDatum))
@@ -1341,6 +1473,7 @@ pg_namespace_aclmask(Oid nsp_oid, AclId userid,
 	Datum		aclDatum;
 	bool		isNull;
 	Acl		   *acl;
+	AclId		ownerId;
 
 	/* Superusers bypass all permission checking. */
 	if (superuser_arg(userid))
@@ -1385,14 +1518,13 @@ pg_namespace_aclmask(Oid nsp_oid, AclId userid,
 				(errcode(ERRCODE_UNDEFINED_SCHEMA),
 				 errmsg("schema with OID %u does not exist", nsp_oid)));
 
+	ownerId = ((Form_pg_namespace) GETSTRUCT(tuple))->nspowner;
+
 	aclDatum = SysCacheGetAttr(NAMESPACEOID, tuple, Anum_pg_namespace_nspacl,
 							   &isNull);
 	if (isNull)
 	{
 		/* No ACL, so build default ACL */
-		AclId		ownerId;
-
-		ownerId = ((Form_pg_namespace) GETSTRUCT(tuple))->nspowner;
 		acl = acldefault(ACL_OBJECT_NAMESPACE, ownerId);
 		aclDatum = (Datum) 0;
 	}
@@ -1402,7 +1534,7 @@ pg_namespace_aclmask(Oid nsp_oid, AclId userid,
 		acl = DatumGetAclP(aclDatum);
 	}
 
-	result = aclmask(acl, userid, mask, how);
+	result = aclmask(acl, userid, ownerId, mask, how);
 
 	/* if we have a detoasted copy, free it */
 	if (acl && (Pointer) acl != DatumGetPointer(aclDatum))
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 214bda2245a..d02683245ac 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/acl.c,v 1.104 2004/05/07 00:24:57 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/acl.c,v 1.105 2004/06/01 21:49:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -17,6 +17,7 @@
 #include <ctype.h>
 
 #include "catalog/namespace.h"
+#include "catalog/pg_group.h"
 #include "catalog/pg_shadow.h"
 #include "catalog/pg_type.h"
 #include "commands/dbcommands.h"
@@ -35,8 +36,11 @@ static void putid(char *p, const char *s);
 static Acl *allocacl(int n);
 static const char *aclparse(const char *s, AclItem *aip);
 static bool aclitem_match(const AclItem *a1, const AclItem *a2);
-static Acl *recursive_revoke(Acl *acl, AclId grantee,
-				 AclMode revoke_privs, DropBehavior behavior);
+static void check_circularity(const Acl *old_acl, const AclItem *mod_aip,
+							  AclId ownerid);
+static Acl *recursive_revoke(Acl *acl, AclId grantee, AclMode revoke_privs,
+							 AclId ownerid, DropBehavior behavior);
+static bool in_group(AclId uid, AclId gid);
 
 static AclMode convert_priv_string(text *priv_type_text);
 
@@ -554,10 +558,19 @@ acldefault(GrantObjectType objtype, AclId ownerid)
 		aip++;
 	}
 
+	/*
+	 * Note that the owner's entry shows all ordinary privileges but no
+	 * grant options.  This is because his grant options come "from the
+	 * system" and not from his own efforts.  (The SQL spec says that
+	 * the owner's rights come from a "_SYSTEM" authid.)  However, we do
+	 * consider that the owner's ordinary privileges are self-granted;
+	 * this lets him revoke them.  We implement the owner's grant options
+	 * without any explicit "_SYSTEM"-like ACL entry, by internally
+	 * special-casing the owner whereever we are testing grant options.
+	 */
 	aip->ai_grantee = ownerid;
 	aip->ai_grantor = ownerid;
-	/* owner gets default privileges with grant option */
-	ACLITEM_SET_PRIVS_IDTYPE(*aip, owner_default, owner_default,
+	ACLITEM_SET_PRIVS_IDTYPE(*aip, owner_default, ACL_NO_RIGHTS,
 							 ACL_IDTYPE_UID);
 
 	return acl;
@@ -565,21 +578,31 @@ acldefault(GrantObjectType objtype, AclId ownerid)
 
 
 /*
- * Add or replace an item in an ACL array.	The result is a modified copy;
- * the input object is not changed.
+ * Update an ACL array to add or remove specified privileges.
+ *
+ *	old_acl: the input ACL array
+ *	mod_aip: defines the privileges to be added, removed, or substituted
+ *	modechg: ACL_MODECHG_ADD, ACL_MODECHG_DEL, or ACL_MODECHG_EQL
+ *	ownerid: AclId of object owner
+ *	behavior: RESTRICT or CASCADE behavior for recursive removal
+ *
+ * ownerid and behavior are only relevant when the update operation specifies
+ * deletion of grant options.
+ *
+ * The result is a modified copy; the input object is not changed.
  *
  * NB: caller is responsible for having detoasted the input ACL, if needed.
  */
 Acl *
-aclinsert3(const Acl *old_acl, const AclItem *mod_aip,
-		   unsigned modechg, DropBehavior behavior)
+aclupdate(const Acl *old_acl, const AclItem *mod_aip,
+		  int modechg, AclId ownerid, DropBehavior behavior)
 {
 	Acl		   *new_acl = NULL;
 	AclItem    *old_aip,
 			   *new_aip = NULL;
-	AclMode		old_privs,
+	AclMode		old_rights,
 				old_goptions,
-				new_privs,
+				new_rights,
 				new_goptions;
 	int			dst,
 				num;
@@ -590,10 +613,15 @@ aclinsert3(const Acl *old_acl, const AclItem *mod_aip,
 	if (!mod_aip)
 	{
 		new_acl = allocacl(ACL_NUM(old_acl));
-		memcpy((char *) new_acl, (char *) old_acl, ACL_SIZE(old_acl));
+		memcpy(new_acl, old_acl, ACL_SIZE(old_acl));
 		return new_acl;
 	}
 
+	/* If granting grant options, check for circularity */
+	if (modechg != ACL_MODECHG_DEL &&
+		ACLITEM_GET_GOPTIONS(*mod_aip) != ACL_NO_RIGHTS)
+		check_circularity(old_acl, mod_aip, ownerid);
+
 	num = ACL_NUM(old_acl);
 	old_aip = ACL_DAT(old_acl);
 
@@ -626,44 +654,39 @@ aclinsert3(const Acl *old_acl, const AclItem *mod_aip,
 		/* initialize the new entry with no permissions */
 		new_aip[dst].ai_grantee = mod_aip->ai_grantee;
 		new_aip[dst].ai_grantor = mod_aip->ai_grantor;
-		ACLITEM_SET_PRIVS_IDTYPE(new_aip[dst], ACL_NO_RIGHTS, ACL_NO_RIGHTS,
+		ACLITEM_SET_PRIVS_IDTYPE(new_aip[dst],
+								 ACL_NO_RIGHTS, ACL_NO_RIGHTS,
 								 ACLITEM_GET_IDTYPE(*mod_aip));
 		num++;					/* set num to the size of new_acl */
 	}
 
-	old_privs = ACLITEM_GET_PRIVS(new_aip[dst]);
+	old_rights = ACLITEM_GET_RIGHTS(new_aip[dst]);
 	old_goptions = ACLITEM_GET_GOPTIONS(new_aip[dst]);
 
 	/* apply the specified permissions change */
 	switch (modechg)
 	{
 		case ACL_MODECHG_ADD:
-			ACLITEM_SET_PRIVS(new_aip[dst],
-							  old_privs | ACLITEM_GET_PRIVS(*mod_aip));
-			ACLITEM_SET_GOPTIONS(new_aip[dst],
-								 old_goptions | ACLITEM_GET_GOPTIONS(*mod_aip));
+			ACLITEM_SET_RIGHTS(new_aip[dst],
+							   old_rights | ACLITEM_GET_RIGHTS(*mod_aip));
 			break;
 		case ACL_MODECHG_DEL:
-			ACLITEM_SET_PRIVS(new_aip[dst],
-							  old_privs & ~ACLITEM_GET_PRIVS(*mod_aip));
-			ACLITEM_SET_GOPTIONS(new_aip[dst],
-								 old_goptions & ~ACLITEM_GET_GOPTIONS(*mod_aip));
+			ACLITEM_SET_RIGHTS(new_aip[dst],
+							   old_rights & ~ACLITEM_GET_RIGHTS(*mod_aip));
 			break;
 		case ACL_MODECHG_EQL:
-			ACLITEM_SET_PRIVS_IDTYPE(new_aip[dst],
-									 ACLITEM_GET_PRIVS(*mod_aip),
-									 ACLITEM_GET_GOPTIONS(*mod_aip),
-									 ACLITEM_GET_IDTYPE(new_aip[dst]));
+			ACLITEM_SET_RIGHTS(new_aip[dst],
+							   ACLITEM_GET_RIGHTS(*mod_aip));
 			break;
 	}
 
-	new_privs = ACLITEM_GET_PRIVS(new_aip[dst]);
+	new_rights = ACLITEM_GET_RIGHTS(new_aip[dst]);
 	new_goptions = ACLITEM_GET_GOPTIONS(new_aip[dst]);
 
 	/*
 	 * If the adjusted entry has no permissions, delete it from the list.
 	 */
-	if (new_privs == ACL_NO_RIGHTS && new_goptions == ACL_NO_RIGHTS)
+	if (new_rights == ACL_NO_RIGHTS)
 	{
 		memmove(new_aip + dst,
 				new_aip + dst + 1,
@@ -676,40 +699,143 @@ aclinsert3(const Acl *old_acl, const AclItem *mod_aip,
 	 * Remove abandoned privileges (cascading revoke).  Currently we
 	 * can only handle this when the grantee is a user.
 	 */
-	if ((old_goptions & ~new_goptions) != 0
-		&& ACLITEM_GET_IDTYPE(*mod_aip) == ACL_IDTYPE_UID)
+	if ((old_goptions & ~new_goptions) != 0)
+	{
+		Assert(ACLITEM_GET_IDTYPE(*mod_aip) == ACL_IDTYPE_UID);
 		new_acl = recursive_revoke(new_acl, mod_aip->ai_grantee,
 								   (old_goptions & ~new_goptions),
-								   behavior);
+								   ownerid, behavior);
+	}
 
 	return new_acl;
 }
 
 
 /*
+ * When granting grant options, we must disallow attempts to set up circular
+ * chains of grant options.  Suppose A (the object owner) grants B some
+ * privileges with grant option, and B re-grants them to C.  If C could
+ * grant the privileges to B as well, then A would be unable to effectively
+ * revoke the privileges from B, since recursive_revoke would consider that
+ * B still has 'em from C.
+ *
+ * We check for this by recursively deleting all grant options belonging to
+ * the target grantee, and then seeing if the would-be grantor still has the
+ * grant option or not.
+ */
+static void
+check_circularity(const Acl *old_acl, const AclItem *mod_aip,
+				  AclId ownerid)
+{
+	Acl		   *acl;
+	AclItem    *aip;
+	int			i,
+				num;
+	AclMode		own_privs;
+
+	/*
+	 * For now, grant options can only be granted to users, not groups or
+	 * PUBLIC.  Otherwise we'd have to work a bit harder here.
+	 */
+	Assert(ACLITEM_GET_IDTYPE(*mod_aip) == ACL_IDTYPE_UID);
+
+	/* The owner always has grant options, no need to check */
+	if (mod_aip->ai_grantor == ownerid)
+		return;
+
+	/* Make a working copy */
+	acl = allocacl(ACL_NUM(old_acl));
+	memcpy(acl, old_acl, ACL_SIZE(old_acl));
+
+	/* Zap all grant options of target grantee, plus what depends on 'em */
+cc_restart:
+	num = ACL_NUM(acl);
+	aip = ACL_DAT(acl);
+	for (i = 0; i < num; i++)
+	{
+		if (ACLITEM_GET_IDTYPE(aip[i]) == ACL_IDTYPE_UID &&
+			aip[i].ai_grantee == mod_aip->ai_grantee &&
+			ACLITEM_GET_GOPTIONS(aip[i]) != ACL_NO_RIGHTS)
+		{
+			Acl		   *new_acl;
+
+			/* We'll actually zap ordinary privs too, but no matter */
+			new_acl = aclupdate(acl, &aip[i], ACL_MODECHG_DEL,
+								ownerid, DROP_CASCADE);
+
+			pfree(acl);
+			acl = new_acl;
+
+			goto cc_restart;
+		}
+	}
+
+	/* Now we can compute grantor's independently-derived privileges */
+	own_privs = aclmask(acl,
+						mod_aip->ai_grantor,
+						ownerid,
+						ACL_GRANT_OPTION_FOR(ACLITEM_GET_GOPTIONS(*mod_aip)),
+						ACLMASK_ALL);
+	own_privs = ACL_OPTION_TO_PRIVS(own_privs);
+
+	if ((ACLITEM_GET_GOPTIONS(*mod_aip) & ~own_privs) != 0)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_GRANT_OPERATION),
+				 errmsg("grant options cannot be granted back to your own grantor")));
+
+	pfree(acl);
+}
+
+
+/*
  * Ensure that no privilege is "abandoned".  A privilege is abandoned
  * if the user that granted the privilege loses the grant option.  (So
  * the chain through which it was granted is broken.)  Either the
  * abandoned privileges are revoked as well, or an error message is
  * printed, depending on the drop behavior option.
+ *
+ *	acl: the input ACL list
+ *	grantee: the user from whom some grant options have been revoked
+ *	revoke_privs: the grant options being revoked
+ *	ownerid: AclId of object owner
+ *	behavior: RESTRICT or CASCADE behavior for recursive removal
+ *
+ * The input Acl object is pfree'd if replaced.
  */
 static Acl *
 recursive_revoke(Acl *acl,
 				 AclId grantee,
 				 AclMode revoke_privs,
+				 AclId ownerid,
 				 DropBehavior behavior)
 {
-	int			i;
+	AclMode		still_has;
+	AclItem    *aip;
+	int			i,
+				num;
+
+	/* The owner can never truly lose grant options, so short-circuit */
+	if (grantee == ownerid)
+		return acl;
+
+	/* The grantee might still have the privileges via another grantor */
+	still_has = aclmask(acl, grantee, ownerid,
+						ACL_GRANT_OPTION_FOR(revoke_privs),
+						ACLMASK_ALL);
+	revoke_privs &= ~still_has;
+	if (revoke_privs == ACL_NO_RIGHTS)
+		return acl;
 
 restart:
-	for (i = 0; i < ACL_NUM(acl); i++)
+	num = ACL_NUM(acl);
+	aip = ACL_DAT(acl);
+	for (i = 0; i < num; i++)
 	{
-		AclItem    *aip = ACL_DAT(acl);
-
 		if (aip[i].ai_grantor == grantee
 			&& (ACLITEM_GET_PRIVS(aip[i]) & revoke_privs) != 0)
 		{
 			AclItem		mod_acl;
+			Acl		   *new_acl;
 
 			if (behavior == DROP_RESTRICT)
 				ereport(ERROR,
@@ -724,7 +850,12 @@ restart:
 									 revoke_privs,
 									 ACLITEM_GET_IDTYPE(aip[i]));
 
-			acl = aclinsert3(acl, &mod_acl, ACL_MODECHG_DEL, behavior);
+			new_acl = aclupdate(acl, &mod_acl, ACL_MODECHG_DEL,
+								ownerid, behavior);
+
+			pfree(acl);
+			acl = new_acl;
+
 			goto restart;
 		}
 	}
@@ -734,70 +865,177 @@ restart:
 
 
 /*
- * aclinsert (exported function)
+ * aclmask --- compute bitmask of all privileges held by userid.
+ *
+ * When 'how' = ACLMASK_ALL, this simply returns the privilege bits
+ * held by the given userid according to the given ACL list, ANDed
+ * with 'mask'.  (The point of passing 'mask' is to let the routine
+ * exit early if all privileges of interest have been found.)
+ *
+ * When 'how' = ACLMASK_ANY, returns as soon as any bit in the mask
+ * is known true.  (This lets us exit soonest in cases where the
+ * caller is only going to test for zero or nonzero result.)
+ *
+ * Usage patterns:
+ *
+ * To see if any of a set of privileges are held:
+ *		if (aclmask(acl, userid, ownerid, privs, ACLMASK_ANY) != 0)
+ *
+ * To see if all of a set of privileges are held:
+ *		if (aclmask(acl, userid, ownerid, privs, ACLMASK_ALL) == privs)
+ *
+ * To determine exactly which of a set of privileges are held:
+ *		heldprivs = aclmask(acl, userid, ownerid, privs, ACLMASK_ALL);
  */
-Datum
-aclinsert(PG_FUNCTION_ARGS)
+AclMode
+aclmask(const Acl *acl, AclId userid, AclId ownerid,
+		AclMode mask, AclMaskHow how)
 {
-	Acl		   *old_acl = PG_GETARG_ACL_P(0);
-	AclItem    *mod_aip = PG_GETARG_ACLITEM_P(1);
+	AclMode		result;
+	AclMode		remaining;
+	AclItem    *aidat;
+	int			i,
+				num;
 
-	PG_RETURN_ACL_P(aclinsert3(old_acl, mod_aip, ACL_MODECHG_EQL, DROP_CASCADE));
-}
+	/*
+	 * Null ACL should not happen, since caller should have inserted
+	 * appropriate default
+	 */
+	if (acl == NULL)
+		elog(ERROR, "null ACL");
 
-Datum
-aclremove(PG_FUNCTION_ARGS)
-{
-	Acl		   *old_acl = PG_GETARG_ACL_P(0);
-	AclItem    *mod_aip = PG_GETARG_ACLITEM_P(1);
-	Acl		   *new_acl;
-	AclItem    *old_aip,
-			   *new_aip;
-	int			dst,
-				old_num,
-				new_num;
+	/* Quick exit for mask == 0 */
+	if (mask == 0)
+		return 0;
 
-	/* These checks for null input should be dead code, but... */
-	if (!old_acl || ACL_NUM(old_acl) < 0)
-		old_acl = allocacl(0);
-	if (!mod_aip)
+	result = 0;
+
+	/* Owner always implicitly has all grant options */
+	if (userid == ownerid)
 	{
-		new_acl = allocacl(ACL_NUM(old_acl));
-		memcpy((char *) new_acl, (char *) old_acl, ACL_SIZE(old_acl));
-		PG_RETURN_ACL_P(new_acl);
+		result = mask & ACLITEM_ALL_GOPTION_BITS;
+		if (result == mask)
+			return result;
 	}
 
-	old_num = ACL_NUM(old_acl);
-	old_aip = ACL_DAT(old_acl);
+	num = ACL_NUM(acl);
+	aidat = ACL_DAT(acl);
+
+	/*
+	 * Check privileges granted directly to user or to public
+	 */
+	for (i = 0; i < num; i++)
+	{
+		AclItem	   *aidata = &aidat[i];
 
-	/* Search for the matching entry */
-	for (dst = 0;
-		 dst < old_num && !aclitem_match(mod_aip, old_aip + dst);
-		 ++dst)
-		 /* continue */ ;
+		if (ACLITEM_GET_IDTYPE(*aidata) == ACL_IDTYPE_WORLD
+			|| (ACLITEM_GET_IDTYPE(*aidata) == ACL_IDTYPE_UID
+				&& aidata->ai_grantee == userid))
+		{
+			result |= (aidata->ai_privs & mask);
+			if ((how == ACLMASK_ALL) ? (result == mask) : (result != 0))
+				return result;
+		}
+	}
 
-	if (dst >= old_num)
+	/*
+	 * Check privileges granted via groups.  We do this in a separate
+	 * pass to minimize expensive lookups in pg_group.
+	 */
+	remaining = (mask & ~result);
+	for (i = 0; i < num; i++)
 	{
-		/* Not found, so return copy of source ACL */
-		new_acl = allocacl(old_num);
-		memcpy((char *) new_acl, (char *) old_acl, ACL_SIZE(old_acl));
+		AclItem	   *aidata = &aidat[i];
+
+		if (ACLITEM_GET_IDTYPE(*aidata) == ACL_IDTYPE_GID
+			&& (aidata->ai_privs & remaining)
+			&& in_group(userid, aidata->ai_grantee))
+		{
+			result |= (aidata->ai_privs & mask);
+			if ((how == ACLMASK_ALL) ? (result == mask) : (result != 0))
+				return result;
+			remaining = (mask & ~result);
+		}
 	}
-	else
+
+	return result;
+}
+
+
+/*
+ * Is user a member of group?
+ */
+static bool
+in_group(AclId uid, AclId gid)
+{
+	bool		result = false;
+	HeapTuple	tuple;
+	Datum		att;
+	bool		isNull;
+	IdList	   *glist;
+	AclId	   *aidp;
+	int			i,
+				num;
+
+	tuple = SearchSysCache(GROSYSID,
+						   ObjectIdGetDatum(gid),
+						   0, 0, 0);
+	if (HeapTupleIsValid(tuple))
 	{
-		new_num = old_num - 1;
-		new_acl = allocacl(new_num);
-		new_aip = ACL_DAT(new_acl);
-		if (dst > 0)
-			memcpy((char *) new_aip,
-				   (char *) old_aip,
-				   dst * sizeof(AclItem));
-		if (dst < new_num)
-			memcpy((char *) (new_aip + dst),
-				   (char *) (old_aip + dst + 1),
-				   (new_num - dst) * sizeof(AclItem));
+		att = SysCacheGetAttr(GROSYSID,
+							  tuple,
+							  Anum_pg_group_grolist,
+							  &isNull);
+		if (!isNull)
+		{
+			/* be sure the IdList is not toasted */
+			glist = DatumGetIdListP(att);
+			/* scan it */
+			num = IDLIST_NUM(glist);
+			aidp = IDLIST_DAT(glist);
+			for (i = 0; i < num; ++i)
+			{
+				if (aidp[i] == uid)
+				{
+					result = true;
+					break;
+				}
+			}
+			/* if IdList was toasted, free detoasted copy */
+			if ((Pointer) glist != DatumGetPointer(att))
+				pfree(glist);
+		}
+		ReleaseSysCache(tuple);
 	}
+	else
+		ereport(WARNING,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+				 errmsg("group with ID %u does not exist", gid)));
+	return result;
+}
+
+
+/*
+ * aclinsert (exported function)
+ */
+Datum
+aclinsert(PG_FUNCTION_ARGS)
+{
+	ereport(ERROR,
+			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+			 errmsg("aclinsert is no longer supported")));
+
+	PG_RETURN_NULL();			/* keep compiler quiet */
+}
+
+Datum
+aclremove(PG_FUNCTION_ARGS)
+{
+	ereport(ERROR,
+			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+			 errmsg("aclremove is no longer supported")));
 
-	PG_RETURN_ACL_P(new_acl);
+	PG_RETURN_NULL();			/* keep compiler quiet */
 }
 
 Datum
@@ -816,8 +1054,7 @@ aclcontains(PG_FUNCTION_ARGS)
 		if (aip->ai_grantee == aidat[i].ai_grantee
 			&& ACLITEM_GET_IDTYPE(*aip) == ACLITEM_GET_IDTYPE(aidat[i])
 			&& aip->ai_grantor == aidat[i].ai_grantor
-			&& (ACLITEM_GET_PRIVS(*aip) & ACLITEM_GET_PRIVS(aidat[i])) == ACLITEM_GET_PRIVS(*aip)
-			&& (ACLITEM_GET_GOPTIONS(*aip) & ACLITEM_GET_GOPTIONS(aidat[i])) == ACLITEM_GET_GOPTIONS(*aip))
+			&& (ACLITEM_GET_RIGHTS(*aip) & ACLITEM_GET_RIGHTS(aidat[i])) == ACLITEM_GET_RIGHTS(*aip))
 			PG_RETURN_BOOL(true);
 	}
 	PG_RETURN_BOOL(false);
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 59131122cbc..f5ac89e2579 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/utils/acl.h,v 1.69 2004/05/11 17:36:13 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/utils/acl.h,v 1.70 2004/06/01 21:49:22 tgl Exp $
  *
  * NOTES
  *	  An ACL array is simply an array of AclItems, representing the union
@@ -63,13 +63,16 @@ typedef struct AclItem
 /*
  * The AclIdType is stored in the top two bits of the ai_privs field
  * of an AclItem.  The middle 15 bits are the grant option markers,
- * and the lower 15 bits are the actual privileges.
+ * and the lower 15 bits are the actual privileges.  We use "rights"
+ * to mean the combined grant option and privilege bits fields.
  */
 #define ACLITEM_GET_PRIVS(item)    ((item).ai_privs & 0x7FFF)
 #define ACLITEM_GET_GOPTIONS(item) (((item).ai_privs >> 15) & 0x7FFF)
+#define ACLITEM_GET_RIGHTS(item)   ((item).ai_privs & 0x3FFFFFFF)
 #define ACLITEM_GET_IDTYPE(item)   ((item).ai_privs >> 30)
 
 #define ACL_GRANT_OPTION_FOR(privs) (((AclMode) (privs) & 0x7FFF) << 15)
+#define ACL_OPTION_TO_PRIVS(privs)  (((AclMode) (privs) >> 15) & 0x7FFF)
 
 #define ACLITEM_SET_PRIVS(item,privs) \
   ((item).ai_privs = ((item).ai_privs & ~((AclMode) 0x7FFF)) | \
@@ -77,6 +80,9 @@ typedef struct AclItem
 #define ACLITEM_SET_GOPTIONS(item,goptions) \
   ((item).ai_privs = ((item).ai_privs & ~(((AclMode) 0x7FFF) << 15)) | \
 					 (((AclMode) (goptions) & 0x7FFF) << 15))
+#define ACLITEM_SET_RIGHTS(item,rights) \
+  ((item).ai_privs = ((item).ai_privs & ~((AclMode) 0x3FFFFFFF)) | \
+					 ((AclMode) (rights) & 0x3FFFFFFF))
 #define ACLITEM_SET_IDTYPE(item,idtype) \
   ((item).ai_privs = ((item).ai_privs & ~(((AclMode) 0x03) << 30)) | \
 					 (((AclMode) (idtype) & 0x03) << 30))
@@ -86,6 +92,8 @@ typedef struct AclItem
 					 (((AclMode) (goption) & 0x7FFF) << 15) | \
 					 ((AclMode) (idtype) << 30))
 
+#define ACLITEM_ALL_PRIV_BITS		((AclMode) 0x7FFF)
+#define ACLITEM_ALL_GOPTION_BITS	((AclMode) 0x7FFF << 15)
 
 /*
  * Definitions for convenient access to Acl (array of AclItem) and IdList
@@ -143,7 +151,7 @@ typedef ArrayType IdList;
 
 
 /*
- * ACL modification opcodes
+ * ACL modification opcodes for aclupdate
  */
 #define ACL_MODECHG_ADD			1
 #define ACL_MODECHG_DEL			2
@@ -212,8 +220,10 @@ typedef enum AclObjectKind
  * routines used internally
  */
 extern Acl *acldefault(GrantObjectType objtype, AclId ownerid);
-extern Acl *aclinsert3(const Acl *old_acl, const AclItem *mod_aip,
-		   unsigned modechg, DropBehavior behavior);
+extern Acl *aclupdate(const Acl *old_acl, const AclItem *mod_aip,
+					  int modechg, AclId ownerid, DropBehavior behavior);
+extern AclMode aclmask(const Acl *acl, AclId userid, AclId ownerid,
+					   AclMode mask, AclMaskHow how);
 
 /*
  * SQL functions (from acl.c)
diff --git a/src/include/utils/errcodes.h b/src/include/utils/errcodes.h
index cdee2737f0d..270eb2073bc 100644
--- a/src/include/utils/errcodes.h
+++ b/src/include/utils/errcodes.h
@@ -11,7 +11,7 @@
  *
  * Copyright (c) 2003, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/include/utils/errcodes.h,v 1.11 2004/05/16 23:18:55 neilc Exp $
+ * $PostgreSQL: pgsql/src/include/utils/errcodes.h,v 1.12 2004/06/01 21:49:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -59,6 +59,8 @@
 #define ERRCODE_WARNING_DYNAMIC_RESULT_SETS_RETURNED		MAKE_SQLSTATE('0','1', '0','0','C')
 #define ERRCODE_WARNING_IMPLICIT_ZERO_BIT_PADDING	MAKE_SQLSTATE('0','1', '0','0','8')
 #define ERRCODE_WARNING_NULL_VALUE_ELIMINATED_IN_SET_FUNCTION	MAKE_SQLSTATE('0','1', '0','0','3')
+#define ERRCODE_WARNING_PRIVILEGE_NOT_GRANTED		MAKE_SQLSTATE('0','1', '0','0','7')
+#define ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED		MAKE_SQLSTATE('0','1', '0','0','6')
 #define ERRCODE_WARNING_STRING_DATA_RIGHT_TRUNCATION	MAKE_SQLSTATE('0','1', '0','0','4')
 #define ERRCODE_WARNING_DEPRECATED_FEATURE	MAKE_SQLSTATE('0','1', 'P','0','1')
 
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index f5a6c039d54..83903f6b979 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -89,7 +89,7 @@ ERROR:  permission denied for relation atest2
 COPY atest2 FROM stdin; -- fail
 ERROR:  permission denied for relation atest2
 GRANT ALL ON atest1 TO PUBLIC; -- fail
-ERROR:  permission denied for relation atest1
+WARNING:  no privileges were granted
 -- checks in subquery, both ok
 SELECT * FROM atest1 WHERE ( b IN ( SELECT col1 FROM atest2 ) );
  a | b 
@@ -225,7 +225,7 @@ GRANT USAGE ON LANGUAGE c TO PUBLIC; -- fail
 ERROR:  language "c" is not trusted
 SET SESSION AUTHORIZATION regressuser1;
 GRANT USAGE ON LANGUAGE sql TO regressuser2; -- fail
-ERROR:  permission denied for language sql
+WARNING:  no privileges were granted
 CREATE FUNCTION testfunc1(int) RETURNS int AS 'select 2 * $1;' LANGUAGE sql;
 CREATE FUNCTION testfunc2(int) RETURNS int AS 'select 3 * $1;' LANGUAGE sql;
 REVOKE ALL ON FUNCTION testfunc1(int), testfunc2(int) FROM PUBLIC;
@@ -550,7 +550,7 @@ ERROR:  grant options can only be granted to individual users
 SET SESSION AUTHORIZATION regressuser2;
 GRANT SELECT ON atest4 TO regressuser3;
 GRANT UPDATE ON atest4 TO regressuser3; -- fail
-ERROR:  permission denied for relation atest4
+WARNING:  no privileges were granted
 SET SESSION AUTHORIZATION regressuser1;
 REVOKE SELECT ON atest4 FROM regressuser3; -- does nothing
 SELECT has_table_privilege('regressuser3', 'atest4', 'SELECT'); -- true