MDEV-29458: Role grant commands do not propagate all grants

There was an issue in updating in-memory role datastructures when
propagating role grants.

The issue is that changing a particular role's privilege (on any
privilege level, global, database, etc.)
was done such that it overwrote the entire set of bits for that
particular level of privileges.

For example:
grant select on *.* to r1 -> sets the access bits to r1 to select,
regardless of what bits were present for role r1 (inherited from any
other roles).

Before this fix, the rights of role r1 were propagated to any roles r1
was granted to, however the propagated rights did *not* include the
complete rights r1 inherited from its own grants.

For example:
grant r2 to r1;
grant select on *.* to r2;
grant insert on *.* to r1; # This command completely disregards the
                           # select privilege from r2.

In order to correct this, ensure that before rights are propagated
onwards, that the current's role rights have been updated from its
grants.

Additionally, the patch exposed a flaw in the DROP ROLE code.
When deleting a role we removed all its previous grants, but what
remained was the actual links of roles granted to the dropped role.
Having these links present when propagating grants meant that we would
have leftover ACL_xxx entries.

Ensure that the links are removed before propagating grants.

4 files changed, 414 insertions, 102 deletions

diff --git a/mysql-test/suite/roles/recursive_dbug.result b/mysql-test/suite/roles/recursive_dbug.result
index 417602c5c60..6a86df655e7 100644
--- a/mysql-test/suite/roles/recursive_dbug.result
+++ b/mysql-test/suite/roles/recursive_dbug.result
@@ -56,7 +56,7 @@ connection default;
 grant select on *.* to role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	19
+Debug_role_merges_global	20
 Debug_role_merges_db	0
 Debug_role_merges_table	0
 Debug_role_merges_column	0
@@ -106,7 +106,7 @@ connection default;
 revoke select on *.* from role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
+Debug_role_merges_global	29
 Debug_role_merges_db	0
 Debug_role_merges_table	0
 Debug_role_merges_column	0
@@ -124,8 +124,8 @@ connection default;
 grant select on mysql.* to role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	8
+Debug_role_merges_global	29
+Debug_role_merges_db	9
 Debug_role_merges_table	0
 Debug_role_merges_column	0
 Debug_role_merges_routine	0
@@ -164,8 +164,8 @@ connection default;
 revoke select on mysql.* from role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	16
+Debug_role_merges_global	29
+Debug_role_merges_db	17
 Debug_role_merges_table	0
 Debug_role_merges_column	0
 Debug_role_merges_routine	0
@@ -177,9 +177,9 @@ connection default;
 grant select on mysql.roles_mapping to role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	16
-Debug_role_merges_table	8
+Debug_role_merges_global	29
+Debug_role_merges_db	17
+Debug_role_merges_table	9
 Debug_role_merges_column	0
 Debug_role_merges_routine	0
 connection foo;
@@ -217,9 +217,9 @@ connection default;
 revoke select on mysql.roles_mapping from role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	16
-Debug_role_merges_table	16
+Debug_role_merges_global	29
+Debug_role_merges_db	17
+Debug_role_merges_table	17
 Debug_role_merges_column	0
 Debug_role_merges_routine	0
 connection foo;
@@ -230,10 +230,10 @@ connection default;
 grant select(User) on mysql.roles_mapping to role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	16
-Debug_role_merges_table	24
-Debug_role_merges_column	8
+Debug_role_merges_global	29
+Debug_role_merges_db	17
+Debug_role_merges_table	26
+Debug_role_merges_column	9
 Debug_role_merges_routine	0
 connection foo;
 select count(*) from mysql.roles_mapping;
@@ -272,10 +272,10 @@ connection default;
 grant select(Host) on mysql.roles_mapping to role3;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	16
-Debug_role_merges_table	30
-Debug_role_merges_column	14
+Debug_role_merges_global	29
+Debug_role_merges_db	17
+Debug_role_merges_table	33
+Debug_role_merges_column	16
 Debug_role_merges_routine	0
 connection foo;
 select count(concat(User,Host,Role)) from mysql.roles_mapping;
@@ -312,10 +312,10 @@ connection default;
 revoke select(User) on mysql.roles_mapping from role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	16
