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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

+1, subscribing

Susurrus’s picture

There'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?

pwolanin’s picture

found the original suggestion - it was from Hannes Lilljequist http://lists.drupal.org/pipermail/development/2008-January/028498.html

kbahey’s picture

I support this trend in general.

Do we know if it breaks any queries that are already in core that are not ANSI compliant?

pwolanin’s picture

There 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.

pwolanin’s picture

FileSize
1.25 KB

re-rolled patch to resolve conflict

hswong3i’s picture

Version: 6.x-dev » 7.x-dev
Priority: Normal » Critical

+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 ;-)

catch’s picture

Category: feature » task

Now 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.

hswong3i’s picture

Status: Needs review » Reviewed & tested by the community

Anyway, I think that is no question for apply this patch, then debug buggy core queries. I guess it should be RTBC, any idea?

sun’s picture

@catch: If it wouldn't be enabled for releases, then most contrib module maintainers would not know if their SQL is ANSI compatible.

catch’s picture

@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?

Dries’s picture

Status: Reviewed & tested by the community » Fixed

I 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.

chx’s picture

Title: require ANSI mode for MySQL » require ANSI mode for MySQL -- but this many times??
Category: task » bug
Status: Fixed » Reviewed & tested by the community
FileSize
682 bytes

Running 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.

pwolanin’s picture

@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?

pwolanin’s picture

If this was not well-tested it should be rolled back and a better patch created.

chx’s picture

It 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.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Sowwy.

Anonymous’s picture

Status: Fixed » Closed (fixed)

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