maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Vicențiu!

Next time, please, cc: (or to: ) your commit to commits@xxxxxxxxxxx.
Or use the post-commit trigger from

http://bazaar.launchpad.net/~maria-captains/mariadb-tools/trunk/files/head:/git_template/

> commit 4ede7cdbe51332d1b07def8830974877ebcee936
> Author: Vicențiu Ciorbaru <cvicentiu@xxxxxxxxx>
> Date:   Wed Feb 4 23:08:05 2015 +0000
> 
>     MDEV-6918 Create a way to see a user's default role.
>     
>     Added an extra column to i_s_applicable_roles, named IS_DEFAULT.
>     The column displays which role is the default role for the user
>     querying the table.

Looks good, thanks!
A couple of minor comments, see below.
Ok to push after that.

> diff --git a/mysql-test/suite/roles/i_s_applicable_roles_is_default.test b/mysql-test/suite/roles/i_s_applicable_roles_is_default.test
> new file mode 100644
> index 0000000..1782f7a
> --- /dev/null
> +++ b/mysql-test/suite/roles/i_s_applicable_roles_is_default.test
> @@ -0,0 +1,61 @@

I'd suggest to start the test with

--enable_connect_log

it'll make the result file a bit easier to follow.

> +create user foo;
> +create role role1;
> +create role role2;
> +create role role3;
> +
> diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
> index 96fff6d..45ce43d 100644
> --- a/sql/sql_acl.cc
> +++ b/sql/sql_acl.cc
> @@ -10349,6 +10349,17 @@ applicable_roles_insert(ACL_USER_BASE *grantee, ACL_ROLE *role, void *ptr)
>    else
>      table->field[2]->store(STRING_WITH_LEN("NO"), cs);
>  
> +  /* Default role is only valid when looking at a role granted to a user. */
> +  if (!is_role)
> +  {
> +    if (!is_role && data->user->default_rolename.length &&

you don't need a second !is_role under if(!is_role).

I suppose that's a remnant of the first version of your patch, when you
returned NO for roles granted to roles :)

> +        !strcmp(data->user->default_rolename.str, role->user.str))
> +      table->field[3]->store(STRING_WITH_LEN("YES"), cs);
> +    else
> +      table->field[3]->store(STRING_WITH_LEN("NO"), cs);
> +    table->field[3]->set_notnull();
> +  }
> +
>    if (schema_table_store_record(table->in_use, table))
>      return -1;
>    return 0;

Regards,
Sergei