I don't think there's any reason to offer mysql as a database library when mysqli is available. The only reason to use mysql when mysqli is available is to access MySQL 3.x and 4.0 databases, which Drupal 6 will not support.

CommentFileSizeAuthor
#5 no_mysql_if_mysqli_1.patch1.04 KBDavid Strauss
#2 no_mysql_if_mysqli_0.patch1.03 KBDavid Strauss
#1 no_mysql_if_mysqli.patch1.03 KBDavid Strauss
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

David Strauss’s picture

David Strauss
he/him
CreditAttribution: David Strauss commented
FileSize
no_mysql_if_mysqli.patch1.03 KB

My cvs diff produced a cabbage patch kid. This only removes mysql as an installation option when mysqli is available. It also removes a redundant call to drupal_detect_database_types().

David Strauss’s picture

David Strauss
he/him
CreditAttribution: David Strauss commented
FileSize
no_mysql_if_mysqli_0.patch1.03 KB

Updated to remove an extra space.

David Strauss’s picture

David Strauss
he/him
CreditAttribution: David Strauss commented
Status: Active » Needs review
drewish’s picture

drewish CreditAttribution: drewish commented

+1 this i a logical change, mysqli is the better way to go. i'd remove that comment along with the redundant drupal_detect_database_types() call.

David Strauss’s picture

David Strauss
he/him
CreditAttribution: David Strauss commented
FileSize
no_mysql_if_mysqli_1.patch1.04 KB

This patch removes the extraneous comment.

David Strauss’s picture

David Strauss
he/him
CreditAttribution: David Strauss commented
Status: Needs review » Reviewed & tested by the community

This is a really simple change. I think this is RTBC.

chx’s picture

chx CreditAttribution: chx commented

As most ppl have only the choice between mysql and mysqli this is a very good patch as the select box itself will disappear. If someone installs without PHP knowledge she will have no idea what mysqli whatsoever, so it's very good to automate the choosing.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

I was just following this on irc, looked in my settings.php file, and whoops - it's been mysql (no i) since we originally installed on drupal 4.5.x - I kept c&ping that line in from settings.php to settings.php every upgrade and it never crossed my mind to use mysqli until I saw this patch.

So I think it'd be more than worth adding a prompt somewhere in the upgrade process, or maybe the status logs?, suggesting people switch. Not necessarily part of this patch though.

David Strauss’s picture

David Strauss
he/him
CreditAttribution: David Strauss commented

@catch settings.php is typically not writable during upgrades, but we could put it in as a warning on the site report page if you're using mysql and mysqli is available.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

we could put it in as a warning on the site report page if you're using mysql and mysqli is available.

That sounds great. Big +1 to the idea in general.

I would've caught it as soon as I upgraded to 5.x had that warning been in place, but as it was I overlooked completely. I guess 4.5.x defaulted to mysql so I used that, and never gave it another thought afterwards. Most people just copy their settings.php back into the folder after upgrading drupal files, so you might never even look at your settings.php after installing - especially now it's web based.

Crell’s picture

Crell CreditAttribution: Crell commented

What exactly is the benefit of mysqli over mysql when you're not using any of the newer MySQL features? I don't believe we are...

David Strauss’s picture

David Strauss
he/him
CreditAttribution: David Strauss commented

@Crell

* There's no advantage for mysql over mysqli.
* We're already requiring MySQL 4.1, the minimum DB for mysqli.
* Pointless choices are bad things to ask of users.
* mysqli has cleaner code than mysql.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Good catch. Thanks, David.

David Strauss’s picture

David Strauss
he/him
CreditAttribution: David Strauss commented

@catch See #149537 for my patch to add the warning.

David Strauss’s picture

David Strauss
he/him
CreditAttribution: David Strauss commented
Assigned: Unassigned » David Strauss
Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)