When using multiple databases, db_error and db_affected_rows will fail to return correct values because they are called with arguments. According to php.net, this means that the last opened database is used instead of the active database.

This created a problem for us with caching, as rows had changed on the last active database, but not on the last opened one .. causing db_affected_rows to return 0 and not insert anything into the cache.

I'm attaching a patch that adds $active_db as a parameter to all of the *_last_error and *_affected_rows function calls in database.*.inc so that it will return the correct values.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rkerr’s picture

FileSize
1.21 KB

Attaching PostgreSQL patch file for database.pgsql.inc

rkerr’s picture

Issue description should say .. "called withOUT arguments" :)

rkerr’s picture

This should probably also include the fixed db_escape_string() for mysql to use

global $active_db;
mysql_real_escape_string($text, $active_db);

instead of the current (as of 4.6.3) addslashes function.

Cvbge’s picture

Hi.

What about mysqli patch?

Connection part:
+ $connection = pg_connect($conn_string) or die(pg_last_error($connection));
+ $connection = mysql_connect($url['host'], $url['user'], $url['pass'], TRUE) or die(mysql_error($connection));
If *_connect() function fails it returns FALSE. Do pg_last_error() or mysql_error() work when you give them FALSE? I doubt it (it does not work for postgresql at least; php.net documentation is wrong)
(Current code is wrong too IMO)

Other than that, seems ok.

rkerr’s picture

You're right about the connect() failing. It's pointless to check for an error when the connection wasn't even made.
I hadn't considered mysqli because I was only looking at 4.6. . . but it looks like this issue still applies to 4.7 head so I can make more patches.

m3avrck’s picture

Version: 4.6.3 » x.y.z
Status: Needs review » Needs work

Will this be backported to 4.6.4? Not sure, but this is a good fix for CVS. How about a MySQLi too and just one patch to fix them all? :-)

rkerr’s picture

The patches I originally attached are 4.6.3... But I'll repatch against 4.6.4 as well as 4.7 HEAD (including mysqli version). That seems to make the most sense.

rkerr’s picture

Looks like the mysqli file in 4.7 HEAD already has $active_db for all the functions.

So only mysql and pgsql need to be patched.

rkerr’s picture

FileSize
2.83 KB

Ran "diff -rupN old new > tree.diff" between the tarball source trees for 4.6.4 against a copy with my updates to database.mysql.inc and database.pgsql.inc.

Attaching patch file for 4.6.4...

rkerr’s picture

FileSize
3.62 KB

Ran "diff -rupN old new > tree.diff" between the tarball source trees for 4.7 HEAD (CVS) against a copy with my updates to database.mysql.inc and database.pgsql.inc.

Attaching patch file for 4.7 HEAD (CVS)...

rkerr’s picture

Status: Needs work » Needs review
rkerr’s picture

New patches to sync with HEAD

rkerr’s picture

New patch to sync with HEAD (postgresql version)

Dries’s picture

Committed to HEAD. Thanks.

rkerr’s picture

Attaching patch against DRUPAL-4-6 cvs for Mysql.

rkerr’s picture

Attaching patch against DRUPAL-4-6 cvs for PostgreSQL.

Dries’s picture

Status: Needs review » Fixed

Committed to DRUPAL-4-6. Thanks.

rkerr’s picture

Lovely! Many thanks.

Anonymous’s picture

Status: Fixed » Closed (fixed)