I found my apache log flooded with warnings. On closer inspection looks like db_rewrite_sql inserts DISTINCT where it does not belong (inside LEFT JOIN statement). Here is rewritten SQL out of the logfile:

SELECT ht.* 
FROM node n 
	LEFT JOIN helptip ht ON ht.DISTINCT(n.nid) = n.nid 
	LEFT JOIN helptip_user_data ht_hidden ON ht_hidden.uid=0 AND ht_hidden.nid=n.nid AND ht_hidden.relation='hidden'  
	INNER JOIN node_access na ON na.nid = n.nid  
WHERE 	(na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all') OR 
	(na.gid = 0 AND na.realm = 'og_public') OR 
	(na.gid = 0 AND na.realm = 'og_all'))) AND 
		n.status = 1 AND 
		ht_hidden.relation IS NULL AND 
		('node/28189' LIKE ht.path OR 'feed/items/system/2007/03/13/borat_from_the_cutting_room_floor' LIKE ht.path) AND 
		weight >= -10 AND weight <= 10 

ORDER BY ht.weight, RAND() LIMIT 1 

The problem is regular expression in db_distinct_field, which does not understand the SQL fed to it by helptip. The fix is trivial tweak of one line where SELECT statement is:

Original code (excerpt):

    $result = db_query(db_rewrite_sql("SELECT ht.* FROM {node} n ...

Replacement code:

    $result = db_query(db_rewrite_sql("SELECT nid FROM {node} n ...

Attached module file can be commited to 4.7. This still needs to be propagated into 5.0.

CommentFileSizeAuthor
#4 127433.diff1.49 KBDave Cohen
helptip.module.txt13.67 KBdkruglyak

Comments

Dave Cohen’s picture

Assigned: Unassigned » Dave Cohen
Status: Reviewed & tested by the community » Needs review

I'm travelling at the moment. Will commit this when I get a chance, but not sure when that will be.

Thanks for the detailed report and fix. In the future please try to submit patches in the form of a diff, rather than a complete module file.

dkruglyak’s picture

yogadex, there is really not much to review here.

It is a trivial change of select column syntax (4 characters) which is not material but is needed to pass db rewrite muster.

I will try to learn how to create patch files properly...

dkruglyak’s picture

Status: Needs review » Reviewed & tested by the community

Could we commit the fix to 4.7 and 5.1? Trivial and ready to go...

Dave Cohen’s picture

StatusFileSize
new1.49 KB

Here's the actual patch I've checked in (to DRUPAL-4-7, DRUPAL-5 and HEAD). It's slightly different from your original, so please open this bug again if your log warnings persist.

Thanks for the reminder, I was out of the country for a couple months. I suspect this is not the only thing I dropped the ball on.

Dave Cohen’s picture

Status: Reviewed & tested by the community » Fixed

Forgot the change status.

Anonymous’s picture

Status: Fixed » Closed (fixed)