[drupal-devel] [feature] MySQLi support
Dries
drupal-devel at drupal.org
Fri Sep 9 06:35:09 UTC 2005
Issue status update for
http://drupal.org/node/29910
Post a follow up:
http://drupal.org/project/comments/add/29910
Project: Drupal
Version: cvs
Component: database system
Category: feature requests
Priority: normal
Assigned to: Anonymous
Reported by: nsk
Updated by: Dries
Status: patch (code needs review)
Add a print "DB TYPE = $db_type"; to one of the update functions and see
what happens ...
Dries
Previous comments:
------------------------------------------------------------------------
Mon, 29 Aug 2005 06:59:53 +0000 : nsk
Attachment: http://drupal.org/files/issues/nskpatch.tar.bz2 (4.14 KB)
I added MySQLi support to Drupal by adding a database.mysqli.inc file.
After chx's suggestion I also removed $row from db_result from both
mysqli and mysql files.
Users are supposed to connect to Drupal with a mysqli "URL" in their
settings.php. Just adding an "i" in the mysql entry in settings.php
would work.
MySQLi is the new MySQL client in PHP which connects to MySQL 4.1, 5.0
and beyond. MySQLi replaces the older MySQL library which was popular
with MySQL 3.23 and 4.0.
http://gr.php.net/manual/en/ref.mysqli.php
The older MySQL client does not support full functionality with MySQL
4.1, that's why MySQLi is important.
MySQLi works in PHP 4.1.3 or above, and is more popular in PHP 5.
MySQLi means MySQL Improved.
This patch should be included in core.
------------------------------------------------------------------------
Mon, 29 Aug 2005 07:08:07 +0000 : nsk
Attachment: http://drupal.org/files/issues/nskpatch1.diff (7.1 KB)
Here is the diff for mysqli against Drupal CVS HEAD.
------------------------------------------------------------------------
Mon, 29 Aug 2005 07:08:56 +0000 : nsk
Attachment: http://drupal.org/files/issues/nskpatch2.diff (645 bytes)
Here is the diff for mysql against DRUPAL CVS HEAD. The change was
proposed by chx on IRC.
------------------------------------------------------------------------
Mon, 29 Aug 2005 07:13:28 +0000 : nsk
With this message hereby I release all my changes to the files in this
patch (database.mysqli.inc and database.mysql.inc) in GPL 2 (and later
versions), as well as releasing them into the public domain. The
changes I made are show in the diffs. An external URL where users can
find some documentation is:
http://jnanabase.org/index.php/User:NSK/Software/drupal_and_mysqli
------------------------------------------------------------------------
Mon, 29 Aug 2005 13:22:42 +0000 : Thomas Ilsche
big +1, however not tested yet (will do later today)
------------------------------------------------------------------------
Mon, 29 Aug 2005 16:03:33 +0000 : Thox
See duplicate issue: #24264 [1].
[1] http://drupal.org/node/24264
------------------------------------------------------------------------
Mon, 29 Aug 2005 16:16:17 +0000 : m3avrck
Using that file, I get this error right away:
Parse error: syntax error, unexpected ',' in C:\Documents and
Settings\tserbinski\My
Documents\websites\drupal_cvs\drupal\includes\database.mysqli.inc on
line 34
Doing some digging through the documentation, it looks like the
mysqli_real_connect() [2] is being used *improplerly*.
This function call works with no problems:
@mysqli_real_connect($connection, $url['host'], $url['user'],
$url['pass'], (substr($url['path'], 1)), $url['port'], null,
MYSQLI_CLIENT_FOUND_ROWS);
If this is indeed the correct call (it appears to be but can't really
test whether the ports, sockets part is working), then the code circa
line 27 that checks for the port can be deleted since the function call
handles this.
Additionally, the database check on line 51 needs to be moved up since
the database is selected during the mysqli_real_connect() now.
Overall, code needs some work and cleanup.
Peformance wise, noticed a 10ms decrease in page generation times for a
few pages (not thorougly tested performance wise however, just a quick
and dirty observation).
[2] http://us2.php.net/manual/en/function.mysqli-real-connect.php
------------------------------------------------------------------------
Mon, 29 Aug 2005 16:17:34 +0000 : m3avrck
Forgot to mention tested this with PHP5.0.3 and MySQL 4.1.11 on Windows
2003 SP1.
------------------------------------------------------------------------
Mon, 29 Aug 2005 16:20:36 +0000 : m3avrck
Thox, the reported duplicate issue is using an out of date
database.mysql.inc file. I believe Drumm added quite a bit of code for
friendlier errors. Seems those two files need to be merged for proper
MySQLi support. I'll see what I can do later today if I have some time.
------------------------------------------------------------------------
Mon, 29 Aug 2005 16:56:24 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/database.mysqli_2.inc (8.51 KB)
Ok I've taken the latest HEAD version of database.mysql.inc, merged this
with NSK's recommendations, and then merged that with Thox's
database.mysqli.inc (see link a few comments up) whose database code
was based on out of an out of date version of the include file.
Also went through and tweaked and it looks like this patch is ready to
go. Just tested it here and it is running *now* on average, 40ms
*faster* than the regular mysql library.
Patch needs some more testing but I believe it is ready to go.
Optionally, lines 27 can be removed and the mysqli_real_connect() can
be updated with the $url['port']. No way to confirm if this will work
100% but we can get rid of some extraneous code if someone can confirm.
See attached file.
------------------------------------------------------------------------
Mon, 29 Aug 2005 17:01:57 +0000 : m3avrck
Updated status of feature request.
------------------------------------------------------------------------
Mon, 29 Aug 2005 20:38:18 +0000 : Thomas Ilsche
Attachment: http://drupal.org/files/issues/database.mysql.inc.file (8.83 KB)
Tested the file -> however des not work with HEAD due to [1].
However with a slight change it works well.
I have attached the changed one (renamed so the content is not
htmlscrambled)
[1] http://drupal.org/node/22911
------------------------------------------------------------------------
Mon, 29 Aug 2005 20:45:53 +0000 : m3avrck
Thomas this looks like the regular mysql file... nothing changed in
this. Attached wrong file I am thinking?
------------------------------------------------------------------------
Mon, 29 Aug 2005 20:57:03 +0000 : Thomas Ilsche
Attachment: http://drupal.org/files/issues/database.mysqli.inc.file (8.71 KB)
your right. sorry, wrong file.
------------------------------------------------------------------------
Mon, 29 Aug 2005 21:14:17 +0000 : m3avrck
Great catch, working perfect now with latest HEAD. Any more testers, I
think this code is just about ready to submitted to core.
------------------------------------------------------------------------
Tue, 30 Aug 2005 00:22:49 +0000 : kbahey
>From the performance point of view, MySQLi can be slower or faster than
regular MySQL, depending on how ti is used.
According to this benchmark [3], using mysqli_stmt() has a performance
edge over mysql_query(), which is in turn faster than mysqli_query().
NSK or Thomas can take a look and decide if it is worth so to do it.
[3] http://www.johnjawed.com/benchmarks/
------------------------------------------------------------------------
Sat, 03 Sep 2005 19:14:42 +0000 : nsk
Yes I agree mysqli_stmt() is faster, if I have time I'll implement this
later.
------------------------------------------------------------------------
Mon, 05 Sep 2005 15:56:59 +0000 : moshe weitzman
tested - works for me.
------------------------------------------------------------------------
Mon, 05 Sep 2005 15:58:04 +0000 : moshe weitzman
oops. set to proper status
------------------------------------------------------------------------
Mon, 05 Sep 2005 15:59:48 +0000 : m3avrck
Although I do agree this code is working, we should in fact rewrite it
to make user of the more advanced MySQLi techniques, notably
mysqli_stmt() for *true* performance gains. Otherwise this patch
doesn't really offer any incentives.
------------------------------------------------------------------------
Mon, 05 Sep 2005 16:18:04 +0000 : moshe weitzman
one step at a time, folks. prepared statements is a much more complex
undertaking. there are benefits without using that feature. i quote
from http://www.zend.com/php5/articles/php5-mysqli.php:
-----------------------
Beyond gaining access to the new features of MySQL 4.1+, why would
anyone want to switch to using ext/mysqli?
In addition to the functionality mentioned above, ext/mysqli also has
some other serious benefits:
* Greater speed. Enhancements in both the extension and in MySQL
have made most operations faster, with certain operations becoming up
to 40 times faster as compared to ext/mysql.
* Better security. In older versions of the MySQL RDBMS, the
possibility existed for an attacker to extract weak password hashes
from the network and then recreate a user's password. The new
authentication procedure is much more robust and mirrors the
attack-resistant authentication procedure of tools like SSH.
------------------------------------------------------------------------
Tue, 06 Sep 2005 14:19:20 +0000 : m3avrck
very true. i guess this patch then is indeed a good "first" step to get
the initial, working functionality into core. then after that though,
we do need to revisit the issue and update code to make better use of
mysqli_stmt(), which will not only provide better performance, but much
*better* security, and we could probably start to get rid of redudant
code elsewhere in the code for such checks.
------------------------------------------------------------------------
Tue, 06 Sep 2005 18:54:16 +0000 : Dries
I spent 30 minutes reading about mysqli and it looks nice, although it
is not clear whether the old (or current) mysql extension will continue
to evolve, or whether that is going to be deprecated. Anything about
this in the PHP roadmap? Such information would help value the
importance of this patch.
As pointed out in the comments, the current Drupal mysqli backend has
no advantage over the Drupal mysql backend. To me, it is not clear
what potential performance improvements we are looking at or how this
would affect our database API. Also, is any of this ANSI SQL? How is
this going to affect the PostgreSQL backend/support?
I'd like to see more discussion, before adding this to core. At this
point, I don't think the code is ready to be committed simply because
it's not clear how we'll go forward afterwards. While working, Let's
try to outline what is ahead.
------------------------------------------------------------------------
Tue, 06 Sep 2005 19:58:37 +0000 : nsk
The question is not whether mysqli is better than mysql. The issue is to
make Drupal work with mysqli because some people use mysqli instead of
mysql. For example, I could not install Drupal on my home test server
because I only use mysqli here. It's possible to have both mysqli and
mysql installed on the same PHP installation, but Drupal should not
force people to install mysql. mysqli is used for MySQL 4.1 and 5.0,
while mysql is used for MySQL 3.23-4.0.
------------------------------------------------------------------------
Tue, 06 Sep 2005 20:06:19 +0000 : Cvbge
As I understand this is just another database type, like mysql or pgsql.
So you can use $db_url = 'mysqli://blahblah' and it implements normal
db_* functions, am I right?
------------------------------------------------------------------------
Tue, 06 Sep 2005 20:28:14 +0000 : m3avrck
No, MySQLi is the *new* version of MySQL that is incorporated by default
with PHP5. It cannot use the same MySQL functions because a lot has
changed from 4.0 to 4.1 and this is future upgradable with the
forthcoming 5.0 as well. It is my best guess that future versions of
PHP will deprecate the current MySQL functions as the new MySQL 4.1
(and soon 5.0) becomes more and more widespread (both of which work
with MySQLi only).
------------------------------------------------------------------------
Tue, 06 Sep 2005 20:51:55 +0000 : Cvbge
I don't understand, can I use it as $db_url = 'mysqli://blah' or not?
------------------------------------------------------------------------
Tue, 06 Sep 2005 20:56:31 +0000 : m3avrck
Yes you can use that to connect to a MySQL 4.1 database, *however* the
code *will* be broken. regular mysql_ functions won't work, they to be
mysqli_. Sorry for the confusion.
------------------------------------------------------------------------
Wed, 07 Sep 2005 07:56:00 +0000 : Dries
maverick, can you point us to additional information that states that
mysqli is the default mysql extension for PHP5? Anything put forward
by the PHP team on the issue of mysql versus mysqli? Such information
is not critical, but I'd like to get good understanding of the
situation.
------------------------------------------------------------------------
Wed, 07 Sep 2005 07:58:03 +0000 : Dries
Some extra insights from Goba:
http://lists.drupal.org/archives/drupal-docs/2005-08/msg00076.html.
------------------------------------------------------------------------
Wed, 07 Sep 2005 08:08:28 +0000 : Cvbge
Either I don't uderstand what is this about, or you do not understand my
questions.
Why mysqli support can't be made the same way as mysql and pgsql? They
are all php extensions and might be (un)available.
------------------------------------------------------------------------
Wed, 07 Sep 2005 08:27:31 +0000 : Goba
Dries, what we are looking forward with mysqli instead of mysql is:
* You can only connect to a mysql 4.x or 5.x database with mysqli,
unless you use old_password() to set the MySQL user password. The new
password encoding in MySQL 4+.x is more secure, this is why it is
pushed forward.
* People expect mysqli to be faster when dealing with mysql 4.x+, since
it uses a new API (might use speedier stuff not supported by previous
database clients). This is just something people assume, it would be
nice to benchmark.
* The real plus in using Drupal with MySQL 4.1.x+ is that it finally
supports real uft8 text handling [4], ie. sorting, searching, etc. This
is IMHO where Drupal is looking forward. I don't know whether the old
mysql client in PHP can utilize the full potential of the new charset
features, since these might not be client dependent.
So the immediate pluses in MySQL 4.x+ are more secure passwords, real
utf8 support and perceived faster operation.
[4] http://dev.mysql.com/doc/mysql/en/charset-unicode.html
------------------------------------------------------------------------
Wed, 07 Sep 2005 10:46:41 +0000 : nsk
mysqli is not a new database, it's a new database client for MySQL
4.1/5.0 which is installed by default in PHP 5 instead of mysql (the
old client not updated anymore). We should not really discuss whether
mysqli is faster or slower. We should instead focus on the real issue:
Users with MySQL4.1/5.0 and PHP5 urgently need mysqli support in
Drupal, so that they won't have to load the old mysql extension. PgSQL
has nothing to do with mysqli. We talk about PHP extensions here, not
databases. It is possible to get both mysqli and mysql compiled into
PHP 5 at the same time, but it's isn't easy (many people get errors and
can't compile). The patch works by "emulating" mysqli as another
database type so you could connect to it by using a mysqli "URL" in
settings.php. That was just what chx recommended me to do, and some
people in IRC didn't like my initial idea to use "if" checks (which is
how I have implemented mysqli support on my software, and it even
recognises what client the user has installed on-the-fly without any
need for configuration, I have posted an example here [5], it's really
easy). I think we should focus on making Drupal 4.7 compatible with
mysqli, so that developers won't be forced to load the old mysql if
they don't want to, and later we can see how we can better utilise
mysqli's new features and speed it up.
[5] http://jnanabase.org/index.php/MySQLi
------------------------------------------------------------------------
Wed, 07 Sep 2005 10:55:39 +0000 : Dries
Ok. Thanks for the additional information. From the looks we need
mysqli support in Drupal 4.7. So let's focus on the technical aspects
first. Is anyone opposed to introducing a new backend, or do some
people prefer to add checks in the existing mysql backend? We should
be able to commit this shortly.
------------------------------------------------------------------------
Wed, 07 Sep 2005 10:59:26 +0000 : killes at www.drop.org
I'd prefer to have it as another backend. reason: Drupal might be moving
to mysql 4 and we might want a mysql3 backend or Drupal continues to
support mysql 3 and somebody wants a mysql4 backend (not that it would
help too much).
Also, a new backend is much cleaner and you avoid a lot of if-thens in
the course of executing a page.
------------------------------------------------------------------------
Wed, 07 Sep 2005 11:01:34 +0000 : Goba
Separate backend +1. It is much easier to maintain as well as it is much
more performant (ie there is no need to load and continually skip unused
code).
------------------------------------------------------------------------
Wed, 07 Sep 2005 11:05:55 +0000 : nsk
if you use "if" checks, you can make it work without intervention from
the administrator: Drupal could automatically check whether mysqli is
available and use it on-the-fly, and if it isn't loaded it would
fallback to mysql. With a separate mysqli included file, the
administrator would have to specifically require their Drupal
installation to use mysqli. This could cause some confusion for people
who upgrade from php4 to php5 and their compilation only provides
mysqli in php5, they may not even know about mysqli and wonder why
Drupal doesn't work after they upgraded to php5, until someone informs
them that they have either to recompile with mysql or change their
settings.php in Drupal. Good documentation could help with this, of
course.
------------------------------------------------------------------------
Wed, 07 Sep 2005 11:07:00 +0000 : chx
It's simply not a question to have a separate backend or not. It's
absolutely non-drupal to have two backends in one .inc.
------------------------------------------------------------------------
Wed, 07 Sep 2005 11:22:59 +0000 : jadwigo
"With a separate mysqli included file, the administrator would have to
specifically require their Drupal installation to use mysqli. This
could cause some confusion for people who upgrade from php4 to php5 and
their compilation only provides mysqli in php5, they may not even know
about mysqli and wonder why Drupal doesn't work after they upgraded to
php5, until someone informs them that they have either to recompile
with mysql or change their settings.php in Drupal. Good documentation
could help with this, of course.
"
So we only need to inform them...
IMO it's important enough that if you're confused as a user you should
be, because then you start wondering what has changed. If you maintain
a site and upgrade essential tools like mysql and php it's only normal
to know what is going on.
For users who don't or can't maintain their own servers we should
provide some information on the system requirements page [6] and in the
install.txt
[6] http://drupal.org/node/270
------------------------------------------------------------------------
Wed, 07 Sep 2005 11:31:58 +0000 : stefan nagtegaal
Well, we can include an 'compatibity_test.php' script inside the main
drupal distro, which checks if all things which are required are
present? (PHP-version, compiled with MySQL, etc)
And after that does the same test for optional requirements like GD,
Mod_Rewrite, mbstring, etc...
How about that?
Stefan
------------------------------------------------------------------------
Wed, 07 Sep 2005 13:24:48 +0000 : nsk
The idea to have a dependencies check script is great.
------------------------------------------------------------------------
Wed, 07 Sep 2005 13:47:24 +0000 : m3avrck
Now correct me if I'm wrong, but can't we just stick to how the original
patch is working, e.g, create a new file database.mysqli.inc and then
just change the $db_url string in settings.php so it knows to pickup up
mysqli instead of mysql? This seems to be working great.
As of a side note, on Windows, it is *much* easier to run the MySQL and
MySQLi extensions in parallel with MySQL 4.1 ... that is how I'm
currently running my dev machine and I can switch back and forth
between regular database.mysql.inc and database.mysqli.inc just by
changing the $db_url in settings.php.
It would be my best recommendation to keep this approach for a few
Drupal release cycles (say up to 5.1/5.2) and then deprecate the
database.mysql.inc file and then eventually remove it as PHP5.1
(forthcoming) and MySQL 4.1 become dominant default installs.
------------------------------------------------------------------------
Wed, 07 Sep 2005 13:49:31 +0000 : Goba
compatibity_test.php or something along the lines is going to be part of
the install system anyway, as far as I know.
------------------------------------------------------------------------
Wed, 07 Sep 2005 14:10:26 +0000 : moshe weitzman
I prefer separate backend too. I think the auto-switching between mysql
and mysqli is great and easy to implement. Just create an if() block
right before we include database.x.inc. That happens in db_set_active()
[line 112 of includes/database.inc]. The if should look like:
<?php
// automatically use mysqli if loaded. it is more secure: <a href="http://www.php.net/mysqli">http://www.php.net/mysqli</a>
if ($db_type == 'mysql' && extension_loaded("mysqli")) {
$db_type == 'mysqli';
}
?>
Sorry, I can't reroll the patch and re-test at the moment.
------------------------------------------------------------------------
Wed, 07 Sep 2005 14:37:57 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/drupal_4.patch (1.94 KB)
Ok here is a patch that fixes some references to mysql and adds mysqli
as well.
------------------------------------------------------------------------
Wed, 07 Sep 2005 14:40:40 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/database.mysqli_3.inc (9 KB)
This is the patch from comment #13 that has been tested and working
great on WinXP with PHP 5.0.4 and MySQL 4.1.11. Sorry couldn't get both
of these patches to roll into one.
Also, I see no *reason* to include code to auto detect MySQLi over
MySQL unless we add support to do this for PostgreSQL as wel. Otherwise
that isn't consistent. It is clear in the settings.php that either
mysql, mysqli, or pgsql can be used to setup a database. Easy enough.
------------------------------------------------------------------------
Wed, 07 Sep 2005 14:45:35 +0000 : m3avrck
Correction, that last file should be placed in includes/ as
database.mysqli.inc. Then just edit $db_url in settings.php to make use
of it and switch from MySQL to MySQLi support.
------------------------------------------------------------------------
Wed, 07 Sep 2005 15:27:51 +0000 : m3avrck
Further testing does show one *big* break in Drupal, updates.inc.
Updates.inc is testing to see if ($GLOBALS['db_type'] == 'mysql'). If
you set the the $db_url to mysqli, *no* updates work. The code will be
the same for mysql and mysqli, so I think $GLOBALS['db_type'] needs to
be ammended so that it accounts for both mysql and mysqli connections
(since they are essentially the same in respect to SQL code).
------------------------------------------------------------------------
Wed, 07 Sep 2005 15:51:15 +0000 : chx
if (substr($GLOBALS['db_type'], 0, 5) == 'mysql') big problem fixed.
------------------------------------------------------------------------
Wed, 07 Sep 2005 15:54:58 +0000 : Goba
+1 on chx's big problem fix :)
------------------------------------------------------------------------
Wed, 07 Sep 2005 16:24:55 +0000 : m3avrck
+1 to that fix as well :) Just need to make note of that in any furture
database updates after MySQLi support is added.
------------------------------------------------------------------------
Wed, 07 Sep 2005 20:11:17 +0000 : Dries
Please add a helper function for that to the update script. Something
like using_mysql() or so. I'll commit this patch as soon update script
is taken care of. Great work.
------------------------------------------------------------------------
Wed, 07 Sep 2005 21:41:38 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/drupal_7.patch (14.46 KB)
Ok here is an updated patch that patches updates.inc as well. I put a
check at the top that assigns $GLOBALS['db_type'] to it's only local
var $db_type and then uses chx's above recommendation to make the fix.
Works great!
Use this patch in conjuctions with the above database.mysqli.inc file!
------------------------------------------------------------------------
Wed, 07 Sep 2005 21:48:37 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/drupal_8.patch (14.45 KB)
Fixed a typo.
------------------------------------------------------------------------
Thu, 08 Sep 2005 20:42:33 +0000 : Dries
I don't see how you can use global variables like that; $db_type will be
undefined in the update functions.
db_connect() should not return false -- the existing db_connect()
functions appear to do their own error handling/reporting. Maybe the
PHPdoc needs to be updated.
(On a related note, there appears to be quite a bit of duplicated code
in the error handling. A couple weeks ago, when the Drupal database
server was down, these database error handling kicked in and we were
sharing quite a bit of information -- like the hostname/IP of the
database server -- with the rest of the world. Not sure that is a good
idea.)
------------------------------------------------------------------------
Thu, 08 Sep 2005 21:34:17 +0000 : m3avrck
Don't see the problem with $db_type. Only reason I did that in
updates.inc is to reduce redunancy. Other option would be to do:
if (($GLOBALS['db_type'] == 'mysql') || ($GLOBALS['db_type'] ==
'mysqli')){
That is just so the correct SQL is used. Otherwise, the rest of the
time $db_type needs to be *mysqli* and not just *mysql*.
Make sense? Comments on which approach is better?
More information about the drupal-devel
mailing list