-Debug_role_merges_table	38
-Debug_role_merges_column	22
+Debug_role_merges_global	29
+Debug_role_merges_db	17
+Debug_role_merges_table	41
+Debug_role_merges_column	24
 Debug_role_merges_routine	0
 connection foo;
 select count(concat(User,Host)) from mysql.roles_mapping;
@@ -327,10 +327,10 @@ connection default;
 revoke select(Host) on mysql.roles_mapping from role3;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	16
-Debug_role_merges_table	44
-Debug_role_merges_column	28
+Debug_role_merges_global	29
+Debug_role_merges_db	17
+Debug_role_merges_table	47
+Debug_role_merges_column	30
 Debug_role_merges_routine	0
 connection foo;
 select count(concat(Host)) from mysql.roles_mapping;
@@ -342,11 +342,11 @@ create function fn1() returns char(10) return "fn1";
 grant execute on procedure test.pr1 to role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	16
-Debug_role_merges_table	44
-Debug_role_merges_column	28
-Debug_role_merges_routine	8
+Debug_role_merges_global	29
+Debug_role_merges_db	17
+Debug_role_merges_table	47
+Debug_role_merges_column	30
+Debug_role_merges_routine	9
 connection foo;
 call pr1();
 ERROR 42000: execute command denied to user 'foo'@'localhost' for routine 'test.pr1'
@@ -360,11 +360,11 @@ connection default;
 grant execute on function test.fn1 to role5;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	16
-Debug_role_merges_table	44
-Debug_role_merges_column	28
-Debug_role_merges_routine	13
+Debug_role_merges_global	29
+Debug_role_merges_db	17
+Debug_role_merges_table	47
+Debug_role_merges_column	30
+Debug_role_merges_routine	15
 connection foo;
 select fn1();
 fn1()
@@ -373,11 +373,11 @@ connection default;
 revoke execute on procedure test.pr1 from role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	16
-Debug_role_merges_table	44
-Debug_role_merges_column	28
-Debug_role_merges_routine	21
+Debug_role_merges_global	29
+Debug_role_merges_db	17
+Debug_role_merges_table	47
+Debug_role_merges_column	30
+Debug_role_merges_routine	23
 connection foo;
 call pr1();
 ERROR 42000: execute command denied to user 'foo'@'localhost' for routine 'test.pr1'
@@ -388,11 +388,11 @@ connection default;
 revoke execute on function test.fn1 from role5;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	16
-Debug_role_merges_table	44
-Debug_role_merges_column	28
-Debug_role_merges_routine	26
+Debug_role_merges_global	29
+Debug_role_merges_db	17
+Debug_role_merges_table	47
+Debug_role_merges_column	30
+Debug_role_merges_routine	28
 connection foo;
 select fn1();
 ERROR 42000: execute command denied to user 'foo'@'localhost' for routine 'test.fn1'
@@ -403,67 +403,67 @@ drop function fn1;
 grant select on mysql.roles_mapping to role3;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	16
-Debug_role_merges_table	50
-Debug_role_merges_column	28
-Debug_role_merges_routine	26
+Debug_role_merges_global	29
+Debug_role_merges_db	17
+Debug_role_merges_table	54
+Debug_role_merges_column	30
+Debug_role_merges_routine	28
 grant select on mysql.roles_mapping to role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	16
-Debug_role_merges_table	53
-Debug_role_merges_column	28
-Debug_role_merges_routine	26
+Debug_role_merges_global	29
+Debug_role_merges_db	17
+Debug_role_merges_table	58
+Debug_role_merges_column	30
+Debug_role_merges_routine	28
 revoke select on mysql.roles_mapping from role3;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	16
-Debug_role_merges_table	54
-Debug_role_merges_column	28
-Debug_role_merges_routine	26
+Debug_role_merges_global	29
+Debug_role_merges_db	17
+Debug_role_merges_table	59
+Debug_role_merges_column	30
+Debug_role_merges_routine	28
 revoke select on mysql.roles_mapping from role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	16
-Debug_role_merges_table	62
-Debug_role_merges_column	28
-Debug_role_merges_routine	26
+Debug_role_merges_global	29
+Debug_role_merges_db	17
+Debug_role_merges_table	67
+Debug_role_merges_column	30
+Debug_role_merges_routine	28
 grant select on mysql.* to role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	24
