I can't get the tests added by #2647470: Write tests to pass on sqlite. postgres and mysql work fine. Debugging has lead me to realise that the way SQLBase::mapjoinable() works doesn't really do it for sqlite. Whilst SQLite can attach any database to each other it is not as simple as the assumptions made in SQLBase::mapjoinable(). These assumptions are that if the database settings have the same host, username, password and port settings that you can query the source and destination databases in the same query. SQLite databases don't actually have any of these settings. They only have the database setting (which points to a file) and the prefix. If SQLite databases have the same file but different prefixes then they can be joined by the existing database code - if not they can't.
We've had a similar issue before #2343213: SQLBase::mapjoinable does not support SQLite - but #2647470: Write tests is the first issue testing the migrate UI.
Only allow SQLBase::mapjoinable() for sqlite databases when the database value is the same.
None
None
None
None
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 2675000-2.patch | 2.86 KB | alexpott |
Comments
Comment #2
alexpottComment #3
alexpottThis is blocking the addition of tests in #2647470: Write tests which will block the migrate UI in core... so this is a migrate critical.
Comment #4
gábor hojtsyWell, as per https://www.sqlite.org/lang_attach.html, ATTACH/DETACH works to add another database (to make them joinable). That sounds like is a bigger problem to solve though and I understand we should make the code properly test against what it currently supports. The fixes are very straightforward based on existing logic in mapJoinable() and #2343213: SQLBase::mapjoinable does not support SQLite. Also the added test coverage of cross-database joinability is good.
Comment #5
alexpottComment #6
catchThis looks fine. Just committed to 8.1.x for now, so leaving RTBC against 8.0.x for a cherry-pick later on.
Comment #8
catch