node_get_by_uuid should not pass query through db_rewrite_sql

I second this patch!

I just spent a good two hours trying to figure out why node_get_by_uuid() wasn't working for a node that clearly exists. It turns out that db_rewrite_sql() was transforming the SQL into the following:

SELECT n.nid
     FROM {node} AS n
     INNER JOIN {uuid_node} AS un ON n.nid = un.nid
       WHERE (n.language ='en' OR n.language ='' OR n.language IS NULL) AND (  un.uuid = '%s')

Notice the language stuff in there. The node I was trying to look up had language of "pl". So, node_get_by_uuid() will actually work or not work, depending on the user's current language. Also, when running from drush, the current language is always set to "en". So, this prevents you from using the uuid_features module to export/import any nodes that aren't in English.

Anyway, I definitely think db_rewrite_sql() is inappropriate here. If you can node_load() it, you should be able to node_get_by_uuid() to get it. db_rewrite_sql() is more for showing a list of nodes to the user, not the low-level API stuff.

Thanks @vectoroc for writing the patch! I hope it gets accepted.

Can you please port this to use the db_select() for building the queries, I'm afraid the double quotes in the query can cause database incompatibilities. So better having db_select() generate a compatible SQL query for us.

While the approach here looks like a good improvement, nothing has happened here in a long time. Maybe it only is affecting a few people, and perhaps could be done after the beta3 release?

c.f. #1149546: Please release 6.x-beta3, as it is not possible to upgrade to Drupal-7.x if uuid-6.x-beta2 is installed

Can you please port this to use the db_select() for building the queries, I'm afraid the double quotes in the query can cause database incompatibilities. So better having db_select() generate a compatible SQL query for us.

This is a Drupal 6 patch. db_select() is a Drupal 7 function. It would definitely be appropriate in the Drupal 7 version of these same changes, but not here.

While the approach here looks like a good improvement, nothing has happened here in a long time. Maybe it only is affecting a few people, ...

Well, I think everyone that is affected by this problem is using this patch. :-) I know I'm using it on a few sites (I do lots of multilingual sites). But I'll feel better once it's accepted upstream and I don't have to worry about it anymore.

That said, if there are other critical bug fixes that need to get released, who are we to hold up the release...

The maintainer's concerns in #8 were not addressed here. The usual policy for Drupal patches is to fix for the current release (d7) first; this means that we'd need a d7 patch using db_select before this can be committed to the d7 branch. If db_select is not going to be used for d6 (it could be, but only if the dbtng module becomes a dependency -- maybe not the best idea), then it should be demonstrated that the concern with double quotes in #8 is not an issue, or fixed if it is.

Alright, I'll work on an updated patch without double quotes in the SQL.

I have a feeling this isn't a problem in Drupal 7 because the mechanism for rewriting SQL is different but I'll do some testing.

Checked out the Drupal 7 version of this module. This bug doesn't exist there as far as I can tell! The equivalent function is entity_get_id_by_uuid(). It uses db_select() and DOES NOT add any problematic tags, for example ->addTag('node_access'). (And I think node access is still respected in the entity_uuid_load() function because it uses entity_load() underneath).

So, this is a Drupal 6 only fix! I'll rework the patch in a moment.

Attached is a patch with a smaller footprint, that simply removes the problematic calls to db_rewrite_sql() without changing the SQL string any.

Drupal 6 core is no longer supported. We are no longer supporting 6.x-1.x versions of this module. I am closing this issue as won't fix.