-Debug_role_merges_table	62
-Debug_role_merges_column	28
-Debug_role_merges_routine	26
+Debug_role_merges_global	29
+Debug_role_merges_db	26
+Debug_role_merges_table	67
+Debug_role_merges_column	30
+Debug_role_merges_routine	28
 grant select on test.* to role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	32
-Debug_role_merges_table	62
-Debug_role_merges_column	28
-Debug_role_merges_routine	26
+Debug_role_merges_global	29
+Debug_role_merges_db	35
+Debug_role_merges_table	67
+Debug_role_merges_column	30
+Debug_role_merges_routine	28
 revoke select on mysql.* from role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	40
-Debug_role_merges_table	62
-Debug_role_merges_column	28
-Debug_role_merges_routine	26
+Debug_role_merges_global	29
+Debug_role_merges_db	43
+Debug_role_merges_table	67
+Debug_role_merges_column	30
+Debug_role_merges_routine	28
 revoke select on test.* from role1;
 show status like 'debug%';
 Variable_name	Value
-Debug_role_merges_global	27
-Debug_role_merges_db	48
-Debug_role_merges_table	62
-Debug_role_merges_column	28
-Debug_role_merges_routine	26
+Debug_role_merges_global	29
+Debug_role_merges_db	51
+Debug_role_merges_table	67
+Debug_role_merges_column	30
+Debug_role_merges_routine	28
 connection default;
 drop user foo@localhost;
 drop role role1;
