Hi Sergei!
Thanks for the review. I'll wait for buildbot to test it out and I'll pushj
afterwards.
On Mon, 13 Feb 2017 at 17:25 Sergei Golubchik <s...@mariadb.org> wrote:
> Hi, Vicentiu!
>
> Looks fine, thanks. Few comments below.
>
> On Feb 13, vicentiu wrote:
> > revision-id: 822aa21672ba6650a31a3ec05ab5c1bce7a80c1e
> (mariadb-10.2.3-198-g822aa21672b)
> > parent(s): b745ea489f0698c328383d9b8ec20d2f9777b1b8
> > author: Vicențiu Ciorbaru
> > committer: Vicențiu Ciorbaru
> > timestamp: 2017-02-13 14:44:00 +0200
> > message:
> >
> > MDEV-11170: MariaDB 10.2 cannot start on MySQL 5.7 datadir
> >
> > PART 2 of the fix adds the logic of not using password column, unless it
> > exists. If password column is missing we attempt to use plugin &&
> > authentication_string columns.
> >
> > diff --git a/mysql-test/r/no_password_column-mdev-11170.result
> b/mysql-test/r/no_password_column-mdev-11170.result
> > new file mode 100644
> > index 00000000000..c3920a1b530
> > --- /dev/null
> > +++ b/mysql-test/r/no_password_column-mdev-11170.result
> > @@ -0,0 +1,170 @@
> ...
> > +# Reset to final original state.
> > +#
> > +drop table mysql.user;
> > +create table mysql.user like backup_user;
> > +insert into mysql.user select * from backup_user;
>
> you could RENAME TABLE backup_user to mysql.user.
>
Done.
> > +drop table backup_user;
> > +flush privileges;
> > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
> > index 568aff9ac92..7e03253ca98 100644
> > --- a/sql/sql_acl.cc
> > +++ b/sql/sql_acl.cc
> > @@ -2071,7 +2103,7 @@ static bool acl_load(THD *thd, const Grant_tables&
> tables)
> > 8, 8, MYF(0));
> >
> > /* check default role, if any */
> > - if (!is_role && user_table.num_fields() > DEFAULT_ROLE_COLUMN_IDX)
> > + if (!is_role && user_table.default_role())
>
> all these changes could've went into the first patch...
>
The reason why this patch contains those extra checks is that I wanted to
mess as little as
possible with the current logic in the first patch, and only introduce the
Grant_table class.
> > {
> > user.default_rolename.str=
> > get_field(&acl_memroot, user_table.default_role());
> > @@ -4030,11 +4092,16 @@ static int replace_user_table(THD *thd, const
> User_table &user_table,
> >
> > rights= user_table.get_access();
> >
> > - DBUG_PRINT("info",("table fields: %d",user_table.num_fields()));
> > - if (combo.pwhash.str[0])
> > + DBUG_PRINT("info",("table fields: %d", user_table.num_fields()));
> > + /* If we don't have a password column, we'll use the
> authentication_string
> > + column later. */
> > + if (combo.pwhash.str[0] && user_table.password())
> > user_table.password()->store(combo.pwhash.str, combo.pwhash.length,
> > system_charset_info);
> > - if (user_table.num_fields() >= 31) /* From 4.0.0 we have more
> fields */
> > + /* We either have the password column, the plugin column, or both.
> Otherwise
> > + we have a corrupt user table. */
> > + DBUG_ASSERT(user_table.password() || user_table.plugin());
>
> you cannot assert the table structure. ALTER TABLE shouldn't crash the
> server.
>
Kept it like this, as discussed.
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
