db_table_exists() doesn't work on a site i've set up in a subdirectory w/ a db prefix of dbname.prefix_, although it works on the 'default' site. so it's throwing all kinds of errors updating 4.7 to 5.1, and throwing off the cck update (which also calls that function). a manual db_query("SHOW TABLES LIKE {'node_type'}"); also fails, but works in mysql

to test:
set up a site in 5.1 with a db prefix of 'DBNAME.PREFIX_' and run install.php
set up your user
preview a node, with the following php code w/ the php filter on (or use devel for the same effect)

<?php
if (db_table_exists('node')) {
  print 'yes';
}
?>

this works if you have a standard installation with no db prefix, or one with the db prefix of 'PREFIX_', but fails with 'DBNAME._PREFIX'. however, because the prefix of DBNAME.PREFIX_ works everywhere else in drupal, and there are some existing sites using this configuration, it should be supported for consistency, or we should give a warning to the admin (maybe in install/update) if the db prefix uses this format.

CommentFileSizeAuthor
#3 install_php_db_prefix_verify.patch.txt1.07 KBdww

Comments

dww’s picture

dww
we/he/they
commented
Title: db_table_exists() fails with db prefix of DBNAME.PREFIX_ » db_escape_table() prevents using '.' in a DB table prefix

db_lock_table() will break on a prefix that includes . too. see db_escape_table(). that's what's going on here. so, either . should be allowed in DB table names, in which db_escape_table() needs to be patched (1 byte diff), or it's being prohibited for a reason, in which case we should document that DB table prefixes must contain only alphanumeric and _. i'm not sure what's the right answer, so i'm not rolling a patch yet.

aaron’s picture

aaron commented

well, i just realized it seems to be a mysql issue, actually (using mysql 4):

this works:
show tables like 'PREFIX_node_type';

this doesn't:
show tables like 'DBNAME.PREFIX_node_type';

so maybe we should throw a warning when installing/update if the db prefix is set up like that. (i've seen that combination used on sites sharing the user and other tables).

dww’s picture

dww
we/he/they
commented
Assigned: Unassigned » dww
Status: Active » Needs review
StatusFileSize
new install_php_db_prefix_verify.patch.txt1.07 KB

yeah, this is just lame. install.php's validation of the prefix disagrees with db_escape_table(), and that's a bug. sadly, this means breaking the translation of the validation error message string, but the existing string is wrong, so there's really no alternative. i was going to just re-use db_escape_table() instead of having these slightly different regexps in both places, but db_escape_table() isn't defined in the scope of installer.php, so just fixing the regexp to match is the smaller change.

attached patch applies cleanly and works for install.php in both HEAD and DRUPAL-5 (1 line offset in DRUPAL-5).

dww’s picture

dww
we/he/they
commented
Title: db_escape_table() prevents using '.' in a DB table prefix » installer.php incorrectly allows '.' in a DB table prefix
aaron’s picture

aaron commented

Seems a shame to just disallow dots, because they can be really useful across shared table installations across different databases. If there isn't a solution, of course this would be the next best thing. However, the only case I can think of where the dot fails is in this particular instance (the show tables like...)

I've posted a question at http://forums.mysql.com/read.php?101,147694,147694#msg-147694 to see if there might be another solution (to determine if a table exists). Although you mentioned there's a similar problem with db_lock_table? Does a manual query it creates with a dot also fail? Let's see, it would be: LOCK TABLES {DBNAME.PREFIX_tablename} WRITE;

aaron’s picture

aaron commented

Seems a shame to just disallow dots, because they can be really useful across shared table installations across different databases. If there isn't a solution, of course this would be the next best thing. However, the only case I can think of where the dot fails is in this particular instance (the show tables like...)

I've posted a question at http://forums.mysql.com/read.php?101,147694,147694#msg-147694 to see if there might be another solution (to determine if a table exists). Although you mentioned there's a similar problem with db_lock_table? Does a manual query it creates with a dot also fail? Let's see, it would be: LOCK TABLES DBNAME.PREFIX_tablename WRITE;

aaron’s picture

aaron commented

oops :)

Steven’s picture

Steven commented

This has been brought up before... it has a (different) meaning both on mysql and pgsql. Should probably be documented better instead of disallowed.

dww’s picture

dww
we/he/they
commented

no matter what, it's a bug that the installer validates this one way, and db_escape_table() does something else. so, either:

- both should be changed to be pgsql vs. mysql aware, or
- both should be changed to support the lowest common demoninator

my patch accomplishes the 2nd. if someone believes strongly in the first, it'd require more work, and probably would involve larger changes than should happen in a stable release. regardless, we'll need some documentation of the change, but i believe the best place to document is in the help text and error messages of the UI itself, whenever possible. ;)

aaron’s picture

aaron commented

so my understanding of this would be that new sites run through the installer would no longer be allowed to share tables across databases. however, an existing site, or a site overriding db_prefix in the settings.php file would still be able to override the behavior (although they would need to make the fixes manually in the appropriate .install files to work around any issues cropping up from this). is this correct?

if so, then i agree that perhaps this is the way to go for now (so long as we also add appropriate messages & documentation).

aaron’s picture

aaron commented

on the other hand, db_lock_table is probably broken on these sites too, since it depends on the malformed db_escape_table. it doesn't throw any errors on our sites using shared tables across multiple databases, although i'm not sure it would w/o further testing. however, from viewing the code, i suspect the tables aren't actually being locked now, so i need to look into that more.

however, testing in mysql, the query used by db_lock_table works fine with a dot prefix, so i believe we need to change that function to allow it, since it's used everywhere you set the cache or a variable (rather than just in the installer/updater with db_table_exists).

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Fixed

Committed to 5.x.

Please feel free to reopen on 6.x or later if you are fixing the underlying issues.

dww’s picture

dww
we/he/they
commented
Version: 5.1 » 6.x-dev
Status: Fixed » Reviewed & tested by the community

This is still broken in HEAD, so it seems like we should at least commit this there, too. Same patch (#3) still applies with offset, so I'm not re-rolling it.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Needs work

Seems to me like a feature removed. As you said, a stable release would be required to have a better solution. We are just building up that stable release :)

dww’s picture

dww
we/he/they
commented
Assigned: dww » Unassigned

I have other things I must work on. If someone wants to do a better version for D6, so be it, but what's there now is broken. I predict no one will do anything more far-reaching and you'll end up applying the same patch in 2 months, only by then, it won't apply cleanly anymore. ;)

Anyway, if someone else cares about this more strongly, please assign to yourself and do something about it.

Thanks,
-Derek

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Postponed (maintainer needs more info)

Is this still an issue?

mdupont’s picture

mdupont
Primary language French
Location Switzerland
commented
Status: Postponed (maintainer needs more info) » Closed (duplicate)

Duplicate of #286986: MySQL: SHOW TABLES LIKE database.tablename doesn't work which is more active.