tags:

views:

58

answers:

2

Perplexed here.... This code:

$qry = sprintf("INSERT INTO news_sites_homepage_grab 
                VALUES ('', %d, '%s', NOW(), NOW())", 
                        $site_id, mysql_real_escape_string($html));

...is executed in a loop where $html changes every time. This code executes once but the next time, the script simply dies. No warnings/errors, nothing. $html is a string representing a webpage so it could be very long. I have upped the memory limit in PHP to 32M and set max_allowed_packet in MySQL to 16M but nothing.

Anyone have any ideas? Thanks!

UPDATE: Here is the function that is called within a loop.

function save_html($site_id, $html) {
global $db;
try {
    $qry = sprintf("INSERT INTO site_grab VALUES ('', %d, '%s', NOW(), NOW())",
                   $site_id,
                   mysql_real_escape_string($html));

    $db->insert($qry);

}
catch(Exception $e) {
    echo 'Caught exception: ',  $e->getMessage(), "\n";
}

return;

}

+3  A: 

My recommendation would be to forgo troubleshooting this issue and switch to PDO. It supports prepared statements, which aren't vulnerable to SQL injection when you use parameters and are much more performant for repeated queries.

Here's a simple refactoring of your function, which assumes $db holds a PDO instance:

function save_html($site_id, $html) {
    global $db;
    static $insert = Null;
    if (isnull($insert)) {
        /* explicitly name the columns, in case you later add more to the table
           or change their order. I'm also guessing `id` is auto-incrementing,
           so I'm leaving it out.
         */
        $insert = $db->prepare("INSERT INTO site_grab (sid, site_text, date_added, date_modified) VALUES (?, ?, NOW(), NOW())");
    }
    try {
        $insert->execute(array($site_id, $html));
    } catch(Exception $e) {
        echo 'Caught exception: ',  $e->getMessage(), "\n";
    }

    return;
}

If date_added has type TIMESTAMP, you can set its default value to CURRENT_TIMESTAMP and leave it out of the insert. Alternatively, add an ON UPDATE CURRENT_TIMESTAMP property to the date_modified column, which means you don't need to explicitly set the field when you update a row. If inserting is going to be more common than updating, do the former; otherwise, do the latter.

You could also store prepared statements in an object so they're accessible from multiple functions and replace the global $db with some sort of service locator (the simplest way is to use a static function or property of some class) or use dependency injection (read "Inversion of Control Containers and the Dependency Injection pattern").

outis
Worked like a charm. Thanks. Any idea why my code would've failed?
frio80
To be honest, I didn't look very hard at the original. The old mysql driver is so outdated I never bother trying to get code that uses it to work.
outis
A: 

I preffer using PDO and prepared statements but to (try to) answer your question try adding $db as the link identifier to mysql_real_escape_string() function.

Does that help? If not, try echoing the output of the mysql_error() function.

Sorry for the lack of formating, I'm mobile ATM.

Alix Axel
Thanks. Tried sending $db into the mysql_real_escape_stirng() but still nothing.
frio80