Page MenuHomePhabricator

Suboptimal query for list=logevents
Closed, DeclinedPublic

Description

If an empty leprop parameter is passed to list=logevents, the query contains many empty sets. For example, this query gives me:

{
    "batchcomplete": "",
    "continue": {
        "lecontinue": "20160924214304|77568834",
        "continue": "-||"
    },
    "query": {
        "logevents": [
            {},
            {},
            {},
            {},
            {},
            {},
            {},
            {},
            {},
            {}
        ]
    }
}

There should either be some omnipresent leprop value(s) or this should be an error. From performance point of view, the database query still joins the logging, user and page tables, even for this trivial query.

Event Timeline

Anomie added a subscriber: Anomie.

There should either be some omnipresent leprop value(s) or this should be an error.

If someone can find a use for this (maybe testing if a specific combination of leaction+letitle has any entries without caring what those entries are?) there's no reason to stop them. If you have no use for it, then just don't do it.

From performance point of view, the database query still joins the logging, user and page tables, even for this trivial query.

Technically, yeah, we could avoid those joins. But I doubt it's worth worrying about in practice, since lacking leprop=ids|user is probably rather rate, both joins are eq_ref, and anyway MariaDB's optimizer is smart enough to optimize them out completely when they're not actually necessary:

mysql:wikiadmin@db1080 [enwiki]> explain SELECT /* ApiQueryLogEvents::execute  */  log_id,log_type,log_action,log_timestamp,log_deleted  FROM `logging` LEFT JOIN `page` ON ((log_namespace=page_namespace) AND (log_title=page_title)) LEFT JOIN `user` ON ((user_id=log_user))   WHERE (log_type != 'suppress')  ORDER BY log_timestamp DESC,log_id DESC LIMIT 11;
+------+-------------+---------+-------+---------------+-------+---------+------+------+-------------+
| id   | select_type | table   | type  | possible_keys | key   | key_len | ref  | rows | Extra       |
+------+-------------+---------+-------+---------------+-------+---------+------+------+-------------+
|    1 | SIMPLE      | logging | index | type_time     | times | 16      | NULL |   18 | Using where |
+------+-------------+---------+-------+---------------+-------+---------+------+------+-------------+
1 row in set (0.00 sec)

So I doubt it's worth the added code complexity.