Someone in their eternal wisdom added NO_ENGINE_SUBSTITUTION to the TRADITIONAL SQL mode in MySQL 5.5. I can't fathom how a totally mysql specific behaviour one way or another is more traditional than the other but this makes it impossible to use MyISAM with MySQL 5.5 (it's a pita anyways as you need to add default-storage-engine=myisam to your my.cnf as I learned from a mysql bug report...). I have timed drush si -y minimal to be about 6.5s with InnoDB (per file, on tmpfs) and 2.8s with MyISAM(on tmpfs too). Knowing the hundreds of Drupal installs created while running tests this is no triffle matter for developers. It does not affect any users in any ways, what so ever it only cuts the time needed for tests in half on MySQL 5.5. I have copied the meaning of TRADITIONAL http://dev.mysql.com/doc/refman/5.1/en/server-sql-mode.html#sqlmode_trad... from here into the patch. It's the same in 5.0 and 5.5 -- just 5.5 also has this accursed mode.
Comment | File | Size | Author |
---|---|---|---|
#4 | traditional_broken_55.patch | 1.1 KB | chx |
traditional_broken_55.patch | 845 bytes | chx | |
Comments
Comment #1
chx CreditAttribution: chx commentedFiled http://bugs.mysql.com/?id=60963
Comment #2
bfroehle CreditAttribution: bfroehle commentedAnd MySQL just turned you down. I guess that leaves us with the patch in #1.
Comment #3
Crell CreditAttribution: Crell commented*facepalm*
If we are explicitly specifying the modes, we should state why we're doing it that way rather than just using TRADITIONAL so that some well-meaning developer doesn't revert it for us later for code simplification.
Comment #4
chx CreditAttribution: chx commentedSure
Comment #5
Damien Tournoud CreditAttribution: Damien Tournoud commentedThanks, MySQL :)
Comment #6
chx CreditAttribution: chx commentedwebchick asked this to test this on every mysql version supported. That won't happen.
Comment #7
webchickI just love being completely misrepresented.
I said "this could use some testing." It passes on MySQL 5.5. It passes on whatever the testbot is using. I'd love for a couple of people running MAMP or similar to click around a bit with this patch applied and make sure it doesn't break anything, since this is changing DatabaseConnection_mysql, which affects almost everyone.
Comment #8
chx CreditAttribution: chx commentedHere's
sys_vars.cc
This is the same as the docs say. What else can I say? Merely clicking around won't ever reveal any problems with this. Either we believe MySQL docs or won't fix this. I am seriously sick of the commit process right now.
Comment #9
chx CreditAttribution: chx commentedSorry didnt mean to crosspost.
Comment #10
chx CreditAttribution: chx commentedBTW this issue is a great demonstration (together with the update helper issue recently) of everything wrong with the core process right now. Originally this was committed on what grounds? Through testing of the MySQL SQL modes? Nope. It was commited because the docs said so, the maintainers believed the docs and now on the same grounds we won't commit a fix.
Edit: put your hands up if you fully grok what the current mode line does! I can attest that I don't. I can read the MySQL manual but that's it.
Comment #11
chx CreditAttribution: chx commentedRe-read #344575: Force MySQL to run in ANSI compatability mode. and tell me where the discussions of the modes that TRADITIONAL occured. (Hint: nowhere. We were talking ANSI. TRADITIONAL just sneaked in.)
Comment #12
chx CreditAttribution: chx commentedI have to admit making a serious mistake by re-entering core development at this point. Won't happen again for some time: I downgraded to MySQL 5.1, removed the my issues block from my dashboard and I will return to spending my time on relation for another month. We will see whether core re-emerges from development hell by then.
Comment #13
claar CreditAttribution: claar commentedWow, I'm personally sorry to see you take a break from core dev, chx. You'll be missed and the project will be hindered by it -- I look forward to your return.
That said, I must assume that your frustrations run deeper than this one issue. Because what I see here from webchick is completely reasonable -- #344575: Force MySQL to run in ANSI compatability mode. was committed during active D7 development. It's irrelevant that it was committed on little evidence, since that's often where the bar is set at that point in the dev cycle. Well, reasonable for D7, at least.
Anyway, I was going to put this issue back at "needs review", but I have checked the patch against the mysql code posted in #8 and the docs and am convinced that this patch is solid -- I believe it's more than RBTC for D8, and personally I'm comfortable with it for D7 (for what my +1 is worth).
Comment #14
claar CreditAttribution: claar commentedGrr.
Comment #15
chx CreditAttribution: chx commentedNo, it's completely unreasonable to say "we do not trust documentation" and then presuming there is easy testing that would reveal bugs in this. edit: that the bot woulndt reveal. There reason I won't fixed is that if we do not trust the MySQL documentation then the only way is to write complicated tests for each of the modes and run it on the hundred plus minor versions we support on every OS.
Comment #16
gadams CreditAttribution: gadams commentedsubscribing
Comment #17
Crell CreditAttribution: Crell commentedFor the record, I'm +1 on the patch in #4.
Comment #18
catch#4: traditional_broken_55.patch queued for re-testing.
Comment #19
xjmTagging issues not yet using summary template.
Comment #20
marcingy CreditAttribution: marcingy commentedHave run this on windows 7 with mysql 5.5.8 on php 5.3.5 against the database and node creation tests and got a 100% pass rate
Comment #21
Dries CreditAttribution: Dries commentedCommitted to 8.x.
It is not clear to me if there may be side-effects for existing Drupal 7 sites. I'm just moving it to 7.x for now.
Comment #22
chx CreditAttribution: chx commentedWell, existing sites, no, unless they rely on trying to create a table with Drupal and getting an error instead :) I do not think that's normal behaviour.
Comment #23
webchickWell, creating content types and fields creates tables. As does installing modules.
Since asking for testing back in #7 (April), it looks like only marcingy actually did it, or at least did it and reported back to the issue (on Windows no less. Nice!). I played around some with my local MAMP-based setup and didn't hit any issues, and since the code sprints at DrupalCon, which were primarily focused on D8, happened without incident, this is probably good to go.
Committed and pushed to 7.x.