Don't you know it, 10 minutes after releasing 2.3 I find a new bug... In theory, with a MySQL source, if the legacy database is on the same server and has the same credentials as your Drupal database, in MigrateSourceSQL you can let mapJoinable default to TRUE and the map table will be incorporated into the base query, which can be a significant optimization for big migrations. Unfortunately, it doesn't work - because getQualifiedMapTable(), in a misguided optimization, does not qualify the map table when it's in the default database. Patch coming...

CommentFileSizeAuthor
#6 fix-map-join_1480762-6.patch1.47 KBmikeryan
#1 fix-map-join_1480762_1.patch659 bytesmikeryan

Comments

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Active » Needs review
StatusFileSize
new fix-map-join_1480762_1.patch659 bytes
apotek’s picture

apotek commented

Mike, thank you so much for release 2.3.

Wondering if I should patch my 2.3 right now, or if you are planning on committing this change back to the release.

Thanks.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented

I don't think there's a way to patch a stable release - at any rate, doing so would be kind of hinky. I suggest using the patch (or the -dev release after it's committed).

I didn't get a chance to fully test it, hence the "needs review" status. If you do test it and are satisified, please update to the rtbc status and I'll commit it.

Thanks.

apotek’s picture

apotek commented

I won't be able to test it adequately at the moment since my setup doesn't resemble the use-case and would require significant retooling to get there.

If no one shows up to test it, I might have some time to try to create a scenario in a couple weeks.

I've put all my updates to Migrate in a non-production branch so far, was curious only about the patch/release issue in the sense that I wanted to be working with a sanctioned milestone release when I finally roll this out to production, but I understand that's not possible.

Anyone else able to test this?

Also, I'm on D6. I don't see any reason why the patch wouldn't be correct on either platform since the code swap looks identical, but perhaps it would be good to patch your D6 dev release with this while you're at it.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs review » Needs work

Actually, this change breaks simpletest completely - it would need to handle prefixes to work in the general case.

Yes, when the fix is made it will be added to D6 as well.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs work » Needs review
StatusFileSize
new fix-map-join_1480762-6.patch1.47 KB

Here's another shot at it - seems to work for normal db-to-db migrations, and in simpletest. It will not work for a migration from an external database into a prefixed Drupal installation (but then it never did, so no regression) - in those cases you'll have to use mapJoinable => FALSE.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs review » Fixed

Committed to D6 and D7.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.