Skip to content

Commit

Permalink
Denormalise status from the node table to avoid joins.
Browse files Browse the repository at this point in the history
Since these queries are run on every page load (except pages where campaigns are explicitly cached), they need to be as fast as possible.

This reverts commit ed533a3.
  • Loading branch information
mikl committed Dec 7, 2011
1 parent 70108cf commit c4d904e
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 12 deletions.
2 changes: 2 additions & 0 deletions ding_campaign.admin.inc
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ function ding_campaign_admin_rule_form_submit($form, &$form_state) {
// delta, and use the new key to overwrite the delta setting on the rule.
foreach (array_values($form_state['values']['rules']) as $delta => $rule) {
$rule['delta'] = $delta;
$rule['status'] = 1;

drupal_write_record('ding_campaign_rule', $rule);
}
}
Expand Down
47 changes: 45 additions & 2 deletions ding_campaign.install
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ function ding_campaign_schema() {
'not null' => TRUE,
'description' => '{node}.nid for node',
),
'status' => array(
'description' => 'Denormalised mirror of {node}.status so we can filter out unpublished campaigns without requiring a JOIN',
'type' => 'int',
'size' => 'tiny',
'not null' => TRUE,
'default' => 0,
),
'delta' => array(
'type' => 'int',
'unsigned' => TRUE,
Expand All @@ -97,7 +104,7 @@ function ding_campaign_schema() {
),
'primary key' => array('nid', 'delta'),
'indexes' => array(
'nid' => array('nid'),
'nid_status' => array('nid', 'status'),
'type' => array('type'),
'value' => array('value'),
'value_id' => array('value_id'),
Expand Down Expand Up @@ -154,7 +161,7 @@ function ding_campaign_update_6003(&$sandbox) {
/**
* Implementation of hook_update_N().
*
* Create cache table for Ding campaigns.
* Delete cache table for Ding campaigns.
*/
function ding_campaign_update_6004(&$sandbox) {
$ret = array();
Expand All @@ -164,3 +171,39 @@ function ding_campaign_update_6004(&$sandbox) {
return $ret;
}

/**
* Add status column to campaign rule table.
*/
function ding_campaign_update_6005(&$sandbox) {
$ret = array();

db_add_field($ret, 'ding_campaign_rule', 'status', array(
'type' => 'int',
'size' => 'tiny',
'not null' => TRUE,
'default' => 0,
));

// Add status to the index, since we always filter on it.
db_drop_index($ret, 'ding_campaign_rule', 'nid');
db_add_index($ret, 'ding_campaign_rule', 'nid_status', array('nid', 'status'));

return $ret;
}

/**
* Set rule status from node table.
*/
function ding_campaign_update_6006(&$sandbox) {
$ret = array();

$ret[] = update_sql("
UPDATE {ding_campaign_rule} AS dcr
LEFT JOIN node AS n USING (nid)
SET dcr.status = n.status
WHERE n.status <> 0
");

return $ret;
}

25 changes: 15 additions & 10 deletions ding_campaign.module
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,20 @@ function ding_campaign_nodeapi(&$node, $op, $teaser, $page) {
break;
case 'update':
if ($node->type == 'campaign') {
// Check if theres an existing row in {ding_campaign}
$prev_vid = db_result(db_query("SELECT vid FROM {ding_campaign} WHERE nid=%d;", $node->nid));
// Check if there's an existing row in {ding_campaign}
$prev_vid = db_result(db_query("SELECT vid FROM {ding_campaign} WHERE nid = %d", $node->nid));
// If this is a new node or we're adding a new revision,
if ($node->revision || !$prev_vid) {
drupal_write_record('ding_campaign', $node);
}
else {
drupal_write_record('ding_campaign', $node, array('nid', 'vid'));
}

db_query('UPDATE {ding_campaign_rule} SET status = %d WHERE nid = %d', array(
':status' => $node->status,
':nid' => $node->nid,
));
}
else {
// When updating a node, purge the current campaign selections first.
Expand Down Expand Up @@ -149,13 +154,13 @@ function ding_campaign_nodeapi(&$node, $op, $teaser, $page) {
// We use $now as delta to be reasonably sure not to have
// delta collisions within the same nid, since nid+delta is
// the primary key for ding_campaign_rule.
$cols[] = "(%d, $now, 'page', '%s', %d)";
$cols[] = "(%d, 1, $now, 'page', '%s', %d)";
$params[] = $campaign_nid;
$params[] = $node->title . ' [nid:' . $node->nid . ']';
$params[] = $node->nid;
}

db_query('INSERT INTO {ding_campaign_rule} (nid, delta, type, value, value_id) VALUES ' . implode(',', $cols), $params);
db_query('INSERT INTO {ding_campaign_rule} (nid, status, delta, type, value, value_id) VALUES ' . implode(',', $cols), $params);
}
}
break;
Expand Down Expand Up @@ -411,14 +416,14 @@ function ding_campaign_get_relevant($context, $max_count = -1, $offset = 0) {
else {
$nid = (int) $context['page'];
}
$query = db_query("SELECT d.nid FROM {ding_campaign_rule} d JOIN {node} n USING (nid) WHERE n.status = 1 AND d.type = 'page' AND d.value_id = %d", $nid);
$query = db_query("SELECT nid FROM {ding_campaign_rule} WHERE type = 'page' AND value_id = %d;", $nid);

This comment has been minimized.

Copy link
@kasperg

kasperg Dec 7, 2011

Contributor

Shouldn't this include status = 1?

while ($camp_nid = db_result($query)) {
$campaigns[$camp_nid] += 9;
}
}

// Get all path-based campaign rules.
$query = db_query("SELECT d.* FROM {ding_campaign_rule} d JOIN {node} n USING (nid) WHERE n.status = 1 AND d.type = 'path'");
$query = db_query("SELECT * FROM {ding_campaign_rule} WHERE type = 'path';");

This comment has been minimized.

Copy link
@kasperg

kasperg Dec 7, 2011

Contributor

Shouldn't this include status = 1?

while ($row = db_fetch_array($query)) {
$path = drupal_get_path_alias($_GET['q']);
// Compare with the internal and path alias (if any).
Expand All @@ -444,7 +449,7 @@ function ding_campaign_get_relevant($context, $max_count = -1, $offset = 0) {
$condition = "= '" . $keys . "'";
}

$query = db_query("SELECT d.nid FROM {ding_campaign_rule} d JOIN {node} n USING (nid) WHERE n.status = 1 AND d.type = 'search_term' AND d.value " . $condition);
$query = db_query("SELECT nid FROM {ding_campaign_rule} WHERE type = 'search_term' AND status = 1 AND value " . $condition);
while ($camp_nid = db_result($query)) {
$campaigns[$camp_nid] += 7;
}
Expand All @@ -457,7 +462,7 @@ function ding_campaign_get_relevant($context, $max_count = -1, $offset = 0) {
else {
$nid = (int) $context['library'];
}
$query = db_query("SELECT d.nid FROM {ding_campaign_rule} d JOIN {node} n USING (nid) WHERE n.status = 1 AND d.type = 'library' AND d.value_id = %d;", $nid);
$query = db_query("SELECT nid FROM {ding_campaign_rule} WHERE type = 'library' AND status = 1 AND value_id = %d;", $nid);
while ($camp_nid = db_result($query)) {
$campaigns[$camp_nid] += 5;
}
Expand All @@ -470,15 +475,15 @@ function ding_campaign_get_relevant($context, $max_count = -1, $offset = 0) {
else {
$tid = (int) $context['taxonomy'];
}
$query = db_query("SELECT d.nid FROM {ding_campaign_rule} d JOIN {node} n USING (nid) WHERE n.status = 1 AND d.type = 'taxonomy' AND d.value_id = %d;", $tid);
$query = db_query("SELECT nid FROM {ding_campaign_rule} WHERE type = 'taxonomy' AND status = 1 AND value_id = %d;", $tid);
while ($camp_nid = db_result($query)) {
$campaigns[$camp_nid] += 3;
}
}

// Give a small (and random) amount of relevancy to generic campaigns
// that have not scored in any of the other categories.
$query = db_query("SELECT d.nid FROM {ding_campaign_rule} d JOIN {node} n USING (nid) WHERE n.status = 1 AND d.type = 'generic'");
$query = db_query("SELECT nid FROM {ding_campaign_rule} WHERE type = 'generic AND status = 1'");
while ($camp_nid = db_result($query)) {
if (!isset($campaigns[$camp_nid]) || $campaigns[$camp_nid] == 0) {
$campaigns[$camp_nid] = (mt_rand(1, 10) / 10);
Expand Down

0 comments on commit c4d904e

Please sign in to comment.