maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Vicențiu!

On Jan 21, Vicențiu Ciorbaru wrote:
> Hi Sergei!
> 
> I agree with most review comments except one:
> 
> > > information.
> > > 
> > > diff --git a/mysql-test/include/switch_to_mysql_user.inc b/mysql-
> > > test/include/switch_to_mysql_user.inc
> > > index f5801db6114..eff1d98c2df 100644
> > > --- a/mysql-test/include/switch_to_mysql_user.inc
> > > +++ b/mysql-test/include/switch_to_mysql_user.inc
> > > @@ -45,6 +45,9 @@ CREATE TABLE mysql.user (
> > >    plugin char(64) CHARACTER SET latin1 DEFAULT '' NOT NULL,
> > >    authentication_string TEXT NOT NULL,
> > >    password_expired ENUM('N','Y') COLLATE utf8_general_ci DEFAULT
> > > 'N' NOT NULL,
> > > +  password_last_changed datetime NULL,
> > > +  password_lifetime int(20) unsigned NULL,
> > > +  account_locked enum('N','Y') COLLATE utf8_general_ci DEFAULT 'N'
> > > NOT NULL,
> > 
> > No-no. mysql.user structure was frozen in time the moment I pushed
> > the
> > commit with the new structure. It doesn't change anymore.
> > 
> > To test 5.7 support, just add ALTER TABLE in one specific test file.
> > See main/system_mysql_db_507.test
> 
> If we are to freeze mysql.user and be as "compatible" as possible , I
> would freeze it with the latest version of 5.7 included, it feels the
> most complete.
> 
> Any other reason that this is a no-no? :)

We aren't to freeze mysql.user to be as "compatible" as possible.
We're to freeze it to never deal with it again. And not to break
existing tools that check it. This table is dead, it's never used
besides some third-party tools that I know almost nothing about.  And
these tools don't need new columns either (but they do need non-empty
password column, we've got complaints when I changed that).

And, anyway, the patch doesn't correct upgrade 10.4.1 mysql.user view
(without account_locked column) to the new mysql.user view (with
account_locked column). But I think that fixing it is a waste of time :)

> As for password_xxx columns, they were added at the same time here so
> as to have the  final format in and simplify logic slightly with
> mysql.user tabular. Your suggestion of handling it via types could work
> even with the password columns I think.
> 
> One could split it into 2 commits, one that just adds columns and
> minimal changes to keep things working and one that adds account
> locking logic. 

One reason to have password expiration related columns in the password
expiration commit it to see that they are those actual columns that the
code needs. And that password expiration code (pushed later) won't
change the structure once again, because it'll need something different.

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx