Line 94 ... 102 of taxonomy_nco.module

    $sql = "SELECT tid_b, nco FROM {taxonomy_nco}
            WHERE tid_a IN (:tid_list)
            AND (a_and_b >= :minimum_match_intersection)
            AND (nco >= :minimum_match_nco)
            ORDER BY nco DESC";    
    $placeholders = array(':tid_list' => $tid_list, 
                          ':minimum_match_intersection' => $minimum_match_intersection, 
                          ':minimum_match_nco' => $minimum_match_nco);
    $result = db_query($sql, $placeholders);
    foreach($result as $record) {
      $output[$record->tid_b] = $record->nco;
    }

The above works fine for single tids. However, if tid_list contains multiple tids (comma separated), the above appears to ignore all but the first tid.

Comments

aparimana’s picture

Also: another bug in the same code:

With multiple tids, the query is likely to pull back multiple nco ratings for each tid_b. The code above with end up passing back the nco value of the last-listed-instance of each tid_b, which is essentially random. A more useful behaviour would be to pass back the maximum nco for each tid_b.

aparimana’s picture

Status: Active » Needs review

I have fixed both of these by rewriting the query using the abstraction objects:

    $sel = db_select('taxonomy_nco','t')
      ->fields('t',array('tid_b'))
      ->condition('tid_a',$tids,'IN')
      ->condition('a_and_b',$minimum_match_intersection,'>=')
      ->condition('nco',$minimum_match_nco,'>=')
      ->groupBy('tid_b');

    //where there are multiple tids passed, there may be multiple nco results for each related tid
    //therefore we select the instance of each related tid that has the greatest nco
    $ncoAlias = $sel->addExpression('max(nco)');

    $rows = $sel->execute();
    foreach($rows as $row){
      $output[$row->tid_b] = $row->$ncoAlias;
    }

Note that the query does not bother ordering the results by nco values, as the php code lower down (line 184) manually sorts the results after merging in forced nco values

The code above is contained in the patch submitted here: http://drupal.org/node/1266546 - I have therefore not created a patch for just these changes

russellb’s picture

Status: Needs review » Fixed

Thanks again AP, good to have the multiple tids going in D7.

Committed:
http://drupal.org/commitlog/commit/12120/e632054bd812ac754e6944a1ccff46a...

russellb’s picture

Status: Fixed » Closed (fixed)