Hi, Sergey! On Feb 24, Sergey Petrunia wrote:
Hi Varun,
The code for the task is mostly fine. But I would like to question the user experience.
== Ignoring an index causes InnoDB table rebuild ==
First (and foremost), making an index ignored (or not ignored) in an InnoDB table rebuilds the table:
MariaDB [test]> set session alter_algorithm='instant'; alter table t1 alter index c ignore; Query OK, 0 rows affected (0.001 sec)
ERROR 1845 (0A000): ALGORITHM=INSTANT is not supported for this operation. Try ALGORITHM=COPY
This misses the point of the feature. If one needs to rebuild the table anyway, they can just drop (or recreate) the index.
Curiously, it works with MyISAM. I think, the ideal solution would be to make ignoring/not ignoring the index an SQL layer change that doesn't concern the storage engine. If this is not possible, lets make InnoDB aware that it doesn't need to rebuild the table in this case.
Absolutely, it should be a SQL layer change only.
== Backward compatibility for SHOW CREATE TABLE ==
SHOW CREATE TABLE will show something like:
KEY `a` (`a`) IGNORE,
I'd prefer "IGNORED", an adjective.
MySQL here is more backward-compatible and shows:
KEY `a` (`a`) /*!80000 INVISIBLE */,
Do we need to do this as well? I have no strong opinion here.
I don't think we do that much nowadays. I'd say, it's enough to check IGNORED isn't printed in compatibility modes (see sql_mode).
== Use of Ignored index in hints ==
I can use Ignored index in FORCE/IGNORE index hints:
MariaDB [test]> explain select * from t1 force index(a) where a<3 and b<3; +------+-------------+-------+------+---------------+------+---------+------+------+-------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +------+-------------+-------+------+---------------+------+---------+------+------+-------------+ | 1 | SIMPLE | t1 | ALL | NULL | NULL | NULL | NULL | 1000 | Using where | +------+-------------+-------+------+---------------+------+---------+------+------+-------------+
This is allowed. I think, this misses the point of the feature. The point of this feature is to check "what will happen if one drops the index". For this particular query, the effect will be like this:
No, as we discussed, I actually like that force index overrides ignored. I'd suggest to go for the simpler implementation. And if we'll see overwhelming proofs that the behavior is incorrect, then we'll change it. I suppose it means, keep FORCE working for now, don't add extra code to disallow it.
== optimizer_switch flag ==
Seeing "ignore_indexes=on" in optimizer_switch looks scary.
I would say, we don't need an @@optimizer_switch flag. Maybe, Serg disagrees.
In this case I agree, we don't need an optimizer_switch flag. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org