Missing index menu_router tab_root

subscribe

Bumping to D7.

The index should be on (tab_root, weight, title). This will eliminate the filesort, as well.

@Damien

Still a big issue on 6.x sites - menu_local_tasks calls a query on every page that for me took 500ms or so before the index, less than 5 after.

@stormsweeper: we fix issues in the current development version (Drupal 7), before backporting them.

19:44 < chx> killes: you must not change tab_root without changing path and menu_link router_path

19:56 < chx> killes: paths => 128, title => 200.

You can specify the number of characters the index should use - you can use the first 64 or 128 out of a 255 characters field. I don't think limiting the length in the database is the right way to do it.

Re-rolled with limited index length as FiReaNG3L suggested.

I would rather limit the size of the title part. In any case, we need to validate that MySQL can do something with this partial index.

Here's some explains from Postgres on a 6.x database, if it's helpful. No real difference between just the index on tab_root and the additional columns; the latter just gets used the same as if it were only on the single column.

drupalpgsql=> select count(*) from menu_router;
 count 
-------
   413
(1 row)

drupalpgsql=> explain analyze SELECT * FROM menu_router WHERE tab_root = 'profile' ORDER BY weight, title;
                                                  QUERY PLAN                                                  
--------------------------------------------------------------------------------------------------------------
 Sort  (cost=28.17..28.18 rows=1 width=375) (actual time=0.264..0.264 rows=0 loops=1)
   Sort Key: weight, title
   ->  Seq Scan on menu_router  (cost=0.00..28.16 rows=1 width=375) (actual time=0.255..0.255 rows=0 loops=1)
         Filter: ((tab_root)::text = 'profile'::text)
 Total runtime: 0.308 ms
(5 rows)

drupalpgsql=> create index menu_router_tab_root_idx on menu_router (tab_root);
CREATE INDEX
drupalpgsql=> explain analyze SELECT * FROM menu_router WHERE tab_root = 'profile' ORDER BY weight, title;
                                                                  QUERY PLAN                                                                  
----------------------------------------------------------------------------------------------------------------------------------------------
 Sort  (cost=8.28..8.28 rows=1 width=375) (actual time=0.062..0.062 rows=0 loops=1)
   Sort Key: weight, title
   ->  Index Scan using menu_router_tab_root_idx on menu_router  (cost=0.00..8.27 rows=1 width=375) (actual time=0.051..0.051 rows=0 loops=1)
         Index Cond: ((tab_root)::text = 'profile'::text)
 Total runtime: 0.098 ms
(5 rows)

drupalpgsql=> drop index menu_router_tab_root_idx;DROP INDEX
drupalpgsql=> create index menu_router_tab_root_idx on menu_router (tab_root, weight, substr(title,64));
CREATE INDEX
drupalpgsql=> explain analyze SELECT * FROM menu_router WHERE tab_root = 'profile' ORDER BY weight, title;
                                                                  QUERY PLAN                                                                  
----------------------------------------------------------------------------------------------------------------------------------------------
 Sort  (cost=8.28..8.28 rows=1 width=375) (actual time=0.066..0.066 rows=0 loops=1)
   Sort Key: weight, title
   ->  Index Scan using menu_router_tab_root_idx on menu_router  (cost=0.00..8.27 rows=1 width=375) (actual time=0.058..0.058 rows=0 loops=1)
         Index Cond: ((tab_root)::text = 'profile'::text)
 Total runtime: 0.099 ms
(5 rows)

@stormsweeper
The syntax of substr is substr(string, from [, count]), thus the index you created has no effect. Not sure, though, whether a correct index helps Postgres in any way.

@smk-ka
Sorry, you're correct. You're right, though, it doesn't make much difference. Postgres doesn't seem to care for substring indexes in any case - you'll note that only the whole column index is uses for the order by.

drupalpgsql=> create index menu_router_tab_root_idx on menu_router (tab_root, weight, substr(title,1,64));
CREATE INDEX

