Page 3 of 4
Posted: Wed Jun 06, 2007 9:38 am
by fishnyc22
Did you mean to have this line like this. Something's not jiving here:
Code: Select all
AND user_message.message_id = first_message
I believe in one of my posts above (in my personal mess of code) I had just one sql query that was ordered by message_id. Is 1 query and cycling though the batches based on message_id more efficient (prob not the way I wrote it) than multiple queries? Just curious.
Posted: Wed Jun 06, 2007 1:39 pm
by Chris Corbyn
No sorry, take that out. I was experimenting with the query. As you can see, the code I'm posting has not been tested. That line was leftover from something else I was trying

Posted: Wed Jun 06, 2007 2:03 pm
by fishnyc22
That still results in the same:
recipient 1, message 1
recipient 2, message 2
recipient 3, message 3
Is that what you intended? Or did you just want to pull all recipients of one message. If you're busy..I understand.... I don't want to keep buggin you about this. Worse comes to worse I get it going how I had it before. One query, parsing through the data and creating batches based on message_id.
Thanks buddy
Posted: Wed Jun 06, 2007 2:55 pm
by Chris Corbyn
Sorry. let's start over with that query... it's a higgldy-piggldy mess of two queries
From scratch:
Code: Select all
$sql = "
SELECT
user_message.id,
message.subject,
message.body,
message.from_email,
message.from_name,
user_message.to_email,
user_message.to_name,
user_message.message_id
FROM
message,
user_message
WHERE
user_message.message_id = message.id
AND user_message.status = 'U'
ORDER BY
user_message.message_id
LIMIT " . $max_size;
Posted: Wed Jun 06, 2007 3:10 pm
by fishnyc22
OK, so we are abandoning the idea of getting the first batch then looping through and getting the second batch. The above SQL will pull the first 100 even if the message_id is different. I'm not sure how that will work with the php code on page 2. Since that is not sorting the message_ids. I think we've both been staring at our machines too long.

Posted: Wed Jun 06, 2007 3:12 pm
by Chris Corbyn
The PHP code was updated when I posted earlier today

It now loops through the resulstset, then breaks out of the loop once it hits a different message ID. If that's less than 100 recipients, it goes around again to do another batch

