Is it generally desirable that contributed modules use the Drupal database abstraction whenever possible when accessing external databases? That is, is it generally preferable to use db_query() and friends, instead of mysql_connect(), mysql_query(), etc. whenever possible? I personally think so at the moment, but would certainly like to hear any arguments to the contrary. An argument that it doesn't really matter is an argument against generally using Drupal's DB layer. If one agrees it is generally better to use the Drupal DB layer, there is a significant problem to doing so. The code which supports multiple databases (of which I admit to being partially the author), such as db_set_active(), requires the additional databases to be listed in an array version of the global $db_url variable (up to and including Drupal 6) and $databases (Drupal 7). This array is set in the settings.php file. When using the Drupal 6 installer, this file can be written for the user doing the install. One can assume that most installations will thus likely not involve someone hand editing that file. Yet, except for some ugly kludges involving overwriting the $db_url or $databases variable (a scalar by default) with an array form including the extra external database(s), the only way to add an external database is to hand edit that file. If one wants to write a contrib module which has all the ease of installation which D6 and D7 core have, and which uses an external database, one is rather stuck. One can either (1) kludge up the $db_url or $databases variable by overwriting it, (2) require the user to expertly hand-edit the settings.php file or (3) not use the Drupal DB abstraction layer. Or so it seems. The contrib modules I've looked at so far mostly resort to #3, not using the DB abstraction. I'd especially like to chat with the folks working on the DB layer for D7. It would be nice to provide an interface to allow adding an additional DB programmatically from the contrib module. ..chris
When writing integration modules that are connecting to external data sources in non-supported drupal db languages (MSSQL, orcale, etc) , I have generally used a separate database abstraction layer, such as ADODB or PEARDB. Or better yet, implement an xmlrpc or restxml layer between your app and the external database, so that the External DB doesn't even need to be hosted on the same box as the drupal code tree. I wouldn't call these anointed best practices, but it certainly is the approach that has kept the integration code the cleanest for me. Dave On Aug 21, 2008, at 8:21 PM, Chris Johnson wrote:
Is it generally desirable that contributed modules use the Drupal database abstraction whenever possible when accessing external databases? That is, is it generally preferable to use db_query() and friends, instead of mysql_connect(), mysql_query(), etc. whenever possible?
I personally think so at the moment, but would certainly like to hear any arguments to the contrary. An argument that it doesn't really matter is an argument against generally using Drupal's DB layer.
If one agrees it is generally better to use the Drupal DB layer, there is a significant problem to doing so. The code which supports multiple databases (of which I admit to being partially the author), such as db_set_active(), requires the additional databases to be listed in an array version of the global $db_url variable (up to and including Drupal 6) and $databases (Drupal 7). This array is set in the settings.php file.
When using the Drupal 6 installer, this file can be written for the user doing the install. One can assume that most installations will thus likely not involve someone hand editing that file. Yet, except for some ugly kludges involving overwriting the $db_url or $databases variable (a scalar by default) with an array form including the extra external database(s), the only way to add an external database is to hand edit that file.
If one wants to write a contrib module which has all the ease of installation which D6 and D7 core have, and which uses an external database, one is rather stuck. One can either (1) kludge up the $db_url or $databases variable by overwriting it, (2) require the user to expertly hand-edit the settings.php file or (3) not use the Drupal DB abstraction layer.
Or so it seems. The contrib modules I've looked at so far mostly resort to #3, not using the DB abstraction.
I'd especially like to chat with the folks working on the DB layer for D7. It would be nice to provide an interface to allow adding an additional DB programmatically from the contrib module.
..chris
On Thursday 21 August 2008 10:21:37 pm Chris Johnson wrote:
Is it generally desirable that contributed modules use the Drupal database abstraction whenever possible when accessing external databases? That is, is it generally preferable to use db_query() and friends, instead of mysql_connect(), mysql_query(), etc. whenever possible?
Yes, very much so. All of the security system is built into the abstraction layer; using mysql_query() directly means you're totally on your own for security.
I'd especially like to chat with the folks working on the DB layer for D7. It would be nice to provide an interface to allow adding an additional DB programmatically from the contrib module.
..chris
Well hi there! :-) What you describe was absolutely an issue in the D6 and earlier DB layer. The new DB layer is still not perfect, but has improved on it in two ways: 1) The $databases value is always an array. It can be a flattened array, as some layers are optional, but since the installer populates it in the first place it will in almost every case already be an array. 2) You can actually instantiate a connection object directly. I wouldn't recommend it as then the core system can't switch to it, but it's possible. :-) 3) (Nobody expects the Spanish Inquisition!) The db_*() functions are all dumb wrappers. You can grab a database connection with Database::getConnection($key) and then do everything after that with method calls; $db->query(), $db->insert(), $db->select(), etc. So if you want to use a given database connection and just pull yourself out of the Drupal "active database" definition entirely, you can do so. It's still not perfect, though, I agree. If you have a suggestion for how to make the connection management more flexible without hurting performance (it is along the critical path, so we have to be careful) please post an issue. Just because the mega-patch landed doesn't mean we're done changing things yet. :-) -- Larry Garfield larry@garfieldtech.com
I'm just in the process of writing a osCommerce api for Drupal. I started my work by using db_set_active(). Unfortunaltey this only works as long there are now problems in the code. (Which of course shouldn't be the case for production). Let's make an example: db_set_active('oscommerce'); // read oscommerce data db_set_active(); If an error occurs during the oscommer data access the drupal DB is not found any more. You'll get error messeage telling you that table watchdog couldn't be found. I thing we should use the db_* methods because of security, but replacing the active DB isn't fail proof. Best Regards Ernst 2008/8/22 Larry Garfield <larry@garfieldtech.com>
On Thursday 21 August 2008 10:21:37 pm Chris Johnson wrote:
Is it generally desirable that contributed modules use the Drupal database abstraction whenever possible when accessing external databases? That is, is it generally preferable to use db_query() and friends, instead of mysql_connect(), mysql_query(), etc. whenever possible?
Yes, very much so. All of the security system is built into the abstraction layer; using mysql_query() directly means you're totally on your own for security.
I'd especially like to chat with the folks working on the DB layer for D7. It would be nice to provide an interface to allow adding an additional DB programmatically from the contrib module.
..chris
Well hi there! :-)
What you describe was absolutely an issue in the D6 and earlier DB layer. The new DB layer is still not perfect, but has improved on it in two ways:
1) The $databases value is always an array. It can be a flattened array, as some layers are optional, but since the installer populates it in the first place it will in almost every case already be an array.
2) You can actually instantiate a connection object directly. I wouldn't recommend it as then the core system can't switch to it, but it's possible. :-)
3) (Nobody expects the Spanish Inquisition!) The db_*() functions are all dumb wrappers. You can grab a database connection with Database::getConnection($key) and then do everything after that with method calls; $db->query(), $db->insert(), $db->select(), etc. So if you want to use a given database connection and just pull yourself out of the Drupal "active database" definition entirely, you can do so.
It's still not perfect, though, I agree. If you have a suggestion for how to make the connection management more flexible without hurting performance (it is along the critical path, so we have to be careful) please post an issue. Just because the mega-patch landed doesn't mean we're done changing things yet. :-)
-- Larry Garfield larry@garfieldtech.com
I may not have been clear below. Using $db->query() is just as secure as using db_query(). The security handling happens much further down in the system. db_query() is now basically a dumb convenience wrapper that has really only one line of value in it (once the BC layer is removed). --Larry Garfield On Fri, 22 Aug 2008 15:19:00 +0200, "Ernst Plüss" <ernst.pluess@gmail.com> wrote:
I'm just in the process of writing a osCommerce api for Drupal. I started my work by using db_set_active(). Unfortunaltey this only works as long there are now problems in the code. (Which of course shouldn't be the case for production).
Let's make an example:
db_set_active('oscommerce'); // read oscommerce data db_set_active();
If an error occurs during the oscommer data access the drupal DB is not found any more. You'll get error messeage telling you that table watchdog couldn't be found. I thing we should use the db_* methods because of security, but replacing the active DB isn't fail proof.
Best Regards Ernst
2008/8/22 Larry Garfield <larry@garfieldtech.com>
On Thursday 21 August 2008 10:21:37 pm Chris Johnson wrote:
Is it generally desirable that contributed modules use the Drupal database abstraction whenever possible when accessing external databases? That is, is it generally preferable to use db_query() and friends, instead of mysql_connect(), mysql_query(), etc. whenever possible?
Yes, very much so. All of the security system is built into the abstraction layer; using mysql_query() directly means you're totally on your own for security.
I'd especially like to chat with the folks working on the DB layer for D7. It would be nice to provide an interface to allow adding an additional DB programmatically from the contrib module.
..chris
Well hi there! :-)
What you describe was absolutely an issue in the D6 and earlier DB layer. The new DB layer is still not perfect, but has improved on it in two ways:
1) The $databases value is always an array. It can be a flattened array, as some layers are optional, but since the installer populates it in the first place it will in almost every case already be an array.
2) You can actually instantiate a connection object directly. I wouldn't recommend it as then the core system can't switch to it, but it's possible. :-)
3) (Nobody expects the Spanish Inquisition!) The db_*() functions are all dumb wrappers. You can grab a database connection with Database::getConnection($key) and then do everything after that with method calls; $db->query(), $db->insert(), $db->select(), etc. So if you want to use a given database connection and just pull yourself out of the Drupal "active database" definition entirely, you can do so.
It's still not perfect, though, I agree. If you have a suggestion for how to make the connection management more flexible without hurting performance (it is along the critical path, so we have to be careful) please post an issue. Just because the mega-patch landed doesn't mean we're done changing things yet. :-)
-- Larry Garfield larry@garfieldtech.com
On Fri, Aug 22, 2008 at 12:24 AM, Larry Garfield <larry@garfieldtech.com> wrote:
On Thursday 21 August 2008 10:21:37 pm Chris Johnson wrote:
I'd especially like to chat with the folks working on the DB layer for D7. It would be nice to provide an interface to allow adding an additional DB programmatically from the contrib module.
..chris
Well hi there! :-)
Heh. I know who those are on the short list -- just didn't want to drag anyone into the game involuntarily. :-)
What you describe was absolutely an issue in the D6 and earlier DB layer. The new DB layer is still not perfect, but has improved on it in two ways:
1) The $databases value is always an array. It can be a flattened array, as some layers are optional, but since the installer populates it in the first place it will in almost every case already be an array.
This might solve most of the problem. I haven't used the new D7 layer yet, so I only made a cursory review of the code. I didn't notice that $databases is always an array. As long as a contrib module can always say something like $databases['mydatabase'] = ... without causing errors, I think we may be ok. It's essentially pushing another database onto the stack of known databases, and if that will work without core changes, contrib modules should be able to use the DB abstraction without resorting to strange code.
It's still not perfect, though, I agree. If you have a suggestion for how to make the connection management more flexible without hurting performance (it is along the critical path, so we have to be careful) please post an issue. Just because the mega-patch landed doesn't mean we're done changing things yet. :-)
If I see anything else, I'll let you know. :-) Regarding Ernst Plüss's comment about watchdog DB logging errors occurring while other, non-Drupal databases are active: I could have sworn I fixed that very problem in a patch to 4.7 core a very long time ago. In fact, I'm 99.99% sure I did. It appears there may be a regression, although I'm not sure which version of core of Ernst is referring to here. Thanks much for the reply, Larry. ..chris
The problem occurs in D5. 2008/8/26 Chris Johnson <cxjohnson@gmail.com>
Regarding Ernst Plüss's comment about watchdog DB logging errors occurring while other, non-Drupal databases are active: I could have sworn I fixed that very problem in a patch to 4.7 core a very long time ago. In fact, I'm 99.99% sure I did. It appears there may be a regression, although I'm not sure which version of core of Ernst is referring to here.
Thanks much for the reply, Larry.
..chris
participants (4)
-
Chris Johnson -
David Metzler -
Ernst Plüss -
Larry Garfield