drupalpgsql=> explain analyze SELECT * FROM menu_router WHERE tab_root = 'profile' ORDER BY weight, title;
                                                                  QUERY PLAN                                                                  
----------------------------------------------------------------------------------------------------------------------------------------------
 Sort  (cost=8.28..8.28 rows=1 width=375) (actual time=0.033..0.033 rows=0 loops=1)
   Sort Key: weight, title
   ->  Index Scan using menu_router_tab_root_idx on menu_router  (cost=0.00..8.27 rows=1 width=375) (actual time=0.019..0.019 rows=0 loops=1)
         Index Cond: ((tab_root)::text = 'profile'::text)
 Total runtime: 0.078 ms
(5 rows)

drupalpgsql=> drop index menu_router_tab_root_idx;
DROP INDEX

drupalpgsql=> create index menu_router_tab_root_idx on menu_router (tab_root, weight, title);
CREATE INDEX

drupalpgsql=> explain analyze SELECT * FROM menu_router WHERE tab_root = 'profile' ORDER BY weight, title;
                                                               QUERY PLAN                                                               
----------------------------------------------------------------------------------------------------------------------------------------
 Index Scan using menu_router_tab_root_idx on menu_router  (cost=0.00..8.27 rows=1 width=375) (actual time=0.040..0.040 rows=0 loops=1)
   Index Cond: ((tab_root)::text = 'profile'::text)
 Total runtime: 0.075 ms
(3 rows)

The previous patch failed because other patches increased the numbers in system.install.

trying again.

Bingo?

Can someone run the EXPLAIN on this patch? Then it should be rtbc.

Re-rolled for system_update conflict, here's the EXPLAIN I asked for, RTBC.

Before:

mysql> DESCRIBE SELECT menu_router.* FROM menu_router menu_router WHERE (tab_root = 'node') ORDER BY weight ASC, title ASC;
+----+-------------+-------------+------+---------------+------+---------+------+------+-----------------------------+
| id | select_type | table       | type | possible_keys | key  | key_len | ref  | rows | Extra                       |
+----+-------------+-------------+------+---------------+------+---------+------+------+-----------------------------+
|  1 | SIMPLE      | menu_router | ALL  | NULL          | NULL | NULL    | NULL |  210 | Using where; Using filesort | 
+----+-------------+-------------+------+---------------+------+---------+------+------+-----------------------------+
1 row in set (0.02 sec)

After:

mysql> DESCRIBE SELECT menu_router.* FROM menu_router menu_router WHERE (tab_root = 'node') ORDER BY weight ASC, title ASC;
+----+-------------+-------------+------+-----------------------+-----------------------+---------+-------+------+-------------+
| id | select_type | table       | type | possible_keys         | key                   | key_len | ref   | rows | Extra       |
+----+-------------+-------------+------+-----------------------+-----------------------+---------+-------+------+-------------+
|  1 | SIMPLE      | menu_router | ref  | tab_root_weight_title | tab_root_weight_title | 194     | const |    1 | Using where | 
+----+-------------+-------------+------+-----------------------+-----------------------+---------+-------+------+-------------+

Here is a re-roll for D6, including the explain.

BEFORE:

mysql> DESCRIBE SELECT mr.* FROM drupal_menu_router mr WHERE (tab_root = 'node') ORDER BY weight ASC, title ASC;
+----+-------------+-------+------+---------------+------+---------+------+------+-----------------------------+
| id | select_type | table | type | possible_keys | key  | key_len | ref  | rows | Extra                       |
+----+-------------+-------+------+---------------+------+---------+------+------+-----------------------------+
|  1 | SIMPLE      | mr    | ALL  | NULL          | NULL | NULL    | NULL |  898 | Using where; Using filesort | 
+----+-------------+-------+------+---------------+------+---------+------+------+-----------------------------+
1 row in set (0.00 sec)

