Please, let me describe an example related to 'insert' operation in hook_comment().

1) When a comment is created, comment_save() is invoked and runs this INSERT statement, then invokes hook_comment() for insert operation:

        db_query("INSERT INTO {comments} (cid, nid, pid, uid, subject, comment, format, hostname, timestamp, status, score, users, thread, name, mail, homepage) VALUES (%d, %d, %d, %d, '%s', '%s', %d, '%s', %d, %d, %d, '%s', '%s', '%s', '%s', '%s')", $edit['cid'], $edit['nid'], $edit['pid'], $edit['uid'], $edit['subject'], $edit['comment'], $edit['format'], $_SERVER['REMOTE_ADDR'], $edit['timestamp'], $status, $score, $users, $thread, $edit['name'], $edit['mail'], $edit['homepage']);

        _comment_update_node_statistics($edit['nid']);

        // Tell the other modules a new comment has been submitted.
        comment_invoke_comment($edit, 'insert');

2) Here's a hook_comment() example from commentmail module:

function commentmail_comment($comment, $op){
  switch($op){
    case 'insert':
      $comment_obj = db_fetch_object(db_query('SELECT * FROM {comments} WHERE cid = %d', $comment['cid']));

What happens? The commentmail module needs information that is not in $comment ($edit in the caller's context), so it opts to read the comment that has just been inserted.

If the first argument of hook_comment() had all information of the comment, a SELECT statement could be saved.

It would be as easy as complete the contents of the $edit array before calling the hook. Maybe something like this:

        db_query("INSERT INTO {comments} (cid, nid, pid, uid, subject, comment, format, hostname, timestamp, status, score, users, thread, name, mail, homepage) VALUES (%d, %d, %d, %d, '%s', '%s', %d, '%s', %d, %d, %d, '%s', '%s', '%s', '%s', '%s')", $edit['cid'], $edit['nid'], $edit['pid'], $edit['uid'], $edit['subject'], $edit['comment'], $edit['format'], $_SERVER['REMOTE_ADDR'], $edit['timestamp'], $status, $score, $users, $thread, $edit['name'], $edit['mail'], $edit['homepage']);

        _comment_update_node_statistics($edit['nid']);

        $edit['hostname'] = $_SERVER['REMOTE_ADDR'];
        $edit['status'] = $status;
        $edit['thread'] = $thread;

        // Tell the other modules a new comment has been submitted.
        comment_invoke_comment($edit, 'insert');

The same trick could be applied everywhere else hook_comment() is invoked.

I believe this kind of optimizations is particulary important, depending on modules installed. If every module that implements hook_comment() needs to do its own SELECT... see what I mean?

Comments

webchick’s picture

Version: x.y.z » 6.x-dev
Status: Active » Fixed

I think this is fixed as of 6.x. The offending line now looks like:

        db_query("INSERT INTO {comments} (nid, pid, uid, subject, comment, format, hostname, timestamp, status, thread, name, mail, homepage) VALUES (%d, %d, %d, '%s', '%s', %d, '%s', %d, %d, '%s', '%s', '%s', '%s')", $edit['nid'], $edit['pid'], $edit['uid'], $edit['subject'], $edit['comment'], $edit['format'], ip_address(), $edit['timestamp'], $edit['status'], $thread, $edit['name'], $edit['mail'], $edit['homepage']);

The only two values that aren't in the edit array are ip_address() (which is a simple function call) and $thread, and I can't envision any reason that a contrib module would need access to $thread.

Anonymous’s picture

Status: Fixed » Closed (fixed)

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