Issue with db_table_exists function with shared user configurations and MySQL

Same issue here.

I'm surprised that the last message here is from March 12th.

Today I did a prefix on the cache* tables and BOOM my site went down. No error on screen, HTTPD process maxingout CPU.

Problem: bug in cache.inc

There are numerous queries like this:

db_query("INSERT INTO {%s} bla bla more query here with variables like id=%d AND thingie='%s'", $table, $id, $thingie");

this query is handled by drupal in this way:
1. table_prefixing
2. variable replacement
3. query execution

so in the first step {%s} is being tried to prefix, which obviously doesn't do anything, then in the second step 'cache' is inserted between the braces. Finally the query is executed with an un-prefixed table name where a prefixed table name should have been used.

Workaround:

replace all {%s} in cache.inc with {".$table."}
AND
remove the $table variable from the db_query arguments; the 2nd argument, shouldn't be hard to spot.

Problem solved.

I have set this bug to critical cause it undermines the entire database system (prefixes and dbsharing).

below are all occurences i could find, im running CVS version 1.5 of the file cache.inc

Line 20 :     db_query("DELETE FROM {%s} WHERE expire != %d AND expire <= %d", $table, CACHE_PERMANENT, $cache_flush);
Line 24 :   $cache = db_fetch_object(db_query("SELECT data, created, headers, expire FROM {%s} WHERE cid = '%s'", $table, $key));
Line 95 :   db_query("UPDATE {%s} SET data = %b, created = %d, expire = %d, headers = '%s' WHERE cid = '%s'", $table, $data, time(), $expire, $headers, $cid);
Line 97 :     @db_query("INSERT INTO {%s} (cid, data, created, expire, headers) VALUES ('%s', %b, %d, %d, '%s')", $table, $cid, $data, time(), $expire, $headers);
Line 144 :         db_query("DELETE FROM {%s} WHERE expire != %d AND expire < %d", $table, CACHE_PERMANENT, time());
Line 150 :       db_query("DELETE FROM {%s} WHERE expire != %d AND expire < %d", $table, CACHE_PERMANENT, time());
Line 159 :         db_query("DELETE FROM {%s} WHERE cid LIKE '%s%%'", $table, $cid);
Line 163 :       db_query("DELETE FROM {%s} WHERE cid = '%s'", $table, $cid);

these have to be replaced by:

Line 20 :     db_query("DELETE FROM {".$table."} WHERE expire != %d AND expire <= %d", CACHE_PERMANENT, $cache_flush);
Line 24 :   $cache = db_fetch_object(db_query("SELECT data, created, headers, expire FROM {".$table."} WHERE cid = '%s'", $key));
Line 95 :   db_query("UPDATE {".$table."} SET data = %b, created = %d, expire = %d, headers = '%s' WHERE cid = '%s'", $data, time(), $expire, $headers, $cid);
Line 97 :     @db_query("INSERT INTO {".$table."} (cid, data, created, expire, headers) VALUES ('%s', %b, %d, %d, '%s')", $cid, $data, time(), $expire, $headers);
Line 144 :         db_query("DELETE FROM {".$table."} WHERE expire != %d AND expire < %d", CACHE_PERMANENT, time());
Line 150 :       db_query("DELETE FROM {".$table."} WHERE expire != %d AND expire < %d", CACHE_PERMANENT, time());
Line 159 :         db_query("DELETE FROM {".$table."} WHERE cid LIKE '%s%%'", $cid);
Line 163 :       db_query("DELETE FROM {".$table."} WHERE cid = '%s'", $cid);

if you are using a search and replace capable editor (most of you would and the rest should :P)
replace{%s} with {".$table."}
replace", $table, with ",

do NOT replace {%s} with {$table} because this is PHP for the protected variablename $table and will evaluate to the value of $table without the braces. You have been warned :P

The fix described in the last comment already exists in Drupal 5.x.

I guess I'm re-opening this issue because RMPEL's description doesnt adequately solve the origional problem, and based on DRUMM's response, appears to have blurred the issue. The fix to the actual issue is to do this or something similar:

function db_table_exists($table) {
	$table=explode(".",db_prefix_tables("{" . db_escape_table($table) . "}"));
	if(count($table) > 1)
	  return db_num_rows(db_query("SHOW TABLES  in ".db_escape_table($table[0])." LIKE '" . db_escape_table($table[1]) . "'"));
	else
	  return db_num_rows(db_query("SHOW TABLES LIKE '{" . db_escape_table($table[0]) . "}'"));
}

note that for prefixed tables that are pointing to a different database, the actual database being pointed to is used in the 'in' clause of the "SHOW TABLES" command and places the table portion in the 'LIKE' portion. if the table is not prefixed, it'll just go through the normal show tables command.

This is a big issue for my group because we deploy many sites on a single drupal install, and each site relies on the cache tables not being shared across all sites. This works great untill we try to run update.php on a site, and it attempts to rebuild the cache tables, incorrectly thinking they dont exist.

Hope this helps & thanks in advance for anyone who revises this or converts it into a patch for me!
-Chris

Dear netw3rker

I must admit it's been a long time since I wrote my comment, but reading back, I think you are confusing two separate issues (and maybe i did as well, when i commented it).

Please correct me if i'm wrong but this is how I see it;

The topic starter uses (or abuses) the prefix possibility to save data in different databases (a table referenced like: databasename.tablename).

Your comment

example;
prefix: drupal.
table: cache
when a query accesses {cache}, the drupal query handler will replace this with drupal.cache
no problem there; if one chooses to do this, he/she would have to realise that this is in fact database selection, not table prefixing, and that this causes a problem if multiple sites refer to the same drupal.cache table.

the function you quoted will be used as follows;
$cache_table_exists = db_table_exists("cache");
there is no database selector here, so the "false" conclusion of the if statement will be used.
This will create the query:
SHOW TABLES LIKE '{cache}'
The query handler will replace {cache} with drupal.cache, resulting in the query
SHOW TABLES LIKE 'drupal.cache'
and that will produce a negative response.

if called explicitly with $cache_table_exists = db_table_exists("drupal.cache"); the result will be SHOW TABLES in `drupal` LIKE 'cache' and this will work
This is done by the "true" conclusion of the if statement.

My comment

The problem I wrote down here is a completely different one.
( prefix: drupal_ which is a prefix as intended, not a database selector )
db_query("DELETE FROM {%s} WHERE expire != %d AND expire <= %d", $table, CACHE_PERMANENT, $cache_flush); will cause drupal to first prefix tables, then replace variables. Not the other way round. So the tablename is inserted after the prefixing is done resulting in an unprefixed table name.

This particular problem is acknowledged by Drupal and has been fixed, since it is both non-functional and not according code conventions.

Back to the original problem

The original problem is one during install; the queries involve creation of cache tables.

Now, if two separate sites use the same prefix as the topicstarter has defined, namely 'sitedb.', both sites will try to create tables called 'cache' in the database 'sitedb.'. First one succeeds, second one fails as they are already in place.

Using a database selector as a prefix is the same as not prefixing.

your comment

This is a big issue for my group because we deploy many sites on a single drupal install, and each site relies on the cache tables not being shared across all sites.

To have your cache tables split per site, a.k.a. prefixed, within the same database (single drupal install)
site a: prefix 'foo_'
site b: prefix 'bar_'
will result in site a using foo_cache* tables and site b bar_cache* tables.

This WILL work, or rather this SHOULD work, as this is exactly the code that is bugged and (if i'm not mistaken) fixed in drupal 5.3 or 5.4. (not sure atm)

This particular problem can be described as: Drupal does not accept prefixes for cache tables.

While you have the same problem as I did, the code you quoted is not responsible for the bug. The code you quoted is responsible for the unwanted behaviour the topicstarter is experiencing.

Checking the installer;
system.install correctly issues a query "create table {cache} ..." so the installer is ok
cache.inc (just checked) is patched in version 5.2 of drupal now correcty generating queries like "insert into {cache} ..." so from version 5.2 and up cache.inc is ok

For table prefixing (the prefix does NOT end in a . ) there is no problem whatsoever
if you want to use the same database for all caches (I can see why that would be appealing) use
"sharedcachedatabase.sitea_" as a prefix for site a and "sharedcachedatabase.siteb_" for site b and all the cache tables will be in the same database with different prefixes for all sites, ergo, there will be no sharing of these tables between sites.

Use of a prefix in the same database; there is no . in the prefix;
prefix "sitea_" for site a and "siteb_" for site b will keep the cache tables in the same database as the rest of the site but still separated.

So, to summise:
1. if you use a database selector, dont forget to add a table prefix
2. if you use a prefix to separate the tables between sites, make sure the prefixes are not identical

I'm nothing if not redundant. Also I repeat myself.

Hope my hastedly typed response helps both you and the topicstarter :)

Remon.

There is no patch attached. Please do not use the 'patch (...)' statuses when there is no patch.

db_table_exists() is not called during installation for any table, including cache.

It is best to keep your prefixes different during installation, as summarized at the end of #5. If you want to share tables after installation, change the prefix array and manually remove the unneeded table.

Drupal 5.x does not support shared tables during installation. Newer versions might. If this is changed in Drupal 6.x, a backport would be considered.