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.
The list archive seem to be lagging, but among the copmments in this and related trheads is the idea that multi-database support is easier to achieve if we use standard SQL: http://lists.drupal.org/pipermail/development/2008-January/028320.html
Someone further mentioned that MySQL does indeed have a mode that enhances its ANSI compliance - so why not use it?
Gave this a quick test with mysql, though not tested with msqli.
Comment | File | Size | Author |
---|---|---|---|
#13 | run_simpletest.patch | 682 bytes | chx |
#6 | mysql-ANSI-212918-6.patch | 1.25 KB | pwolanin |
mysql-ANSI-0.patch | 1.23 KB | pwolanin | |
Comments
Comment #1
sun+1, subscribing
Comment #2
Susurrus CreditAttribution: Susurrus commentedThere's information about ANSI compliance here: http://dev.mysql.com/doc/refman/5.0/en/ansi-mode.html.
It discusses a few other things such as the isolation level (I don't know what that is) and the
NO_BACKSLASH_ESCAPES
option. Should these settings be changed also?Comment #3
pwolanin CreditAttribution: pwolanin commentedfound the original suggestion - it was from Hannes Lilljequist http://lists.drupal.org/pipermail/development/2008-January/028498.html
Comment #4
kbahey CreditAttribution: kbahey commentedI support this trend in general.
Do we know if it breaks any queries that are already in core that are not ANSI compliant?
Comment #5
pwolanin CreditAttribution: pwolanin commentedThere are some errors here: http://testing.drupal.org/node/9445
but it's not clear whether they relate to problems in core or problems in the testbed.
Comment #6
pwolanin CreditAttribution: pwolanin commentedre-rolled patch to resolve conflict
Comment #7
hswong3i CreditAttribution: hswong3i commented+1 for D7, but -1 for D6.
Seems our core queries are most likely written in common standard (and so your patch should work fine, I didn't test it yet), but actually they doesn't. According to my current research progress, there is still a lot of syntax/coding/style/etc error found from latest D6 CVS HEAD (well, most of that are not critical in case of MySQL/PostgreSQL). Even this patch solve part of the SQL standard problem, it is far from the complete of our ultimate goal ;-(
On the other hand, I guess we may need extra time to test ALL of our existing implementation, if we restrict MySQL into ANSI compliant after D6 RC2. Says MySQL share ~90% of our existing market, we may not hope to let it become risky in current D6 development cycle.
I love the idea of this patch; BTW, we shouldn't be too hurry for that. I would like to move it for D7 but critical, so we will able to fix this ASAP ;-)
Comment #8
catchNow this is against D7 I don't see a reason for it to be held back. If HEAD gets a bit broken, then it's bugs in the queries that'll be easier to find with the patch applied. However I'm not sure it should be on for actual releases - in the same way that E_ALL is switched off.
Comment #9
hswong3i CreditAttribution: hswong3i commentedAnyway, I think that is no question for apply this patch, then debug buggy core queries. I guess it should be RTBC, any idea?
Comment #10
sun@catch: If it wouldn't be enabled for releases, then most contrib module maintainers would not know if their SQL is ANSI compatible.
Comment #11
catch@sun: do contrib authors know if their php is E_ALL compliant? I think similar arguments apply. Better than sites breaking because someone committed a syntax error that might work on MySQL and postgres but break on Oracle or something.
contrib authors can always develop alongside -dev releases no?
Comment #12
Dries CreditAttribution: Dries commentedI think this is a great idea. We can chose to disable this when we ship Drupal 7, but enabling it during the development cycle is awesome. Committed.
Comment #13
chx CreditAttribution: chx commentedRunning this query on every single successful query broke mysqli_affected_rows 'cos that checked the affected rows of the ansi setting query... As it's proper, a ton of simpletest tests broke. I spent hours hours though hunting down this one. Please please PLEASE run simpletest next time.
Comment #14
pwolanin CreditAttribution: pwolanin commented@chx - is simpltest working again? for a while I was always getting fails on 6.x.
At the top I say: "Gave this a quick test with mysql, though not tested with msqli."
I had assumed that those putting this at RTBC had done more testing. What's the right way to do this?
Comment #15
pwolanin CreditAttribution: pwolanin commentedIf this was not well-tested it should be rolled back and a better patch created.
Comment #16
chx CreditAttribution: chx commentedIt is a fine patch just the mysqli query is in the wrong place. And yes simpletests work now I just submitted one for core inclusion.
Comment #17
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Sowwy.
Comment #18
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.