Hi, I have generated a lot of comments (over 10000 on a single node) with the devel module to have a testing environment. When doing this, I ran over something that seems to be an incompatible SQL query in comment.module.

I am using Drupal 6.1 with PHP 5.2.5 on an Apache 2 Webserver and a PostgreSQL 8.3 DB.

The error message is:

* warning: pg_query() [function.pg-query]: Query failed: FEHLER: mehrere ORDER-BY-Klauseln sind nicht erlaubt in includes/database.pgsql.inc on line 138.
* user warning: query: (SELECT thread FROM drupal_comments WHERE nid = 49 AND status = 0 ORDER BY timestamp DESC LIMIT 500) ORDER BY thread DESC LIMIT 1 in modules/comment/comment.module on line 365.

I don't understand why the query is using parentheses and different LIMIT values - my proposal would be to use something like "SELECT thread FROM {comments} WHERE nid = %d AND status = 0 ORDER BY timestamp DESC, thread DESC LIMIT 1".

I'd be glad to provide more information if needed.

CommentFileSizeAuthor
#1 comment_sql_cleanup.patch1.91 KBpenguinchix0r

Comments

penguinchix0r’s picture

penguinchix0r commented
StatusFileSize
new comment_sql_cleanup.patch1.91 KB

I was able to reproduce this, and the following patch seems to fix the problem. I'm not entirely sure what the author was intending with the query, however, so I'll take a closer look and make sure the new SQL is doing the right thing.

penguinchix0r’s picture

penguinchix0r commented
Status: Active » Needs review
sirprize’s picture

sirprize commented

Seems to work okay :-)
Will test it some more in the next days.

sirprize’s picture

sirprize commented
Status: Needs review » Reviewed & tested by the community

Updating status: I encountered no problems whatsoever, patch looks good too.

pwolanin’s picture

pwolanin commented
Version: 6.1 » 7.x-dev

bug fixes should go to 7.x first

Does this patch still apply?

dries’s picture

dries commented

Do we have tests that validate the order of the comments based on these settings?

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Nope. We also don't have a newest first comment order in core any more as of yesterday, so this needs a re-roll.

pwolanin’s picture

pwolanin commented

Sounds like the existing patch could still be the 6.x backport?

damien tournoud’s picture

damien tournoud commented
Status: Needs work » Closed (duplicate)

This is a duplicate of #396388: Fix syntax of comment ordering subquery, even if this one was technically opened first.

Note that the patch is wrong. That query is deliberately complex, and deliberately has an (implicit) sub-query on the FROM clause. Notice that SELECT FROM (SELECT FROM table ORDER BY x LIMIT xxx) ORDER BY y LIMIT 1 is not the same as SELECT FROM table ORDER BY x, y LIMIT 1.