missing indices on comment table

+1 ing this. We have added an index on (pid,status) for drupal.org to speed up several queries across the comments table.

I've been told the query in question is "select count(cid) from comments where pid = X and status = X." If that's the case, I support this index. However, the better performance gain would be from keeping updated counts. MySQL still has to count the rows, even if an index can hand it the rows on a platter.

To put the performance significance of this into a metaphor:
Current status: Counting the yellow M&Ms in a jar with all colors.
Post-index status: Counting the yellow M&Ms with the yellow ones already organized in a separate pile.
Best status: Running tally of M&Ms by color.

I support this...This really speeds some things up.

Marked #101322: comments are searched by not indexed field pid as a duplicate of this issue.

The place this is used in core is comment_num_replies.

I'm wondering if we could get better queries for comment_num_new with nid, pid,

before.

mysql> EXPLAIN SELECT COUNT(cid) FROM comment WHERE pid = 1 AND status = 1;
+----+-------------+---------+-------------+---------------+------------+---------+------+------+-------------------------------------------------------+
| id | select_type | table   | type        | possible_keys | key        | key_len | ref  | rows | Extra                                                 |
+----+-------------+---------+-------------+---------------+------------+---------+------+------+-------------------------------------------------------+
|  1 | SIMPLE      | comment | index_merge | pid,status    | status,pid | 1,4     | NULL |    1 | Using intersect(status,pid); Using where; Using index | 
+----+-------------+---------+-------------+---------------+------------+---------+------+------+-------------------------------------------------------+
1 row in set (0.00 sec)

mysql> EXPLAIN SELECT COUNT(cid) FROM comment WHERE pid = 1 AND status = 1;
+----+-------------+---------+------+------------------------+--------------------+---------+-------------+------+-------------+
| id | select_type | table   | type | possible_keys          | key                | key_len | ref         | rows | Extra       |
+----+-------------+---------+------+------------------------+--------------------+---------+-------------+------+-------------+
|  1 | SIMPLE      | comment | ref  | pid,comment_pid_status | comment_pid_status | 5       | const,const |    2 | Using index | 
+----+-------------+---------+------+------------------------+--------------------+---------+-------------+------+-------------+
1 row in set (0.00 sec)

Looks better. Patch needs to be rolled from root though.

If we're adding a pid, status index, I don't see any reason to keep the separate index on pid.

Don't want to hijack the issue so let me know if I should open a separate one, but comment_num_new looks like it could be simplified a bit as well:

mysql> EXPLAIN SELECT COUNT(c.cid) FROM node n INNER JOIN comment c ON n.nid = c.nid WHERE n.nid = 1 AND timestamp > '123456789' AND c.status = 1;
+----+-------------+-------+-------+---------------+---------+---------+-------+------+-------------+
| id | select_type | table | type  | possible_keys | key     | key_len | ref   | rows | Extra       |
+----+-------------+-------+-------+---------------+---------+---------+-------+------+-------------+
|  1 | SIMPLE      | n     | const | PRIMARY       | PRIMARY | 4       | const |    1 | Using index | 
|  1 | SIMPLE      | c     | ref   | nid,status    | nid     | 4       | const |    2 | Using where | 
+----+-------------+-------+-------+---------------+---------+---------+-------+------+-------------+
2 rows in set (0.03 sec)

mysql> EXPLAIN SELECT COUNT(cid) FROM comment WHERE nid = 1 AND timestamp > 123456789 AND status = 1;
+----+-------------+---------+------+---------------------+-----------------+---------+-------+------+--------------------------+
| id | select_type | table   | type | possible_keys       | key             | key_len | ref   | rows | Extra                    |
+----+-------------+---------+------+---------------------+-----------------+---------+-------+------+--------------------------+
|  1 | SIMPLE      | comment | ref  | nid,comment_num_new | comment_num_new | 4       | const |    2 | Using where; Using index | 
+----+-------------+---------+------+---------------------+-----------------+---------+-------+------+----

wrong patch.

minus the syntax error.

Committed to CVS HEAD.

Moving back to D6 for consideration.

isn't there a small typo in comment_update_7003?
- db_drop_index($rest, 'comment', 'pid');
+ db_drop_index($ret, 'comment', 'pid');

Yes there is.

Thanks!

D6 backport.

#20 - I think it would be comment_update_6006(), no? In the patch it's 7006.

You are correct, sir.

Patch 22 is incorrect - the table name in the patch is "comment" when it should be "comments".

Third time's a charm.