I was apply this test (#296485: TestingParty08: comments as replies to other comments) during the BADcamp testing party and noticed a bug. It seemed that threading was not growing as it should and was also resulting in php reporting a bad offset in comment.module:732

After digging around a bit, it seems the mysql query was only ever grabbing the parent's thread, and not the maximum string in the thread, so it never incremented the post count. The problem was in sql query that grabs the MAX(thread), it needed to know it was looking for the maximum in the children of the parent.

originally
...thread LIKE '08.01%'

mysql> SELECT subject, comment, thread FROM comments WHERE nid = 6;
+---------+--------------------+--------+
| subject | comment | thread |
+---------+--------------------+--------+
| c1 | Comment 1 | 01/ |
| c1.c1 | first reply of c1 | 01.01/ |
| c1.c2 | second reply of c1 | 01.01/ |
....
+---------+--------------------+--------+

After patch (which is a period in the right place :)
... thread LIKE '08.01.%'

mysql> SELECT subject, comment, thread FROM comments WHERE nid = 6 and thread LIKE '02%';
+-----------+------------------------+-----------+
| subject | comment | thread |
+-----------+------------------------+-----------+
| Comment 2 | second comment | 02/ |
| c2.c1 | first reply of c2 | 02.00/ |
| c2.c2 | second reply | 02.01/ |
| c2.c1.c1 | first reply of a c2.c1 | 02.00.00/ |
| c2.c1.c2 | second reply of c2.c1 | 02.00.01/ |
+-----------+------------------------+-----------+

Threads correctly. Submitting for review.

CommentFileSizeAuthor
comment.module.threading.d7.patch1000 bytesjonskulski

Comments

damien tournoud’s picture

Status: Needs review » Reviewed & tested by the community

This is a regression introduced by the big DB:TNG patch (this commit). The test for it is #296485: TestingParty08: comments as replies to other comments. Restores the previous behavior, ready to be committed.

dries’s picture

Status: Reviewed & tested by the community » Fixed

This looks good, and it does isolate a problem. Committed to CVS HEAD. Thanks.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.