Hi,

6.x-2.0-beta1 had a SQL query which doesn't work on PostgreSQL.

I have made a patch for it.

CommentFileSizeAuthor
#15 2010-04-04_761276_comment_subject_pgsql_D7[unix].patch1.51 KBarhak
#12 comment-subject-pgsql.patch616 bytesnileshgr
#4 comment-subject-pgsql.patch635 bytesnileshgr
comment-subject-pgsql.patch647 bytesnileshgr

Comments

arhak’s picture

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

thanks

arhak’s picture

arhak commented
Status: Reviewed & tested by the community » Postponed (maintainer needs more info)

it seems that at the time I addressed that query as if comments had revisions (like nodes do),
that's why the group by cid,nid,
but cid is primary key, therefore that group by was nonsense

BUT, what version are we talking about?
I can't find that on my repo,
the proposed fix is correct, but it is how it currently is
I just re-downloaded 6.x-2.0-beta1, 6.x-2.x-dev, 7.x-2.0-alpha1, and it is that way on those versions

nileshgr’s picture

nileshgr commented

PostgreSQL follows SQL Standards strictly. Hence you cannot have a 'WHERE' in a select statement which has an aggregate function.
Download 6.x-2.0-beta1, open comment_subject.token.inc and see line 63.

I had this version on my site, and found the errors from php and pg_query in the error log.

Wait a minute. I think the diff arguments were misplaced. I will upload another patch :(

nileshgr’s picture

nileshgr commented
StatusFileSize
new comment-subject-pgsql.patch635 bytes
arhak’s picture

arhak commented

ah... is the other way around
in that case, would be Postgre ok having this:

$query = 'SELECT COUNT(cid) FROM {comments} GROUP BY nid HAVING nid = %d AND cid < %d ORDER BY cid ASC';

you see, it is difficult to read the proposed Group By cid,nid when cid is primary key and therefore unique
also, a reference to Postgree documentation would be ideal for a comment above that query
to know later why it was changed to a groupby-having instead of a simple where

nileshgr’s picture

nileshgr commented

I don't have any reference as of now, but I am sure about it. I will update if I get some reference. MySQL doesn't follow SQL standards strictly so a WHERE may work with an aggregate function. But PostgreSQL is exactly opposite to MySQL in this aspect.

If you alter the code, so that it looks like this:

<?php
$query = 'SELECT COUNT(cid) FROM {comments} GROUP BY nid HAVING nid = %d';
?>

it works. I've tested it. And I think we need only the count of comments irrespective of cid.

arhak’s picture

arhak commented
Status: Postponed (maintainer needs more info) » Needs work

could you please, try the following

$query = 'SELECT COUNT(cid) FROM {comments} WHERE nid = %d GROUP BY nid';
arhak’s picture

arhak commented

And I think we need only the count of comments irrespective of cid

no, only the preceding ones
the restriction and ORDER BY are needed

nileshgr’s picture

nileshgr commented

Yeah this works. It provides the count of comments per node. (PS: Only one row).
It works even with HAVING instead of WHERE, but the order matters.

WHERE works if placed before GROUP BY, opposite for HAVING..

A new finding:

<?php

$sql =  'SELECT COUNT(cid) FROM comments WHERE nid = %d and cid < %d GROUP BY nid';

?>

But if you add ORDER BY clause, it won't.

arhak’s picture

arhak commented

then #5 would do?

arhak’s picture

arhak commented
Status: Needs work » Reviewed & tested by the community

no, #9 is fine, don't need the order by, just the cid < %d
thanks for your contrib

nileshgr’s picture

nileshgr commented
StatusFileSize
new comment-subject-pgsql.patch616 bytes

Here's a patch incase anyone needs it :)

arhak’s picture

arhak commented
Title: PostgreSQL Patch for 6.x-2.0-beta1 » PostgreSQL: aggregate functions demand GROUP BY (SQL standard compliant)
Status: Reviewed & tested by the community » Fixed

committed to 6.x-2.x

arhak’s picture

arhak commented
Version: 6.x-2.0-beta1 » 7.x-2.0-alpha1
Status: Fixed » Patch (to be ported)

the same would be needed for 7.x-2.x

arhak’s picture

arhak commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new 2010-04-04_761276_comment_subject_pgsql_D7[unix].patch1.51 KB

attached path for 7.x-2.x