diff --git a/mysql-test/suite/roles/role_grant_propagate-29458.result b/mysql-test/suite/roles/role_grant_propagate-29458.result
new file mode 100644
index 00000000000..a8ee6e7d987
--- /dev/null
+++ b/mysql-test/suite/roles/role_grant_propagate-29458.result
@@ -0,0 +1,137 @@
+create user foo;
+create database some_db;
+create table some_db.t1 (a int, b int, secret int);
+CREATE PROCEDURE some_db.p1 (OUT param1 INT)
+BEGIN
+SELECT COUNT(*) INTO param1 FROM some_db.t1;
+END;
+//
+CREATE FUNCTION some_db.f1 (param1 INT) RETURNS INT
+BEGIN
+DECLARE c INT;
+SET c = 100;
+RETURN param1 + c;
+END;
+//
+#
+# These roles will form a two level hierarchy.
+# The "select" role will have the select privilege, while
+# the active role will inherit the select role.
+#
+# The active role will be granted a different privilege but on the same
+# level (global, database, table, proc respectively) *after* the 'select'
+# role has been granted its select privilege.
+#
+create role r_select_global;
+create role r_active_global;
+create role r_select_database;
+create role r_active_database;
+create role r_select_table;
+create role r_active_table;
+create role r_select_column;
+create role r_active_column;
+create role r_execute_proc;
+create role r_active_proc;
+create role r_execute_func;
+create role r_active_func;
+grant r_select_global to r_active_global;
+grant r_select_database to r_active_database;
+grant r_select_table to r_active_table;
+grant r_select_column to r_active_column;
+grant r_execute_proc to r_active_proc;
+grant r_execute_func to r_active_func;
+#
+# These 3 roles form a chain, where only the upper level has select
+# privileges and the middle level will receive a grant for the same level
+# privilege, but different kind (for example select on upper and insert
+# on middle).
+#
+# The lower level should inherit both rights.
+#
+create role upper_level;
+create role middle_level;
+create role lower_level;
+grant upper_level to middle_level;
+grant middle_level to lower_level;
+grant r_active_global   to foo;
+grant r_active_database to foo;
+grant r_active_table    to foo;
+grant r_active_column   to foo;
+grant r_active_proc     to foo;
+grant r_active_func     to foo;
+grant lower_level       to foo;
+grant select on *.*        to r_select_global;
+grant select on some_db.*  to r_select_database;
+grant select on some_db.t1 to r_select_table;
+grant select(a) on some_db.t1 to r_select_column;
+grant select on *.*        to upper_level;
+grant execute on procedure some_db.p1 to r_execute_proc;
+grant execute on function  some_db.f1 to r_execute_func;
+#
+# Granting a privilege different than select on the corresponding level.
+# This tests that the base role correctly inherits its granted roles
+# privileges.
+#
+grant insert on *.*        to r_active_global;
+grant insert on some_db.*  to r_active_database;
+grant insert on some_db.t1 to r_active_table;
+grant insert(a) on some_db.t1 to r_active_column;
+grant insert on *.* to middle_level;
+grant alter routine on procedure some_db.p1 to r_active_proc;
+grant alter routine on function some_db.f1 to r_active_func;
+flush privileges;
+connect  con1, localhost, foo,,;
+select * from some_db.t1;
+ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1'
+#
+# Before MDEV-29458 fix, all these commands would return
+# ER_TABLEACCESS_DENIED_ERROR
+#
+set role r_active_global;
+select * from some_db.t1;
+a	b	secret
+set role r_active_database;
+select * from some_db.t1;
+a	b	secret
+set role r_active_table;
+select * from some_db.t1;
+a	b	secret
+set role r_active_column;
+select a from some_db.t1;
+a
+set role lower_level;
+select * from some_db.t1;
+a	b	secret
+set role r_active_proc;
+set @var=100;
+call some_db.p1(@var);
+set role r_active_func;
+select some_db.f1(10);
+some_db.f1(10)
+110
+disconnect con1;
+#
+# Cleanup.
+#
+connection default;
+drop database some_db;
+drop role r_select_global, r_select_database, r_select_table, r_select_column;
+drop role r_active_global, r_active_database, r_active_table, r_active_column;
+drop role r_execute_proc, r_execute_func;
+drop role r_active_proc, r_active_func;
+drop role upper_level, middle_level, lower_level;
+drop user foo;
+#
+# Test that dropping of roles clears the intermediate generated
+# (such as an `acl_dbs` element with 0 init_access, but with access != 0)
+# datastructures.
+#
+create role test_role1;
+create role test_role2;
+grant test_role2 to test_role1;
+grant select on mysql.* to test_role2;
+grant select on mysql.user to test_role2;
+grant select(user) on mysql.user to test_role2;
+drop role test_role1, test_role2;
+create role test_role1;
+drop role test_role1;
diff --git a/mysql-test/suite/roles/role_grant_propagate-29458.test b/mysql-test/suite/roles/role_grant_propagate-29458.test
new file mode 100644
index 00000000000..07c29a3800c
--- /dev/null
+++ b/mysql-test/suite/roles/role_grant_propagate-29458.test
@@ -0,0 +1,164 @@
+--source include/not_embedded.inc
+
+create user foo;
+create database some_db;
+create table some_db.t1 (a int, b int, secret int);
+
+delimiter //;
+CREATE PROCEDURE some_db.p1 (OUT param1 INT)
+ BEGIN
+  SELECT COUNT(*) INTO param1 FROM some_db.t1;
+ END;
+//
+delimiter ;//
+
+delimiter //;
+CREATE FUNCTION some_db.f1 (param1 INT) RETURNS INT
+ BEGIN
+  DECLARE c INT;
+  SET c = 100;
+  RETURN param1 + c;
+ END;
+//
+delimiter ;//
+
+--echo #
+--echo # These roles will form a two level hierarchy.
+--echo # The "select" role will have the select privilege, while
+--echo # the active role will inherit the select role.
+--echo #
+--echo # The active role will be granted a different privilege but on the same
+--echo # level (global, database, table, proc respectively) *after* the 'select'
+--echo # role has been granted its select privilege.
+--echo #
+
+create role r_select_global;
+create role r_active_global;
+
+create role r_select_database;
+create role r_active_database;
+
+create role r_select_table;
+create role r_active_table;
+
+create role r_select_column;
+create role r_active_column;
+
+create role r_execute_proc;
+create role r_active_proc;
+
+create role r_execute_func;
+create role r_active_func;
+
+grant r_select_global to r_active_global;
+grant r_select_database to r_active_database;
+grant r_select_table to r_active_table;
+grant r_select_column to r_active_column;
+grant r_execute_proc to r_active_proc;
+grant r_execute_func to r_active_func;
+
+--echo #
+--echo # These 3 roles form a chain, where only the upper level has select
+--echo # privileges and the middle level will receive a grant for the same level
+--echo # privilege, but different kind (for example select on upper and insert
+--echo # on middle).
+--echo #
+--echo # The lower level should inherit both rights.
+--echo #
+create role upper_level;
+create role middle_level;
+create role lower_level;
+
+grant upper_level to middle_level;
+grant middle_level to lower_level;
+
+grant r_active_global   to foo;
+grant r_active_database to foo;
+grant r_active_table    to foo;
+grant r_active_column   to foo;
+grant r_active_proc     to foo;
+grant r_active_func     to foo;
+grant lower_level       to foo;
+
+grant select on *.*        to r_select_global;
+grant select on some_db.*  to r_select_database;
+grant select on some_db.t1 to r_select_table;
+grant select(a) on some_db.t1 to r_select_column;
+grant select on *.*        to upper_level;
+
+grant execute on procedure some_db.p1 to r_execute_proc;
+grant execute on function  some_db.f1 to r_execute_func;
+
+
+--echo #
+--echo # Granting a privilege different than select on the corresponding level.
+--echo # This tests that the base role correctly inherits its granted roles
+--echo # privileges.
+--echo #
+grant insert on *.*        to r_active_global;
+grant insert on some_db.*  to r_active_database;
+grant insert on some_db.t1 to r_active_table;
+grant insert(a) on some_db.t1 to r_active_column;
+grant insert on *.* to middle_level;
+
+grant alter routine on procedure some_db.p1 to r_active_proc;
+grant alter routine on function some_db.f1 to r_active_func;
+flush privileges;
+
+--connect (con1, localhost, foo,,)
+--error ER_TABLEACCESS_DENIED_ERROR
+select * from some_db.t1;
+
+--echo #
+--echo # Before MDEV-29458 fix, all these commands would return
+--echo # ER_TABLEACCESS_DENIED_ERROR
+--echo #
+set role r_active_global;
+select * from some_db.t1;
+set role r_active_database;
+select * from some_db.t1;
+set role r_active_table;
+select * from some_db.t1;
+set role r_active_column;
+select a from some_db.t1;
+set role lower_level;
+select * from some_db.t1;
+
+set role r_active_proc;
+set @var=100;
+call some_db.p1(@var);
+
+set role r_active_func;
+select some_db.f1(10);
+
+disconnect con1;
+
+--echo #
+--echo # Cleanup.
+--echo #
+connection default;
+
+drop database some_db;
+drop role r_select_global, r_select_database, r_select_table, r_select_column;
+drop role r_active_global, r_active_database, r_active_table, r_active_column;
+drop role r_execute_proc, r_execute_func;
+drop role r_active_proc, r_active_func;
+drop role upper_level, middle_level, lower_level;
+drop user foo;
+
+--echo #
+--echo # Test that dropping of roles clears the intermediate generated
+--echo # (such as an `acl_dbs` element with 0 init_access, but with access != 0)
+--echo # datastructures.
+--echo #
+create role test_role1;
+create role test_role2;
+
+grant test_role2 to test_role1;
+grant select on mysql.* to test_role2;
+grant select on mysql.user to test_role2;
+grant select(user) on mysql.user to test_role2;
+drop role test_role1, test_role2;
+
+create role test_role1;
+drop role test_role1;
diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
index a92da88abba..ecf78f0e049 100644
--- a/sql/sql_acl.cc
+++ b/sql/sql_acl.cc
@@ -8299,6 +8299,7 @@ static int count_subgraph_nodes(ACL_ROLE *role, ACL_ROLE *grantee, void *context
 }
 
 static int merge_role_privileges(ACL_ROLE *, ACL_ROLE *, void *);