AFTER:

mysql> DESCRIBE SELECT mr.* FROM drupal_menu_router mr WHERE (tab_root = 'node') ORDER BY weight ASC, title ASC;
+----+-------------+-------+------+-----------------------+-----------------------+---------+-------+------+-------------+
| id | select_type | table | type | possible_keys         | key                   | key_len | ref   | rows | Extra       |
+----+-------------+-------+------+-----------------------+-----------------------+---------+-------+------+-------------+
|  1 | SIMPLE      | mr    | ref  | tab_root_weight_title | tab_root_weight_title | 194     | const |    1 | Using where | 
+----+-------------+-------+------+-----------------------+-----------------------+---------+-------+------+-------------+
1 row in set (0.00 sec)

Committed to HEAD.

Hm. I'm not sure if this means the 6.x patch needs to be re-rolled to *remove* the update hook I just committed. This is always very confusing. :\ I'll leave it RTBC and Gábor will know what to do.

Committed to Drupal 6 too. Moving back to 7.x to fix the update function. (Marking critical so it does not slip without being fixed before the release).

Patch.

Catch, did you forget to upload a patch?

I did.

In addition to this patch, we need to document (if not already present) that sites should be on the very latest 6.x release before upgrading to 7. If they don't do that, they can miss an index like this one. I don't think we want to support upgrading from any 6.x release to 7.x. Thats too many use cases to test for core and for contrib.

@moshe - since we keep 6.x-7.x extra in, it should be technically possible to upgrade 6.0-7.x.

Personally I think we should only support versions of 6.x available during or after the first 7.x RC (then support everything after that) because the overhead for committing updates to both releases is very high at the moment, and there should be few schema changes to both versions past that point anyway, but that's not the consensus according to #278592: Sync 6.x extra updates with HEAD since I was apparently the only person on that issue with that view, and we'll need a new issue to change it.

Feels a little bit odd, to me, but asking people to upgrade to the latest D6 version is probably the right thing to do anyway.

Committed to CVS HEAD.

Perhaps we should put the index back in with a conditional index check? It is now possible as #360854: db_index_exists() missing, module updates cannot handle indexes properly was just committed. I still think we only have enough resources to test the upgrade path from one D6 release (the latest one).

It's in 7 though, as http://api.drupal.org/api/function/system_update_6052/7

Hi,

I am seeing database upgrade errors on my site's update completion page and the same page also prompted go to Administration pages as well.

Update #6052
Failed: ALTER TABLE {menu_router} ADD INDEX tab_root_weight_title (tab_root(64), weight, title)

Update #6015
Failed: CREATE TABLE {cache_form} ( cid varchar(255) NOT NULL default '', data longblob, expire int NOT NULL default '0', created int NOT NULL default '0', headers text, serialized int(1) NOT NULL default '0', PRIMARY KEY (cid), INDEX expire (expire) ) /*!40100 DEFAULT CHARACTER SET UTF8 */

