Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Create new database with a table:
CREATE DATABASE test;
CREATE TABLE test (id INT AUTO_INCREMENT PRIMARY KEY COMMENT 'Primary ID of the TEST');
GRANT ALL PRIVILEGES ON test.* TO 'test'@'localhost' IDENTIFIED BY 'test';
settings.php
<?php
$databases['default']['test'][] = array (
'database' => 'test',
'username' => 'test',
'password' => 'test',
'host' => 'localhost',
'port' => '',
'driver' => 'mysql',
'prefix' => '',
);
?>
Test:
<?php
$comment = Database::getConnection('default')->schema()->getComment('users', 'uid');
var_dump($comment);
$comment = Database::getConnection('test')->schema()->getComment('test', 'id');
var_dump($comment);
?>
Expected result:
string(28) "Primary Key: Unique user ID."
string(11) "Primary ID of the TEST"
Actual result:
string(28) "Primary Key: Unique user ID."
bool(false)
patch coming up ...
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 1891728-4-mysql-get-comment.patch | 1022 bytes | gielfeldt |
| #1 | 1891728-1-mysql-get-comment.patch | 945 bytes | gielfeldt |
Comments
Comment #1
gielfeldt CreditAttribution: gielfeldt commentedComment #2
gielfeldt CreditAttribution: gielfeldt commentedComment #3
mcrittenden CreditAttribution: mcrittenden commentedLooks fine to me and seems to work as expected.
Comment #4
gielfeldt CreditAttribution: gielfeldt commentedHere's a D8 version too ...
Comment #5
gielfeldt CreditAttribution: gielfeldt commentedSetting to RTBC since patch is identical to the already reviewed 7.x patch, and tests have passed.
Comment #6
catchLooks fine. Committed/pushed to 8.x, moving to 7.x for backport.
Comment #7
c960657 CreditAttribution: c960657 commentedD7 patch (in comment#1) is identical to D8 patch.
Comment #8
David_Rothstein CreditAttribution: David_Rothstein commentedThis seems to make total sense, but it's also one of the scarier-looking one-line patches I've seen in a while, given its potential for breaking things :) And it hasn't gotten too much in the way of detailed reviews and testing (or comments by people who wrote the code originally), so I think I should hold off committing it to Drupal 7 for a bit in case there's something we're missing here.
I guess my biggest question is how do you hit this issue in practice? AFAIK, database targets are intended to be used for e.g. master/slave setups, in which case the database schema should be the same between the two targets anyway, and therefore it doesn't matter much if the wrong one is queried.
Comment #9
David_Rothstein CreditAttribution: David_Rothstein commentedWhen I was looking at this I realized (and confirmed) that this issue happens with the findTables() method too.
Comment #10
gielfeldt CreditAttribution: gielfeldt commentedI stumbled upon it while I was developing AutoSlave. I believe the situation arose, when it was actually a connection called [default][master] that was being used for the queries, rather than [default][default] which is part of what AutoSlave does.
There are probably no code paths in core that triggers this error. I don't think we missed something here, I just think the use-case posted in this thread has never arosen.
Comment #11
gielfeldt CreditAttribution: gielfeldt commentedFWIW the postgres driver of course does not have this issue (as the title implicitly states).
Comment #12
gielfeldt CreditAttribution: gielfeldt commentedSo can this go RTBC or what?
Comment #13
gielfeldt CreditAttribution: gielfeldt commentedWhy don't we fix this? It has already been committed to D8. Currently only the MySQL implementation is broken in D7. Let's commit it! :-)
Comment #17
gielfeldt CreditAttribution: gielfeldt commented#4 has already been committed to D8. #1 for D7 passes. Let's get it in :-)
Comment #18
gielfeldt CreditAttribution: gielfeldt commentedComment #19
mgiffordThe patch in #1 is presently in a production version of a panopoly site we have. I'll check back after a few days and confirm that all is running well with this patch.
Comment #20
mgiffordThis patch is working just fine on http://openconcept.ca/
Comment #21
gregglesLooks sane to me.
Comment #22
David_Rothstein CreditAttribution: David_Rothstein commentedCommitted to 7.x - thanks!
And thanks for confirming that this is running on a production site with no problems, @mgifford. That definitely helps get this one in with some confidence.