Hi Sergei, Please have a look into the patch for MDEV-7288. It consists of the USER/ROLE related part of the big GSoC patch from Sriram that you reviewed earlier, with review suggestions addressed. You earlier had the following suggestions/questions:

...

diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 5f077c2..1e156be 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc ... @@ -9215,10 +9252,26 @@ bool mysql_create_user(THD *thd, List <LEX_USER> &lis */ if (handle_grant_data(tables, 0, user_name, NULL)) { - append_user(thd, &wrong_users, user_name); - - result= TRUE; - continue; + if (thd->lex->is_create_or_replace()) + { + if (handle_grant_data(tables, 1, user_name, NULL) <= 0) + { + append_user(thd, &wrong_users, user_name);

I don't understand that at all. If user creation failed, you try to drop it, but don't create a user again, after the old one was dropped. How could it work?

Yeah, it looks confusing, because handle_grant_data() can do different things depending on the arguments. The call for handle_grant_data(tables, 0, user_name, NULL) does not create any users. It only checks if the user already exists. The call for handle_grant_data(tables, 1, user_name, NULL) drops the user. And the call for replace_user_table() after this block is the one that actually creates the user. I have added some comments in the new code, to make it clearer.

...

...

@@ -9268,7 +9321,7 @@ bool mysql_create_user(THD *thd, List <LEX_USER> &list, (handle_as_role) ? "CREATE ROLE" : "CREATE USER", wrong_users.c_ptr_safe());

- if (some_users_created) + if (some_users_created || !result)

What is that for? To binlog CREATE IF NOT EXISTS?

Yes, that was the reason. I renamed "some_users_created" to "binlog" and changed the logic slightly to set "binlog" to true under the block checking lex->create_info.if_not_exists(), instead of this additional test for "result". So now it should be easier to read: if (binlog) result |= write_bin_log(thd, FALSE, thd->query(), thd->query_length()); Thanks.