db_distinct_field mangles some queries

This issue still exists in 5.7. This issue is most noticable if a module is controlling node access (such as og or tac, etc), which causes node_db_rewrite_sql() to return a value saying that the query must be distinct, which will result in replacing every occurrence of "

This becomes a serious problem for modules which provide views integration and define table aliases that end with a string that happens to match the table/field being made distinct. I've been working on views integration for the project_issue module and started running into this bug when testing in systems implementing node access. In that case, the table alias was 'project_issue_project_node', which meant that 'project_issue_project_node.nid' was being transformed into 'project_issue_project_ DISTINCT(node.nid)' -- problem.

The patch provided works for the mysqli engine, but a similar change is needed for mysql and possibly pgsql. I'll see what I can do.

I duplicated the patch to the other two database include files and redid the patch against 5.7 and so it applies to all three includes. This seems to fix the problems with mysql and mysqli -- however, I'm not in a position to test pgsql right now.

If someone could please test with PostgreSQL and confirm that this fix is a valid approach, perhaps we can get this into core.

I just posted a reply at #229831: db_distinct_field() fails to recognize table name and then realized that this issue was older, so I'm setting that one to be a duplicate of this issue. I'll attach the same patches I attached there, which I marked RTBC. The patch for D5 is just a reroll of Josh's patch in #2. Again, D5 patch tested on mysql and fixes the problem. D6 patch untested but makes the same changes, so should also work.

I'm seeing this behaviour on one system but not another. The queries in question look this:

SELECT count(DISTINCT(node.nid)) FROM node node LEFT JOIN node_field_multireference_data field_coffee_friends ON DISTINCT(node.nid) = field_coffee_friends.nid AND field_coffee_friends.field_name = 'field_coffee_friends' WHERE (node.type IN ('coffee_rating')) AND (( (field_coffee_friends.r_id IN (SELECT uid FROM users WHERE UPPER(users.name) LIKE UPPER('%tamm%'))) OR UPPER(field_coffee_friends.r_text) LIKE UPPER('%tamm%') ))

db_distinct_field() has added the syntactically incorrect DISTINCT() around in the ON condition. The same thing happens for non-count queries.

The system where I see this problem is running Drupal 5.7 on Fedora Core 5 (PHP 5.1.6). And the SQL is mangled even with the above patch applied.

On a Debian stable (PHP 5.2.4) system running Drupal 5.6, the queries work fine.

Are there differences in PCRE compatibility for these sort of backreference conditions?

@steve.m: That sounds like a different bug to me. Is the problem that db_distinct_field() is adding DISTINCT() in two places?

The bug here is if you have a query that looks like:

SELECT pn.nid ......

it gets rewritten into the form

SELECT p DISTINCT(n.nid)

which is not valid.

One way to functionally test this patch is to do the following (this is what I have done):
1. Create a clean Drupal 5.x site using the drupal.org testing installation profile.
2. Install and enable a node access control module (I used simple access but others should also work).
3. Rebuild the node_access table.
4. Before installing the patch here, if you navigate to /project/issues/statistics you should see a SQL error.
5. After applying the patch, the error should not be there any more.

Comments in this issue and the issue that was marked duplicate of this issue suggest that this problem happens in other modules as well. These modules could work around this core bug by referencing tables in queries by obscure names (for example, in the above case the original query could be SELECT pnx.nid ......), but it makes more sense to fix the bug here instead.

This may well be a different bug. Neither site uses the node access module.

The query goes in as:
SELECT DISTINCT(node.nid) FROM node node LEFT JOIN some_table t ON node.nid = ...
and becomes
SELECT DISTINCT(node.nid) FROM node node LEFT JOIN some_table t ON DISTINCT(node.nid) = ...

db_distinct_field() is adding the second, and incorrect, DISTINCT() despite that fact that the query was already selecting DISTINCT(node.nid).

(The query is being generated by views filter code in a module I'm developing, and, as I mentioned, works fine on one system but not on another.)

Heh. Just reread the original description, and this is indeed quite a different problem. Hm.

If someone other than aclight has tested this, feel free to bump the status back up.

This should be fixed in the most recent version first and backported from there. The 6.x patch in #3 does apply to CVS HEAD.

chx suggested to me that I not bother submitting this for 7.x because the db stuff he's working on would remove the need for this function in the first place. He suggested I set the version to 6.x, but as I mentioned above I didn't have a good test case for this on 6.x.

This patch didn't work for me, but the one over at http://drupal.org/node/130242 sure did. That's a related issue, so I thought I'd mention it.

The patch for D5 in #3 worked for me.
Many thanks for solving this. Now all I have to do is keep track of everything.

The patch for D5 in #3 does not apply cleanly to Drupal 5.11. The database.pgsql.inc portion fails.

We've tried implementing this patch into line 363 of our includes/database.mysql.inc file in a variety of different methods, but they all end in site failure.

We have been pasting in:

return preg_replace('/(SELECT.*)(?:\b'. $table .'\.|\s)(?<!DISTINCT\()(?<!DISTINCT\('. $table .'\.)'. $field .'(.*FROM )/AUsi', '\1 '. $field_to_select .'\2', $query);
over
return preg_replace('/(SELECT.*)(?:'. $table .'\.|\s)(?<!DISTINCT\()(?<!DISTINCT\('. $table .'\.)'. $field .'(.*FROM )/AUsi', '\1 '. $field_to_select .'\2', $query);

with no success. We even tried to implement it through our phpmyadmin interface (which I don't think is right).

Can I get a little bit more guidance in implementing this?

The regex-based approach won't even exist anymore in D7 soon enough, so this needn't be dealt with there. Bumping down to D6 to see if Gabor wants to deal with it in D6.

I still have this issue with D6 and project module + content access. Any hope it will be solved?
Tnx.