tags:

views:

51

answers:

1

I have a string of 3 queries that are designed to

  1. find which messages have other messages with the same id which represents replies
  2. find which messages of the results from the first query have the specified user as entering the first message of that string of messages (min timestamp)
  3. find the latest message of that string of messages (max timstamp)

The problem comes with the third query. I get the expected results up to the second query, then when the third is executed, without the MAX(timestamp) as max, I get the expected results. When I add that, I only get the first message for each string of messages when it should be the last, regardless of whether I use min or max and the row count says 1 row returned when there is 2 rows shown. Anyone got any ideas on where I went wrong?

$sql="SELECT reply_chunk_id 
        FROM messages 
        GROUP BY reply_chunk_id 
        HAVING count(reply_chunk_id) > 1 ";
$stmt16 = $conn->prepare($sql);
$result=$stmt16->execute(array('specified_user'));
while($row = $stmt16->fetch(PDO::FETCH_ASSOC)){
    $sql="SELECT user,reply_chunk_id, MIN(timestamp) AS grp_timestamp
            FROM messages WHERE reply_chunk_id=?
            GROUP BY reply_chunk_id HAVING user=?";
    $stmt17 = $conn->prepare($sql);
    $result=$stmt17->execute(array($row['reply_chunk_id'],'specified_user'));
    while($row2 = $stmt17->fetch(PDO::FETCH_ASSOC)){
        $sql="SELECT message, MAX(timestamp) as max FROM messages WHERE reply_chunk_id=?";
        $stmt18 = $conn->prepare($sql);
        $result=$stmt18->execute(array($row2['reply_chunk_id']));
        while($row3 = $stmt18->fetch(PDO::FETCH_ASSOC)){
            echo '<p>'.$row3['message'];
        }
    }
}
echo ' '.$stmt18->rowCount();

create table view of messages, as requested

CREATE TABLE IF NOT EXISTS `messages` (
  `id` int(5) NOT NULL AUTO_INCREMENT,
  `timestamp` int(11) NOT NULL,
  `user` varchar(25) CHARACTER SET utf8 COLLATE utf8_unicode_ci NOT NULL DEFAULT 'anonimous',
  `message` varchar(2000) CHARACTER SET utf8 COLLATE utf8_unicode_ci NOT NULL,
  `topic_id` varchar(35) NOT NULL,
  `reply_chunk_id` varchar(35) NOT NULL,
  PRIMARY KEY (`id`)
) ENGINE=MyISAM  DEFAULT CHARSET=latin1 AUTO_INCREMENT=7 ;
+1  A: 

Since message isn't grouped, exactly which message from the group you'll get isn't defined. If you want the message with the maximum timestamp, you'll need to explicitly select it:

SELECT message, timestamp AS max
    FROM messages 
    WHERE reply_chunk_id=:rcid
      AND timestamp=(SELECT MAX(timestamp) 
                         FROM messages 
                         WHERE reply_chunk_id=:rcid)

or:

SELECT message, timestamp AS max
    FROM messages 
    WHERE reply_chunk_id=?
    ORDER BY timestamp DESC, id
    LIMIT 1

The second query breaks ties (in the very unlikely but possible situation that more than one person posts at the same time) by also selecting the message with the highest id.

General feedback

Many of the variables you set within the loops are invariant, and thus should be moved outside the loop.

$stmt17 will return at most 1 result. Moreover, $stmt18 will return always return exactly one result. Rewriting the second inner while loop (for $stmt17) as an if statement, and simply fetching the result from $stmt18 would be equivalent and clearer as to purpose.

