adserve.inc / adserve_increment fails

snarlydwarf - September 22, 2009 - 19:20
Project:Advertisement
Version:5.x-1.7
Component:adserve.php
Category:bug report
Priority:critical
Assigned:Unassigned
Status:active
Description

This code is wrong:

db_query("UPDATE {ad_statistics} SET count = count + 1 WHERE aid = %d AND action = '%s' AND date = %d AND adgroup = '%s' AND hostid = '%s'", $ad->aid, $action, date('YmdH'), adserve_variable('group'), adserve_variable('hostid'));
// If column doesn't already exist, we need to add it.
if (!db_affected_rows()) {
db_query("INSERT INTO {ad_statistics} (aid, date, action, adgroup, hostid, count) VALUES(%d, %d, '%s', '%s', '%s', 1)", $ad->aid, date('YmdH'), $action, adserve_variable('hostid'), adserve_variable('hostid'));
// If another process already added this row our INSERT will fail, if
// so we still need to increment it so we don't loose a view.
if (!db_affected_rows()) {
db_query("UPDATE {ad_statistics} SET count = count + 1 WHERE aid = %d AND action = '%s' AND date = %d AND adgroup = '%s' AND hostid = '%s'", $ad->aid, $action, date('YmdH'), adserve_variable('group'), adserve_variable('hostid'));
}
}

The 'INSERT INTO' is inserting hostid twice, not adgroup, which means it will insert (in most cases): aid, date, action, '', '', 1....

Which will not match the UPDATE, meaning a row is created for each ad display, not for each ad.

This tends to make mysql very grumpy when it has to search potentially millions of rows. It makes ad reports on busy sites incredibly painful.

FIx: change the first "adserve_variable('hostid')" on the INSERT above to match the UPDATE's with "adserve_variable('group')"

(My copy of adserve.inc is a bit hacked up, so my line numbers won't match.)

 
 

Drupal is a registered trademark of Dries Buytaert.