Hi, Daniel!
On Apr 12, Daniel Black wrote:
>
> For review:
>
> Hi All,
>
> list of all changes:
> https://github.com/openquery/mariadb-server/commits/MDEV-6916-maria-5.5-check_view-r4408
>
> side by side comparision of Sanja/my work against 5.5 head:
>
> https://github.com/MariaDB/server/compare/5.5...openquery:MDEV-6916-maria-5.5-check_view-r4408?expand=1
Great, thanks a lot!
Here, few comments are below:
> >> +static int run_mysqlcheck_views(void)
> >> +{
> >> + if (!opt_mysql_upgrade)
> >> + return 0;
> >> + verbose("Phase 0: Fixing views");
> > Again, was in my previous review:
> > ===
> > No "Phase 0" please, renumber all phases to start from 1.
>
> Took opportunity to make adding/changing phases to be lest conflicts in merge
>
> https://github.com/openquery/mariadb-server/commit/25872e2802a4ae32d4f18132d569599295c963cd
I don't know whether it's a good idea.
It means that, for example "Running 'mysql_fix_privilege_tables'..."
won't always be Phase 3/3, but depending on command-line options it
might be a Phase 2 or 1 (well, may be mysql_fix_privilege_tables phase
cannot - but there certainly are phases that are run conditionally).
I personally don't know how bad it is, perhaps it's fine to renumber
phases dynamically when some phases are skipped. So I have no opinion.
I'm just saying that phases might be renumbered dynamically after your
patch.
> > But on the other hand... When mysqlcheck is invoked from mysql_upgrade,
> > you wouldn't want it to repair all tables. Okay, let's keep your
> > implementation and only repair views when --fix-view-algorithm is set.
>
> Not done yet.
>
> --skip-tables or --{update|repair}-views-only perhaps?
>
>
> > In that case, the option should better be --upgrade-views, and defined as
> >
> > --upgrade-views = { no, yes, from_mysql }
> >
> > note that "no" and "yes" must be in that exactly order, first and
> > second.
>
> and the 'from_mysql' means don't check tables? This doesn't sound
> intuitive. More correctly its a --repair-views.
I believe I meant these three variants:
--upgrade-views = no don't change view frm's
--upgrade-views = yes upgrade view frms adding mariadb_version
field and if is_mysql() returned true
also toggle the view algorithm
--upgrade-views = from_mysql always toggle the view algorithm, even
if is_mysql() returned false.
The last case is useful if a user has migrated from MySQL some time ago,
so it's a mariadb-to-mariadb upgrade but views stay wrong from older
mysql-to-mariadb upgrade.
Regards,
Sergei
_______________________________________________
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