try {
  $threadSql="SELECT reply_chunk_id 
          FROM messages 
          GROUP BY reply_chunk_id 
          HAVING count(reply_chunk_id) > 1 ";
  $firstUserSql="SELECT user, MIN(timestamp) AS grp_timestamp
          FROM messages WHERE reply_chunk_id=?
          GROUP BY reply_chunk_id HAVING user=?";
  $lastMsgSql="SELECT message, MAX(timestamp) as max FROM messages WHERE reply_chunk_id=?";
  $threadQuery = $conn->prepare($threadSql);
  $threadQuery->setFetchMode(PDO::FETCH_ASSOC);
  $firstUserQuery = $conn->prepare($firstUserSql);
  $lastMsgQuery = $conn->prepare($lastMsgSql);

  $result=$threadQuery->execute(array('specified_user'));
  foreach ($threadQuery AS $thread){
      $result=$firstUserQuery->execute(array($thread['reply_chunk_id'],'specified_user'));
      if (FALSE !== ($firstUser = $firstUserQuery->fetch(PDO::FETCH_ASSOC))) {
          $result=$lastMsgQuery->execute(array($thread['reply_chunk_id']));
          $lastMsg = $lastMsgQuery->fetch(PDO::FETCH_ASSOC);
          echo '<p>'.$lastMsg['message'].'</p>';
      }
  }
  echo ' ' . $lastMsgQuery->rowCount();
} catch (PDOException $exc) {
  ...
}

Lastly, a single SQL statement can replace much of the PHP code:

SELECT mchunk.reply_chunk_id, 
       muser.user, MIN(muser.`timestamp`) AS grp_timestamp, 
       mmax.message, mmax.`timestamp` AS max
  FROM messages AS mchunk
  JOIN messages AS muser
    ON mchunk.reply_chunk_id = muser.reply_chunk_id
  JOIN messages AS mmax
    ON mchunk.reply_chunk_id = mmax.reply_chunk_id
  WHERE mmax.timestamp=(SELECT MAX(timestamp) FROM messages AS m WHERE m.reply_chunk_id=mchunk.reply_chunk_id)
  GROUP BY mchunk.reply_chunk_id, muser.user
  HAVING count(mchunk.reply_chunk_id) > 1
    AND muser.user IN ('steve', '0010')
;

This selects all threads started by a specified user that have responses, along with the most recent response.

outis
I really appreciate your help outis, you went way above and beyond in helping me develop my skills and answer my question. I just have some quick questions. 1. wouldn't an sql query like the one you suggested take up a lot of resources? 2. I am not fully sure what is going on in your first suggestion since I am kind of new to programming in php still and use rather simple techniques. The part I do not understand is the if(FALSE line, I have never seen that before. 3. I noticed you used a try and catch. Should I be using that for every query? Thanks again for your help.
Scarface
1. It depends on how many results there are. Using multiple queries will also have a negative impact on server resources and will probably take longer overall. Generally speaking, it's best to have the database handle tasks that require assembling data matching given properties (think of SQL statements as declaring properties; the DBMS returns the data that has those properties). That leaves alteration tasks (such as presenting the data) for the programing language.
outis
2. That line could easily be separated in two: an assignment, and a comparison with FALSE. Fetching results from a DB query generally return FALSE when there are no more rows; the mysql, mysqli and PDO drivers all do this. If a query will return 0 or 1 rows, an `if` can test this by checking if the fetched value is FALSE. `!==` is like `!=` but doesn't juggle types; `FALSE == 0`, but `FALSE !== 0`.
outis
3. You should always handle errors. The try/catch assumes that you've set the error mode to `PDO::ERRMODE_EXCEPTION`, in which case the try/catch is mandatory. Sometimes it will make more sense to set the error mode to something else and check the return values of methods that return success or failure (e.g. PDOStatement::execute), but it's often easier to read to enclose a sequence of PDO method calls in a single try/catch, since the calls usually need to be completed successfully in sequence.
outis
A bigger and more complex change is to separate out the code that accesses the database (a.k.a. the data access layer) from code that presents the data. These are different [concerns](http://en.wikipedia.org/wiki/Separation_of_concerns), and separating them lets you easily change one without affecting the other, and offer different presentations of the same data. Applying the [Model-View-Controller pattern](http://oreilly.com/php/archive/mvc-intro.html) is one way of achieving this separation.
outis
thanks again, you have been a great help outis
Scarface
wow excellent last comment, really appreciate it outis
Scarface
is it possible to donate points lol?
Scarface