db_distinct_field produces MySQL errors

Maybe the original code works with Postgres, but it definitely doesn't work for MySQL. The following function needs to be changed in

• includes/database.mysql.inc
• includes/database.mysqli.inc

On a 5.1 site, I fixed it like this (tested with MySQL 5.0.24):

function db_distinct_field($table, $field, $query) {
  // (?<!text) is a negative look-behind (no need to rewrite queries that already use DISTINCT).

  $my_search = "/SELECT (.*)(,?\s*)(?<!DISTINCT)\s+($table\.)?$field(.*FROM )/AUsi";
  $my_replace = "SELECT DISTINCT \\3$field\\2\\1\\4";

  return preg_replace( $my_search, $my_replace, $query );
}

The original version in 5.1 looked like this:

function db_distinct_field($table, $field, $query) {
  $field_to_select = 'DISTINCT('. $table .'.'. $field .')';
  // (?<!text) is a negative look-behind (no need to rewrite queries that already use DISTINCT).
  return preg_replace('/(SELECT.*)(?:'. $table .'\.|\s)(?<!DISTINCT\()(?<!DISTINCT\('. $table .'\.)'. $field .'(.*FROM )/AUsi', '\1 '. $field_to_select .'\2', $query);
}

On a 4.7 site (which I tested with MySQL v4.1.20), the change is nearly exactly the same, but is in an un-refactored version of the functions above. It is in

• includes/database.inc

in the function db_rewrite_sql.

Before:

  if ($distinct) {
    $field_to_select = 'DISTINCT('. $primary_table .'.'. $primary_field .')';
    // (?<!text) is a negative look-behind (no need to rewrite queries that already use DISTINCT).
    $query = preg_replace('/(SELECT.*)('. $primary_table .'\.)?(?<!DISTINCT\()(?<!DISTINCT\('. $primary_table .'\.)'. $primary_field .'(.*FROM)/AUsi', '\1'. $field_to_select .'\3', $query);
  }

After:

  if ($distinct) {

    // (?<!text) is a negative look-behind (no need to rewrite queries that already use DISTINCT).

    $my_search = "/SELECT (.*)(,?\s*)(?<!DISTINCT)\s+($primary_table\.)?$primary_field(.*FROM )/AUsi";
    $my_replace = "SELECT DISTINCT \\3$primary_field\\2\\1\\4";

    $query = preg_replace( $my_search, $my_replace, $query );
  }

It was pointed out to me that in MySQL, the DISTINCT keyword appears only right after the SELECT statement, and it applies to the entire row. Thus, there is no need to parse a single field out of the select list and pull it to the front. Hence, the fix is *much* simpler.

5.1 version:

/**
 * Add the the 'DISTINCT' keyword to the start of the query, if it isn't already
 * present.
 *
 * @param $table Table containing the field to set as DISTINCT
 * @param $field Field to set as DISTINCT
 * @param $query Query to apply the wrapper to
 * @return SQL query with the DISTINCT wrapper surrounding the given table.field.
 */
function db_distinct_field($table, $field, $query) {

  $dont_want = '/^\s*SELECT\s+(DISTINCT|DISTINCTROW)\s+/si';

  if ( preg_match($dont_want, $query)) {

    return $query;
  }else{
    $my_search = "/^\s*SELECT\s+/si";
    $my_replace = "SELECT DISTINCT ";

    return preg_replace( $my_search, $my_replace, $query );
  }
}

4.7 version (in the db_rewrite_sql function):

  if ($distinct) {

    $dont_want = '/^\s*SELECT\s+(DISTINCT|DISTINCTROW)\s+/si';

    if (! preg_match($dont_want, $query)) {
        $my_search = "/^\s*SELECT\s+/si";
        $my_replace = "SELECT DISTINCT ";
        $query = preg_replace( $my_search, $my_replace, $query );
    }
  }

I am a new Drupaller and have no real understanding of this issue raised here but would like to thank you for taking the time to report it and offer a fix. Since you have provided code have changed the status to patch(code needs review) so this should get some faster attention rom the maintainers.

Yet another improvement to this code. We found a piece of code where select count(DISTINCT(thingy))... was being passed into this function, and it was coming out as select DISTINCT count(DISTINCT(thingy))..., which is too many DISTINCTs.

So here is the yet-again revised 5.1 version (put in database.mysql.inc and database.mysqli.inc):

/**
 * Add the the 'DISTINCT' keyword to the start of the query, if it isn't already
 * present.
 *
 * @param $table Table containing the field to set as DISTINCT
 * @param $field Field to set as DISTINCT
 * @param $query Query to apply the wrapper to
 * @return SQL query with the DISTINCT wrapper surrounding the given table.field.
 */
function db_distinct_field($table, $field, $query) {

  $dont_want = '/^\s*SELECT\b(.*?)\b(DISTINCT|DISTINCTROW)\b/si';

  // If it is not already there, we want to add the word 'distinct', unless it comes
  //  after the word "FROM" (ie. is part of a later sub-query).

  if (! preg_match($dont_want, $query, $my_matches) || preg_match("/FROM/si", $my_matches[1])) {
    $my_search = "/^\s*SELECT\b/si";
    $my_replace = "SELECT DISTINCT";

    return preg_replace( $my_search, $my_replace, $query );
  }else{
    return $query;
  }
}

...and the yet-again revised 4.7 version (still in the db_rewrite_sql function):

  if ($distinct) {
    $dont_want = '/^\s*SELECT\b(.*?)\b(DISTINCT|DISTINCTROW)\b/si';

    // If it is not already there, we want to add the word 'distinct', unless it comes
    //  after the word "FROM" (ie. is part of a later sub-query).

    if (! preg_match($dont_want, $query, $my_matches) || preg_match("/FROM/si", $my_matches[1])) {
            $my_search = "/^\s*SELECT\b/si";
            $my_replace = "SELECT DISTINCT";

            $query = preg_replace( $my_search, $my_replace, $query );
    }
  }

