By velankar on

Here is a part of code from comments module in the function function comment_render

=====================================
........
........
..........

if ($cid) {
// Single comment view.
$result = db_query('SELECT c.cid, c.pid, c.nid, c.subject, c.comment, c.format, c.timestamp, c.name, c.mail, c.homepage, u.uid, u.name AS registered_name, u.picture, u.data, c.score, c.users FROM {comments} c INNER JOIN {users} u ON c.uid = u.uid WHERE c.cid = %d AND c.status = %d GROUP BY c.cid, c.pid, c.nid, c.subject, c.comment, c.format, c.timestamp, c.name, c.mail, u.picture, c.homepage, u.uid, u.name, u.picture, u.data, c.score, c.users', $cid, COMMENT_PUBLISHED);

if ($comment = db_fetch_object($result)) {
$comment->name = $comment->uid ? $comment->registered_name : $comment->name;
$output .= theme('comment_view', $comment, module_invoke_all('link', 'comment', $comment, 1));
}
}
else {
// Multiple comment view
$query .= "SELECT c.cid as cid, c.pid, c.nid, c.subject, c.comment, c.format, c.timestamp, c.name, c.mail, c.homepage, u.uid, u.name AS registered_name, u.picture, u.data, c.score, c.users, c.thread FROM {comments} c INNER JOIN {users} u ON c.uid = u.uid WHERE c.nid = %d AND c.status = %d";

$query .= ' GROUP BY c.cid, c.pid, c.nid, c.subject, c.comment, c.format, c.timestamp, c.name, c.mail, u.picture, c.homepage, u.uid, u.name, u.picture, u.data, c.score, c.users, c.thread';
.........
........
..........
........

==========================
My question is about this group by clause.
What is the purpose of such huge group by clause?

Can anybody explain me?

Thanks in advance
-Mahesh

⋅ Categories: Drupal 4.5.x or older, Drupal 4.6.x, Drupal 4.7.x

Comments

velankar’s picture

velankar commented

Does anybody have any explanation on this.
I am still wondering.
Thanks

tostinni’s picture

tostinni commented

I also wonder exactly the same thing yesterday while looking through this module...
In fact, if you have a cid which is unique, I don't understand the GROUP BY here.

Any idea ?

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

As you probably know, group by in this case has the same effect as "select distinct" so maybe the goal is to get a distinct listing?

I didn't test/investigate too much. I imagine this could be removed or traded out for a "distinct" keyword assuming that is supported in every DB that Drupal supports.

--
Knaddison Family Blog

--
Morris Animal Foundation

velankar’s picture

velankar commented

what is its purpose when cid is the primary key and that is unique anyway.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

As I said, I don't know. You could do a test or two to find out.

If you have a query that results in data like

field1 | field2
1 | asdf
1 | asdf
1 | asdf

You can use a "group by" or a "distinct" to get rid of the duplicate rows.

Why don't you remove the group by and see what happens in a test query? If the result is the same try doing it in the live site...

--
Growing Venture Solutions
Drupal Implementation and Support in Denver, CO

--
Morris Animal Foundation

archimedes-1’s picture

archimedes-1 commented

My guess is that this query is partly historic and partly a bit of a MySQL-ism (though I know of other DBMS platforms that support this kind of thing).

For background, the MySQL 3.23, 4.0, 4.1 Reference Manual, the DISTINCT Optimization section talks a bit about how DISTINCT and GROUP BY can be related.

In most cases, a DISTINCT clause can be considered as a special case of GROUP BY. For example, the following two queries are equivalent:

SELECT DISTINCT c1, c2, c3 FROM t1 WHERE c1 > const;

SELECT c1, c2, c3 FROM t1 WHERE c1 > const GROUP BY c1, c2, c3;

So you can see how GROUP BY may have managed to make its way into the query. The use of SELECT DISTINCT would have been the less confusing and more portable option but that's beside the point at this time.

My guess is that once upon a time, these two queries looked different to what they look like today and the uniqueness filter was necessary. With time and evolution, things moved on but these queries didn't because they managed to escape going under the microscope because things still behaved correctly. Take a close look at the GROUP BY clause - there are two occurrances of the u.picture column. These queries are screaming "review me!"

Anyway, I think you'll be perfectly fine if the GROUP BY clause is removed entirely because each row is guaranteed to be distinct as velankar has already pointed out. The queries should be fractionally quicker too. On a related subject, I am nearing completion of some work regarding getting Drupal to run under ORACLE and these two queries caused errors because ORACLE does not permit the use of GROUP BY in the absence of aggregate functions, so removing the GROUP BY was necessary in my case and things work fine having done that.

By the way, I have found a small number of other "interesting" queries like this in the core modules. I have noted them and my list will be complete by the time I have finished the port.