db_set_active() does not reliably return the previous db's name [#168315]

Might just be mysql context, but it seems that when
array_search($previous_db, $db_conns)
finds the first 'equal' connection, it always finds default even when different db's are connected.

This isn't a problem when only one switch occurs, because the db to switch back to is always 'default', but if two set_active_db frames are nested, then the inner call never switches back to the middle db context.

The attached patch fixes this by explicitly storing the previous name.

Comment	File	Size	Author
#11	fix_setactive5.patch	839 bytes	schuyler1d
#11
#7	fix_setactive4.patch	843 bytes	schuyler1d
#7
#3	fix_setactive3.patch	768 bytes	schuyler1d
#3
#1	fix_setactive2.patch	1.14 KB	schuyler1d
#1
	fix_setactive.patch	1.14 KB	schuyler1d

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

schuyler1d CreditAttribution: schuyler1d commented 20 August 2007 at 15:51

File	Size
fix_setactive2.patch	1.14 KB

sorry, this one's better

Comment #2

drumm

he/him

NY, US

CreditAttribution: drumm commented 27 August 2007 at 02:42

Priority:	Critical	» Normal
Status:	Needs review	» Needs work

I don't think this is critical since not many people connect to more than two databases.

Please use diff -uF. For more information, see http://drupal.org/patch/create.

I think the code might be okay, but I'm not used to reading patches formatted this way.

Comment #3

schuyler1d CreditAttribution: schuyler1d commented 27 August 2007 at 19:13

File	Size
fix_setactive3.patch	768 bytes

sorry for the badly formatted patch. This one should be better.

Comment #4

schuyler1d CreditAttribution: schuyler1d commented 27 August 2007 at 19:18

just for the record, I had marked it critical, because it came up using module Multisite Manager--basically any time something switches active_db and makes a call to watchdog() or anything that calls watchdog() ends up losing its db context.

Comment #5

schuyler1d CreditAttribution: schuyler1d commented 28 August 2007 at 14:29

Status:

Needs work

» Needs review

oops, forgot to change the status

Comment #6

nedjo

he/him/his

English

CreditAttribution: nedjo commented 5 September 2007 at 00:43

Status:

Needs review

» Needs work

This indeed looks like a bug. The problem if I understand it correctly is that the array of connections we're caching is really an array of resource db link identifiers. Assuming two schemas on the same db server, the resource is the same; it's the same connection to a db server, all we've done is switch active databases.

The proposed solution looks good.

This line is now superfluous and so should be deleted:


   $previous_db = $active_db;

Comment #7

schuyler1d CreditAttribution: schuyler1d commented 5 September 2007 at 16:27

Status:

Needs work

» Needs review

File	Size
fix_setactive4.patch	843 bytes

true, updated patch removes superfluous line.

Nedjo's description is correct as I understand it, as well.

Comment #8

schuyler1d CreditAttribution: schuyler1d commented 27 November 2007 at 20:03

Status:

Needs review

» Reviewed & tested by the community

trying to ping core...

Comment #9

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 10 December 2007 at 18:46

schuyler1d also opened an issue for 6.x, which I marked as duplicate of this one.

I looked into this as well, and what the current code does is basically looking for the resource identifier in the resource list, looking up the key used for that. So the question is when new database connection resource identifiers are created in PHP. This could be database dependent even, a new connection with the same user/pass might return a new resource ID, but it is more likely that it will return the same.