Schema object calls Database::getConnectionInfo(), which is dumb

Looks proper to me.

This is an API change during alpha. The procedural wrappers are very deliberately intended for accessing the default database, and possibly alternate target using the $options array.

If you want to access other databases than the currently active one, call Database::getConnection() with the properties you want and get back the raw connection object and do everything off of that. That is by design.

Actually, looking at the patch I sort of see what you're doing here. You're using the old procedural crappy wrappers for schema that we never got around to removing. :-) I'd encourage you to not use those. The schema objects are already per-connection when they're created. See the DatabaseSchema constructor which accepts the connection to which it is bound as a parameter. The target has already been selected at that point. Calling any of the utility functions or Database:: from within one of the database objects is a bug.

This looks like a "by design" to me.

$conn = Database::getConnection('slave', 'default');
$schema = $conn->schema();
$tables = $schema->findTables('foo');
// .. do other stuff here.

Really, for anything interesting or esoteric you want to just bypass the function wrappers entirely. They're just there to simplify the common case. If you're not using the common case, don't bother with them.

Note that you can also chain most of the factory methods, so the above could all be done in a single line. I broke it up so you can see what you're getting back, and you can then keep $conn or $schema around to reuse or pass around or whatever.

Also note that it is possible to have multiple slave servers defined. In that case, you will only get one of them back, and a random one each PHP request. That's by design to support multiple slave servers. If their schemas are different, though, there's a bigger problem. :-)

Oh! Well, that's because the schema in core is still based only on hook_schema(), not on any introspection of the database. hook_schema() is only for the Drupal-local database. It doesn't help with anything else.

Unfortunately we never got around to dynamically detected schemas. I'd love to do that in D8, but right now it's not possible. For MySQL there's the schema module, but I don't know what it's upgrade plans are.

Just talked to Jeremy in IRC. Turns out the *real* problem is that buildTableNameCondition() is calling Database::getConnectionInfo(), which is wrong.

I think the correct solution here is to save the driver options array in the connection object constructor, and provide a method to access it. Then update that schema method to use that rather than calling getConnctionInfo() at all, since that's wrong either way.

When that's fixed, the code I listed above should work as it is supposed to. (Right now it won't.)

Adjusting issue info as appropriate.

For MySQL there's the schema module, but I don't know what it's upgrade plans are.

I've begun the D7 port of schema module - can't say when it'll be truly functional though.

Yep, that's the approach I was talking about.

1) I'd prefer to put the $this->connectionOptions = $connection_options line further up, right before the $dsn is set. That's when it's "done", so let's save it there before we start executing setup queries.

In fact, I'm wondering if we can push that up into the parent constructor. It's not passed right now, but perhaps it should be? Hm.

2) The docblock for the variable is slightly wrong. It's a processed version of the array from settings.php. I'd say something like "the connection information for this connection object".

3) connectionOptions() needs a docblock as well.

Thanks for attacking this!

Quick thoughts on the patch:

- All the other get-functions seem to start with getXXX?

- The difference between getConnectionInfo() and getConnectionOptions() (or connectionOptions()) might be confusing. At least, it is not self-explanatory.

- We should probably have a test for this?

If the operation itself is in the parent class, then it needs to work in all cases even if we don't happen to be using it yet. Otherwise, the postgres and sqlite drivers are broken.

I've attached a new patch that sets the connection information for the other 2 drivers as well, and tweaks the docblock to be clearer.

If the bot approves then this is good to go, but I cannot now mark it RTBC myself. :-) (Jeremy can, though!)

Looks like all of Dries's feedback was addressed. Committed to HEAD, with some minor comment clean-ups in the tests for code standards conformance.