BTW, thank you, q_man, for setting this node to "patch (code needs review)"-- I'm also a new drupaller and didn't realise I could do that.

You're welcome.

BTW. I came across your post when I was searching for a solution that would enable me to make page with a number of tables which list images on a site, by author, associated project and keyword but only giving one sample image/ result per author, project or keyword respectively.
I have created a view which does something like this but have yet to find a way of making it turn out the single example. Would the DISTINCT command be useful here? can I code it into my views? It seems you have a good grap of PHP mysql, any ideas how I could code it? I'm afraid I'm no programmer...

to give you a better idea of what I'm trying to do my work in progress is here:

http://www.disscamera.org.uk/galleries

Thanks for the fix. In my case SELECT node_data_field_person.field_person_nid AS nid FROM {node} was incorrectly getting changed to SELECT node_data_field_person.field_person_DISTINCT(node.nid) AS nid FROM {node}. The incorrect query was being generated by the Views Module but only started failing after I started using the Organic Groups (og) module.

My site is Drupal 4.7.6, but the code looks like the code above that is marked as 5.1, and that's what I used successfully.

Can't review a nonexistent patch.

I know a bit about MySQL, but I'm fuzzy on what this function is meant to do. It states its objective only in terms of invalid syntax ("wrapping fields in distinct()" is as mentioned not possible in MySQL), and it doesn't explain what this is actually meant to accomplish in the results.

If a select query returns this:

First Name  |  Last Name   |   Year
Tom | Smith | 2005
Tom | Miller | 2006
John | Smith | 2004
Jens | Miller | 2005

What should the equivalent of "wrapping the field in distinct()" do when done to "Last Name" - this?

First Name  |  Last Name   |   Year
Tom | Smith | 2005
Tom | Miller | 2006

DISTINCT can only be used on the entire set of selected values. In our example "SELECT DISTINCT" wouldn't have an effect because no rows are exactly alike. It would only work when selecting only a subset of those fields that has duplicates. If the result above is what is intended, this is done with "GROUP BY".

However, DISTINCT can also be used inside an aggregate function like COUNT, and then it will only apply to the fields in the COUNT function.

I'd be happy to patch this, but I don't know what the function should actually do.

Did this ever get solved?

Further example:

When fed the following $query, db_distinct_field() works fine:

SELECT DISTINCT(node.nid)
FROM {node} node
LEFT JOIN {search_index} search_index ON node.nid = search_index.sid
LEFT JOIN {search_index} search_index2 ON node.nid = search_index2.sid
WHERE (%s.%s %s '%s')
AND ((search_index.word='%s' AND search_index2.word='%s'))
AND ((search_index.type='node'))

However when fed:

SELECT DISTINCT(node.nid)
FROM {node} node
LEFT JOIN {search_index} search_index ON node.nid = search_index.sid
WHERE (%s.%s %s '%s')
AND ((search_index.word='%s'))
AND (node.nid NOT IN (SELECT sid FROM search_index WHERE word in ('%s')))
AND ((search_index.type='node'))

It spits out

SELECT DISTINCT(node.nid)
FROM {node} node
LEFT JOIN {search_index} search_index ON DISTINCT(node.nid) = search_index.sid
WHERE (%s.%s %s '%s')
AND ((search_index.word='%s'))
AND (node.nid NOT IN (SELECT sid FROM search_index WHERE word in ('%s')))
AND ((search_index.type='node'))

The table and field parameters are node and nid for both examples. The example comes from views_fastsearch, the difference being the working query is 'lorem ipsum' and the broken one is 'lorem -ipsum'

OK, this is happening because of the second FROM, which allows a match on the the node.nid in the JOIN (and could allow it on the one in the AND).

Here is a patch that works on Drupal 5.7

The same code still exists in the current development version of Drupal, so moving forward for a thorough review.

Thank you, this fixed a distinct error I was getting with the alpha pager module!

Some further examples of where the MySQL versions of db_distinct_field('node', 'nid', $query) in Drupal 6.2 fails:

SELECT DISTINCT(node.nid) nid, node.type FROM becomes SELECT DISTINCT(node.nid) DISTINCT(node.nid), node.type FROM .
SELECT DISTINCT(node.nid) AS nid, node.type FROM becomes SELECT DISTINCT(node.nid) AS DISTINCT(node.nid), node.type FROM.

I expected the queries with alias to be unchanged. Or are these input-queries already invalid in MySQL if the DISTINCT keyword is supposed to be only at the start of query (not around a field)?
The example is based on a query from Views when showing uploaded files for nodes, and enbabling the "distinct" option to avoid showing a node as many times as there are attached files to it.

You may want to check the other DISTINCT-issue http://drupal.org/node/206224, in case you haven't found it.

#12 works really well for me on 5.7, and I've got multiple levels of nested selects. Marking this as RTBC. I'm rerolling the patch, but all I changed was the bad spelling of the word "DISTINCT".

colan: Is this new patch rerolled for 7.x ?

No, I suppose it's not. Good point. According to #13, the code is the same, but this should definitely be tested.

This patch needs more than review. It needs to be changed/rerolled for 7.x.
I am changing the status.
colan: Thanks for testing the one that was there though! :)

Hopefully this will also be back-ported for D5 and D6.

This function no longer exists in Drupal 7. Downgrading to Drupal 6, in case someone wants to work on it there.

Although, could this be in some way related to #284392: db_rewrite_sql causing issues with DISTINCT? Perhaps a dupe?