drupal_write_record() erroneous when running on non default database connection [#955512]

drupal_write_record() calls drupal_get_schema() which tries to load the schema from the cache. But this will fail if you run drupal_write_record() on a non default connection to a different database:

db_set_active('my_connection');
drupal_write_record( ... );
db_set_active('default');

In this case drupal does not find a cache entry on 'my_connection'. This causes drupal to try to write the schema into the cache using 'my_connection' which leads into errors because there are no cache tables.

Comment	File	Size	Author
#7	drupal_write_record-1-D6.patch	746 bytes	claudiu.cristea
#7
#5	drupal_write_record-D6.patch	815 bytes	claudiu.cristea
#5
	drupal_write_record_on _non_default_connection-6.x.patch	570 bytes	mkalkbrenner

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Damien Tournoud CreditAttribution: Damien Tournoud commented 28 October 2010 at 10:22

Status:

Needs review

» Closed (won't fix)

drupal_write_record() relies on the schema. As a consequence, it can only work on the default connection.

Comment #2

mkalkbrenner

German

🇩🇪

CreditAttribution: mkalkbrenner commented 28 October 2010 at 10:54

Status:

Closed (won't fix)

» Needs review

If the schema is already in memory drupal_write_record() works well on different connections if you integrate tables on different databases into the schema:

/**
 * Implementation of hook_install().
 */
function my_module_install() {
  $db = db_set_active('my_connection');
  // Create tables.
  drupal_install_schema(' my_module');
  db_set_active($db);
}

Is it documented anywhere, that this is not allowed?

I already saw a lot of custom modules that integrate legacy databases into drupal that way or move some tables into a different database due to security reasons. The only problem is drupal_write_record() when nothing else loaded the schema before using the default connection (which happens in most cases). The problem also seems to not occur if you use a non database cache.

An other solution might be a workaround in those custom modules like described here:
http://drupal.org/node/372308#comment-2442664

Comment #3

3 November 2010 at 12:32

Status:

Needs review

» Needs work

The last submitted patch, drupal_write_record_on _non_default_connection-6.x.patch, failed testing.

Comment #4

claudiu.cristea

Romanian

Arad 🇷🇴

CreditAttribution: claudiu.cristea commented 1 December 2010 at 09:25

Assigned:	claudiu.cristea	» Unassigned
Status:	Needs review	» Needs work

IMO this is a bug. All database API functions must work regardless if they use the Drupal main connection or other connections. Why db_query() works and drupal_write_record() not? What is the logic? There's no explanation...

The proposed patch seems to fix the issue with some changes. Here's a quick review

+++ includes/common.inc	(working copy)
@@ -3453,7 +3453,13 @@
+  // cocomore: drupal_get_schema() needs to access the cache on the default database.
+  //           now it's possible to run drupal_write_record() for different connections

Use Drupal standards on comments. http://drupal.org/coding-standards#comment

+++ includes/common.inc	(working copy)
@@ -3453,7 +3453,13 @@
+  $db = db_set_active();

Use a more appropriate name for the existing connection. Maybe $current_db (instead of $db).

+++ includes/common.inc	(working copy)
@@ -3453,7 +3453,13 @@
+  if ($db) {

If the current connection is "default" (most of cases) it seems to me that db_set_active() returns "default" not an empty string. The if check should be:
if ($current_db != 'default'). This need to be double checked at http://api.drupal.org/api/drupal/includes--database.inc/function/db_set_...

+++ includes/common.inc	(working copy)
@@ -3453,7 +3453,13 @@
+    db_set_active($db);

Change to: db_set_active($current_db);

Powered by Dreditor.

Comment #5

claudiu.cristea

Romanian

Arad 🇷🇴

CreditAttribution: claudiu.cristea commented 30 November 2010 at 08:30

Assigned:	Unassigned	» claudiu.cristea
Status:	Needs work	» Needs review

File	Size
drupal_write_record-D6.patch	815 bytes

Here's a new patch based on #4 comments.

Comment #6

claudiu.cristea

Romanian

Arad 🇷🇴

CreditAttribution: claudiu.cristea commented 30 November 2010 at 13:03

A workaround, without patching (but not safe enough...):

// Build schema into memory to avoid disk cache reading.
// This call will fill the static $schema inside drupal_get_schema()
// ...so cache_get() will be bypassed.
drupal_get_schema();

db_set_active('my_connection');
drupal_write_record( ... );
db_set_active();

This is not 100% safe. A cache clear may occur between drupal_get_schema() and drupal_write_record().

I think committing the patch will make drupal_write_record() usable also for other DB connections. Of course tables must be created with Schema API... but this is true also for the main database.

Comment #7

claudiu.cristea

Romanian

Arad 🇷🇴

CreditAttribution: claudiu.cristea commented 30 November 2010 at 13:11

File	Size
drupal_write_record-1-D6.patch	746 bytes

Here's a better patch. Moved schema check after reverting back to the current DB connection. This assure that we wont leave the function without switching back to the current connection.

Comment #8

Damien Tournoud CreditAttribution: Damien Tournoud commented 18 November 2011 at 17:01

Assigned:	Unassigned	» claudiu.cristea
Status:	Needs work	» Closed (won't fix)

See #1.

Comment #9

subu.purohit CreditAttribution: subu.purohit commented 9 March 2012 at 12:35

Version:

6.x-dev

» 6.24

Hi all,

Can I use drupal_write_record() function from a simple php script file. I have to run a php script which updates my node_revisions table. I can not add this code into .module file (within drupal ~~struction~~ structure) to avoid extra load because script is very heavy.

My php script exists in my project root folder and I have included "includes/common.inc" and "includes/cache.inc" but now it is giving error for undefined "variable_get()".

Should I keep adding these inc files or it is not possible to use drupal_write_record() from outside drupal ?

Thanks in advance.