Database prefix not implemented correctly, resulting in table not found message

Bug still exists in dev, so updated to 6.x-1.x-dev.

Attaching patch, so setting status to "needs review."

I started re-writing this as suggested (adding proper %s placeholders and using variable substitution to get the tablenames into the queries properly), but as I was doing this, I realized that the code was becoming pretty illegible, since all the parts of the queries were essentially variables: tables, keys, values, all of it.

So I looked at the whole function again and realized that the switch($type), while reducing the number of queries that would have to be written (since it sets variables for the queries), was contributing to the illegibility in the end, and also assumed that the table structure for vocabulary items and term items would be the same indefinitely.

So I decided to remove that, and write a bunch more if/else statements. While this means the source code is longer, it actually represents *less* execution time since there are fewer variables being set, and the if/else is only run once per function call regardless, so it's not like we are testing the $type more than once in any one iteration of this function. Also, since terms tend to outnumber vocabularies, I test for term first, and falling back to the rarer case vocabulary type, which should further improve speed.

While I was re-engineering this function, I took the opportunity to move away from the hook_taxonomy() poorly named $array variable, and call it $item instead, and made the recursive parts of the function (where we call uuid_taxonomy() from within uuid_taxonomy()) a little more future proofed by putting return statements in front of them, in case someone adds further processing at the end of uuid_taxonomy().

Looks like this got stuck in the queue. Any ideas how to get this going again?

I just stumbled into this problem when trying to run some simpletests on code that creates uuids for taxonomy terms.

An alternative way to fix this problem is to just use double curly braces. Patch to uuid.module would look like

@@ -285,11 +285,11 @@
       if (empty($array['uuid']) || !uuid_is_valid($array['uuid'])) {
         $array['uuid'] = uuid_uuid();
       }
-      db_query("INSERT INTO {$table} ($keyfield, uuid) VALUES (%d, '%s')",
+      db_query("INSERT INTO {{$table}} ($keyfield, uuid) VALUES (%d, '%s')",
         $key, $array['uuid']);
       break;
     case 'update':
-      $existing_uuid = db_result(db_query("SELECT uuid FROM {$table} WHERE $keyfield = %d", $key));
+      $existing_uuid = db_result(db_query("SELECT uuid FROM {{$table}} WHERE $keyfield = %d", $key));
       if ($existing_uuid) {
         // If there is an existing uuid, but no new one, remove the existing one.
         if (!isset($array['uuid']) || empty($array['uuid'])) {
@@ -297,7 +297,7 @@
         }
         // If the new uuid is not the same, update it.
         elseif ($array['uuid'] != $existing_uuid) {
-          db_query("UPDATE {$table} SET uuid = '%s' WHERE $keyfield = %d", $array['uuid'], $key);
+          db_query("UPDATE {{$table}} SET uuid = '%s' WHERE $keyfield = %d", $array['uuid'], $key);
         }
         // Otherwise, don't do anything. No update necessary.
       }
@@ -306,7 +306,7 @@
       }
       break;
     case 'delete':
-      db_query("DELETE FROM {$table} WHERE $keyfield = %d", $keyfield, $key);
+      db_query("DELETE FROM {{$table}} WHERE $keyfield = %d", $keyfield, $key);
       break;
   }
 }

An alternative way to fix this problem is to just use double curly braces

@ryan_courtnage: Yes, that will make it work, but since we're using Drupal we need to do more than make it work, we should use the Drupal DB API correctly, as the patch above does.

@SomeStupid: Any idea how to get this patch into the testing queue. Drupal.org is not obeying. Thanks.

Oh, just found out that issue testing is postponted if a branch is deemed unstable. What do we need to do to get the dev branch marked stable?

@apotek

we should use the Drupal DB API correctly

Using a variable to hold the table name is not incorrect AFAIK. It's quite common, and high-quality modules such as Views, CCK and Features do it.

Yes, using a variable to hold the table name is normal, but inserting a variable straight into a query is not good practice. That's what the Drupal db api is for, to handle sql-injection and other filtering.

Don't argue with me, argue with the docs! :) http://api.drupal.org/api/drupal/includes%21database.inc/group/database/6

@SomeStupid So it looks like the dev branch here is still marked unstable, which is blocking all related patches from testing. What can we help with in order to get this whole project moving? thanks.

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.