The code looks the same, bit there are subtle differences.
Posted: Wed Jun 06, 2007 3:14 pm
by Chris Corbyn
To make it a bit easier to sift through (ignoring the fact that the query was wrong in these scripts!):
Code: Select all
w3style.co.uk:~# diff -urN code.orig.php code.new.php
--- code.orig.php 2007-06-06 21:14:04.000000000 +0100
+++ code.new.php 2007-06-06 21:14:34.000000000 +0100
@@ -1,5 +1,3 @@
-$swift =& new Swift(new Swift_Connection_SMTP("localhost"));
-
$max_size = 100;
while ($max_size > 0)
{
@@ -11,29 +9,39 @@
message.from_email,
message.from_name,
user_message.to_email,
- user_message.to_name
+ user_message.to_name,
+ user_message.message_id
FROM
message,
user_message
WHERE
user_message.message_id = message.id
AND user_message.status = 'U'
+ AND user_message.message_id = first_message
GROUP BY
- message.id
+ user_message.id
+ ORDER BY
+ user_message.message_id
LIMIT " . $max_size;
$result = mysql_query($sql, $conn) or die(mysql_error());
//No messages left to send, so stop looping
- if (!$total = mysql_num_rows($result)) break;
-
- $max_size -= $total; //Maybe we've still not got to 100 yet
+ if (!mysql_num_rows($result)) break;
//Build up a list of recipients
$list =& new Swift_RecipientList();
$message = null;
$sender = null;
+ $message_id = -1; //For tracking in the loop
$ids = array();
+ $total = 0;
while ($row = mysql_fetch_assoc($result))
{
+ //Not the same message anymore -- different batch
+ if ($message_id > -1 && $row["message_id"] != $message_id)
+ {
+ break; //Just this loop
+ }
+ $total++;
//No need to repeatedly create $message or $sender in the loop
if ($message === null) $message =& new Swift_Message($row["subject"], $row["body"]);
if ($sender === null) $sender =& new Swift_Address($row["from_email"], $row["from_name"]);
@@ -42,6 +50,8 @@
$ids[] = $row["id"];
}
+ $max_size -= $total; //Maybe we've still not got to 100 yet
+
//Send this batch
$batch =& new Swift_BatchMailer($swift);
$batch->send($message, $list, $sender);
Posted: Wed Jun 06, 2007 3:14 pm
by fishnyc22
Sorry. I was just about to repost that you had changed that. I didnt realize that. I TOTALLY GET whats going on here. I thought we were still shooting for the other method. I'm gonna do my best not to bother you anymore tonight
Thanks a bunch! Very cool of you to walk me though this.
D
Posted: Wed Jun 06, 2007 3:17 pm
by Chris Corbyn
No problem

It doesn't bother me.
Posted: Wed Jun 06, 2007 4:02 pm
by fishnyc22
Ha. Here I am minutes later ... sorry.
I believe there is a bug here...
Code: Select all
if ($message_id > -1 && $row["message_id"] != $message_id)
That will never be true since $message_id is set to -1 just before that line.
Did you mean to set the message_id before the second while loop?
Do I need to flushFailedRecipients() after updating my database at the end of the script?
And my final question: Do we still need the first while loop if we are querying all the messages at once now? Is there a reason to keep it in there?
Thanks
Posted: Wed Jun 06, 2007 4:15 pm
by Chris Corbyn
I missed the bit after the whole condition:
Code: Select all
//Not the same message anymore -- different batch
if ($message_id > -1 && $row["message_id"] != $message_id)
{
break; //Just this loop
}
$message_id = $row["message_id"]; //Keep watching!

Posted: Wed Jun 06, 2007 4:25 pm
by fishnyc22
Right, that makes sense. I'm smarter than this, honest. I should have figured that out.
Is there any concern with calling the SQL call over and over again? Rather than calling it once and using that one record set to do whats necessary?
Any need to "flushFailedRecipients" here?
Posted: Wed Jun 06, 2007 5:16 pm
by Chris Corbyn
fishnyc22 wrote:Is there any concern with calling the SQL call over and over again? Rather than calling it once and using that one record set to do whats necessary?
I'd rather not do that... I'm aware of a better way, I just didn't have much time to play with it in the office today. Lemme post an update in a few mins.
Any need to "flushFailedRecipients" here?
Not in my example since I recreate the $batch object when I start a new batch. If you're re-using the same object then yes you would need to flush it

Posted: Wed Jun 06, 2007 5:45 pm
by Chris Corbyn
This one (if it works) will only pull one resultset:
Code: Select all
<?php
$max_size = 100;
$sql = "
SELECT
user_message.id,
message.subject,
message.body,
message.from_email,
message.from_name,
user_message.to_email,
user_message.to_name,
user_message.message_id
FROM
message,
user_message
WHERE
user_message.message_id = message.id
AND user_message.status = 'U'
ORDER BY
user_message.message_id
LIMIT " . $max_size;
$result = mysql_query($sql, $conn) or die(mysql_error());
$num_rows = mysql_num_rows($result);
$data_pos = -1; //Current position in the resultset
$ids = array();
//Nothing to do if no data
if ($num_rows > 0)
{
$swift =& new Swift(new Swift_Connection_SMTP("localhost"));
$batch =& new Swift_BatchMailer($swift);
do
{
//Build up a list of recipients
$list =& new Swift_RecipientList();
$message = null;
$sender = null;
$message_id = -1; //For tracking in the loop
while ($row = mysql_fetch_assoc($result))
{
$data_pos++;
//Not the same message anymore -- different batch
if ($message_id > -1 && $row["message_id"] != $message_id)
{
mysql_data_seek($result, --$data_pos); //Backtrack ready for next batch
break; //Just this loop
}
$message_id = $row["message_id"];
//No need to repeatedly create $message or $sender in the loop
if ($message === null) $message =& new Swift_Message($row["subject"], $row["body"]);
if ($sender === null) $sender =& new Swift_Address($row["from_email"], $row["from_name"]);
$list->addTo($row["to_email"], $row["to_name"]);
//Collect recipient IDs for use later
$ids[] = $row["id"];
}
//Send this batch
$batch->send($message, $list, $sender);
} while ($data_pos < $num_rows); //Only while we're not at the end of the resultset
//If we have any records to update
if (!empty($ids))
{
//Some may have failed, some may not
$failures = $batch->getFailedRecipients();
foreach ($failures as $key => $value)
{
//Filter for use in SQL (and add single quotes)
$failures[$key] = "'" . mysql_real_escape_string($value) . "'";
}
//Update all that have NOT failed to 'S'
$sql = "
UPDATE user_message
SET status = 'S'
WHERE
id IN (" . implode(",", $ids) . ")";
if (!empty($failures))
{
$sql .= "
AND to_email NOT IN (" . implode(",", $failures) . ")";
}
mysql_query($sql, $conn) or die(mysql_error());
//Update all that have failed to 'F'
if (!empty($failures))
{
$sql = "
UPDATE user_message
SET status = 'F'
WHERE
id IN (" . implode(",", $ids) . "
AND to_email IN (" . implode(",", $failures) . ")";
mysql_query($sql, $conn) or die(mysql_error());
}
}
}
Posted: Wed Jun 06, 2007 9:05 pm
by fishnyc22
wow. never used mysql_data_seek. cool stuff. However its only sending the last message in the record set. It is updating the DB to S for all messages though. I'm trying to debug now. Thanks..very cool stuff.