+static bool merge_one_role_privileges(ACL_ROLE *grantee, PRIVS_TO_MERGE what);
 
 /**
   rebuild privileges of all affected roles
@@ -8318,6 +8319,11 @@ static void propagate_role_grants(ACL_ROLE *role,
   PRIVS_TO_MERGE data= { what, db, name };
 
   /*
+     Before updating grants to roles that inherit from this role, ensure that
+     the effective grants on this role are up-to-date from *its* granted roles.
+  */
+  merge_one_role_privileges(role, data);
+  /*
      Changing privileges of a role causes all other roles that had
      this role granted to them to have their rights invalidated.
 
@@ -8765,7 +8771,6 @@ static int table_name_sort(GRANT_TABLE * const *tbl1, GRANT_TABLE * const *tbl2)
 */
 static int update_role_columns(GRANT_TABLE *merged,
                                GRANT_TABLE **cur, GRANT_TABLE **last)
-
 {
   privilege_t rights __attribute__((unused)) (NO_ACL);
   int changed= 0;
@@ -9135,11 +9140,12 @@ static int merge_role_privileges(ACL_ROLE *role __attribute__((unused)),
   return !changed; // don't recurse into the subgraph if privs didn't change
 }
 
-static bool merge_one_role_privileges(ACL_ROLE *grantee)
+static
+bool merge_one_role_privileges(ACL_ROLE *grantee,
+                               PRIVS_TO_MERGE what)
 {
-  PRIVS_TO_MERGE data= { PRIVS_TO_MERGE::ALL, 0, 0 };
   grantee->counter= 1;
-  return merge_role_privileges(0, grantee, &data);
+  return merge_role_privileges(0, grantee, &what);
 }
 
 /*****************************************************************
@@ -9329,15 +9335,15 @@ static int mysql_table_grant(THD *thd, TABLE_LIST *table_list,
 
     /* Find/create cached table grant */
     grant_table= table_hash_search(Str->host.str, NullS, db_name,
-				   Str->user.str, table_name, 1);
+                                   Str->user.str, table_name, 1);
     if (!grant_table)
     {
       if (revoke_grant)
       {
-	my_error(ER_NONEXISTING_TABLE_GRANT, MYF(0),
+        my_error(ER_NONEXISTING_TABLE_GRANT, MYF(0),
                  Str->user.str, Str->host.str, table_list->table_name.str);
-	result= TRUE;
-	continue;
+        result= TRUE;
+        continue;
       }
       grant_table= new (&grant_memroot) GRANT_TABLE(Str->host.str, db_name,
                                                     Str->user.str, table_name,
@@ -9346,8 +9352,8 @@ static int mysql_table_grant(THD *thd, TABLE_LIST *table_list,
       if (!grant_table ||
           column_priv_insert(grant_table))
       {
-	result= TRUE;				/* purecov: deadcode */
-	continue;				/* purecov: deadcode */
+        result= TRUE;				/* purecov: deadcode */
+        continue;				/* purecov: deadcode */
       }
     }
 
@@ -9849,7 +9855,7 @@ bool mysql_grant_role(THD *thd, List <LEX_USER> &list, bool revoke)
        Only need to propagate grants when granting/revoking a role to/from
        a role
     */
-    if (role_as_user && merge_one_role_privileges(role_as_user) == 0)
+    if (role_as_user)
       propagate_role_grants(role_as_user, PRIVS_TO_MERGE::ALL);
   }
 
@@ -12893,9 +12899,6 @@ static int handle_grant_struct(enum enum_acl_lists struct_no, bool drop,
     size_t old_key_length= acl_role->user.length;
     if (drop)
     {
-      /* all grants must be revoked from this role by now. propagate this */
-      propagate_role_grants(acl_role, PRIVS_TO_MERGE::ALL);
-
       // delete the role from cross-reference arrays
       for (size_t i=0; i < acl_role->role_grants.elements; i++)
       {
@@ -12911,6 +12914,14 @@ static int handle_grant_struct(enum enum_acl_lists struct_no, bool drop,
         remove_ptr_from_dynarray(&grantee->role_grants, acl_role);
       }
 
+      /* Remove all of the role_grants from this user. */
+      while (acl_role->role_grants.elements)
+        delete_dynamic_element(&acl_role->role_grants,
+                               acl_role->role_grants.elements - 1);
+
+      /* all grants must be revoked from this role by now. propagate this */
+      propagate_role_grants(acl_role, PRIVS_TO_MERGE::ALL);
+
       my_hash_delete(&acl_roles, (uchar*) acl_role);
       DBUG_RETURN(1);
     }