Update #6020
Failed: CREATE TABLE {menu_router} ( `path` VARCHAR(255) NOT NULL DEFAULT '', `load_functions` VARCHAR(255) NOT NULL DEFAULT '', `to_arg_functions` VARCHAR(255) NOT NULL DEFAULT '', `access_callback` VARCHAR(255) NOT NULL DEFAULT '', `access_arguments` TEXT DEFAULT NULL, `page_callback` VARCHAR(255) NOT NULL DEFAULT '', `page_arguments` TEXT DEFAULT NULL, `fit` INT NOT NULL DEFAULT 0, `number_parts` SMALLINT NOT NULL DEFAULT 0, `tab_parent` VARCHAR(255) NOT NULL DEFAULT '', `tab_root` VARCHAR(255) NOT NULL DEFAULT '', `title` VARCHAR(255) NOT NULL DEFAULT '', `title_callback` VARCHAR(255) NOT NULL DEFAULT '', `title_arguments` VARCHAR(255) NOT NULL DEFAULT '', `type` INT NOT NULL DEFAULT 0, `block_callback` VARCHAR(255) NOT NULL DEFAULT '', `description` TEXT NOT NULL, `position` VARCHAR(255) NOT NULL DEFAULT '', `weight` INT NOT NULL DEFAULT 0, `file` MEDIUMTEXT DEFAULT NULL, PRIMARY KEY (path), INDEX fit (fit), INDEX tab_parent (tab_parent) ) /*!40100 DEFAULT CHARACTER SET UTF8 */
Failed: CREATE TABLE {menu_links} ( `menu_name` VARCHAR(32) NOT NULL DEFAULT '', `mlid` INT unsigned NOT NULL auto_increment, `plid` INT unsigned NOT NULL DEFAULT 0, `link_path` VARCHAR(255) NOT NULL DEFAULT '', `router_path` VARCHAR(255) NOT NULL DEFAULT '', `link_title` VARCHAR(255) NOT NULL DEFAULT '', `options` TEXT DEFAULT NULL, `module` VARCHAR(255) NOT NULL DEFAULT 'system', `hidden` SMALLINT NOT NULL DEFAULT 0, `external` SMALLINT NOT NULL DEFAULT 0, `has_children` SMALLINT NOT NULL DEFAULT 0, `expanded` SMALLINT NOT NULL DEFAULT 0, `weight` INT NOT NULL DEFAULT 0, `depth` SMALLINT NOT NULL DEFAULT 0, `customized` SMALLINT NOT NULL DEFAULT 0, `p1` INT unsigned NOT NULL DEFAULT 0, `p2` INT unsigned NOT NULL DEFAULT 0, `p3` INT unsigned NOT NULL DEFAULT 0, `p4` INT unsigned NOT NULL DEFAULT 0, `p5` INT unsigned NOT NULL DEFAULT 0, `p6` INT unsigned NOT NULL DEFAULT 0, `p7` INT unsigned NOT NULL DEFAULT 0, `p8` INT unsigned NOT NULL DEFAULT 0, `p9` INT unsigned NOT NULL DEFAULT 0, `updated` SMALLINT NOT NULL DEFAULT 0, PRIMARY KEY (mlid), INDEX path_menu (link_path(128), menu_name), INDEX menu_plid_expand_child (menu_name, plid, expanded, has_children), INDEX menu_parents (menu_name, p1, p2, p3, p4, p5, p6, p7, p8, p9), INDEX router_path (router_path(128)) ) /*!40100 DEFAULT CHARACTER SET UTF8 */

Update #6043
ALTER TABLE {flood} ADD INDEX allow (event, hostname, timestamp)
ALTER TABLE {history} ADD INDEX nid (nid)
ALTER TABLE {blocks} CHANGE `theme` `theme` VARCHAR(64) NOT NULL DEFAULT '', ADD UNIQUE KEY tmd (theme, module, delta), ADD INDEX list (theme, status, region, weight, module)
ALTER TABLE {blocks_roles} ADD INDEX rid (rid)
ALTER TABLE {filters} DROP INDEX weight
Failed: ALTER TABLE {filters} ADD UNIQUE KEY fmd (format, module, delta)
ALTER TABLE {filters} ADD INDEX list (format, weight, module, delta)

ALTER TABLE {profile_values} CHANGE `uid` `uid` INT unsigned NOT NULL DEFAULT 0
Failed: ALTER TABLE {profile_values} ADD PRIMARY KEY (uid, fid)
ALTER TABLE {accesslog} ADD INDEX uid (uid)

Update #6052
Failed: ALTER TABLE {menu_router} ADD INDEX tab_root_weight_title (tab_root(64), weight, title)

Could you let me know whether applying the above patch should fix the above failures and also I could not find the link to download this patch.

thanks
~Paradesi

added code tags