This is an RFC of sorts. :-) At Drupalcon, I was shopping around the idea of a PDO[1] backend for Drupal. For brevity, I'll just reference the blog entry I made on why I think PDO is a good idea[2]. The goal is that PHP 5 users get a nice speed bump and we get a start on eventually shifting Drupal's database layer from the 1997 APIs to the more robust 21st century PHP standards. Most people I talked to liked the idea, so after getting a nod from Dries I've been working on it since the Hackfest and made decent progress. However, I've run into a few snags I want to get a 2nd opinion on (or 3rd, or 4th, or whatever). 1) It turns out that PDO from PECL running under PHP 5.1.6, at least, has issues. Specifically, segfault issues on otherwise perfectly sane queries. This is apparently a known issue with 5.1.6, and one of the reasons 5.2 exists. :-) At present, I have no solution for it other than saying "well, the PDO support is only if you're running 5.2, otherwise use the existing MySQL/PostgreSQL drivers, deal". Does anyone have a problem with that, and if so, an alternate solution? 2) Like any wrapper, PDO, while it offers some really nice features with a common API (like C-level prepared statements, which are what I'm mainly after), has some "lowest common denominator" issues. The main one I've run into so far is that there is no reliable equivalent of mysql_num_rows() for SELECT statements, only for data-changing statements[3]. In testing it doesn't look like the MySQL PDO driver returns anything useful for rowCount() on SELECT. That gives us 3 options. A) Stop using db_num_rows() on result sets. (It's a database-specific feature in the first place.) In places where we use it, use a separate count(*) query instead. That's the more database-agnostic method, and is what the PHP manual recommends[3]. If we go this route, it will involve removing the db_num_rows() function from the existing mysql and postgres drivers and refactoring core queries accordingly. B) Instead, have the PDO wrapper do a full ->fetchAll() on the result set. That gives an array of array or object records (specified in the fetchAll()), which can then be simply sizeof()ed in the PDO/db_num_rows() implementation. The downside here is that you need to specify in the fetchAll() whether you want objects or arrays. To not horribly break any existing APIs (I'm trying to minimize the footprint for now; we can break everything later), we'd need to therefore fetchAll() as an array in the PDO driver and then cast back to an object in db_fetch_object() (or vice versa). That feels quite nasty to me, honestly, and is probably a not-unnoticeable performance hit. C) Drop the PDO idea. It should come as no surprise that this is my least favorite option. :-( As my goal is that in some mythical future when we can require PHP 5 PDO becomes our main database backend (and we can leverage it even more for a cleaner API, faster/more stable prepared statements, etc.), I would favor slimming down and generalizing our database calls now (option A) so that we're better prepared in the future. (It would also make adding support for other database engines like Oracle or MS SQL easier.) I am, however, open to input and suggestions. I am hoping I can get this into Drupal 6, but time will tell if I can get it ready in time. [1] http://www.php.net/pdo [2] http://www.garfieldtech.com/blog/drupalcon-php5 [3] http://us.php.net/manual/en/function.PDOStatement-rowCount.php -- Larry Garfield AIM: LOLG42 larry@garfieldtech.com ICQ: 6817012 "If nature has made any one thing less susceptible than all others of exclusive property, it is the action of the thinking power called an idea, which an individual may exclusively possess as long as he keeps it to himself; but the moment it is divulged, it forces itself into the possession of every one, and the receiver cannot dispossess himself of it." -- Thomas Jefferson
As long as we're discussing major database overhauls, my main concern is moving to proper use of referential integrity and transactions. Locking entire tables has a very detrimental effect on high-traffic site performance. It also messes with existing transactions. Of course, this would require dropping MyISAM support. Transactions need not be annoying. I very recently released a module to make transactions much easier: http://drupal.org/project/pressflow_transaction Yes, it requires PHP 5 to perform its magic, but so does PDO. Larry Garfield wrote:
This is an RFC of sorts. :-)
At Drupalcon, I was shopping around the idea of a PDO[1] backend for Drupal. For brevity, I'll just reference the blog entry I made on why I think PDO is a good idea[2]. The goal is that PHP 5 users get a nice speed bump and we get a start on eventually shifting Drupal's database layer from the 1997 APIs to the more robust 21st century PHP standards. Most people I talked to liked the idea, so after getting a nod from Dries I've been working on it since the Hackfest and made decent progress. However, I've run into a few snags I want to get a 2nd opinion on (or 3rd, or 4th, or whatever).
1) It turns out that PDO from PECL running under PHP 5.1.6, at least, has issues. Specifically, segfault issues on otherwise perfectly sane queries. This is apparently a known issue with 5.1.6, and one of the reasons 5.2 exists. :-) At present, I have no solution for it other than saying "well, the PDO support is only if you're running 5.2, otherwise use the existing MySQL/PostgreSQL drivers, deal". Does anyone have a problem with that, and if so, an alternate solution?
2) Like any wrapper, PDO, while it offers some really nice features with a common API (like C-level prepared statements, which are what I'm mainly after), has some "lowest common denominator" issues. The main one I've run into so far is that there is no reliable equivalent of mysql_num_rows() for SELECT statements, only for data-changing statements[3]. In testing it doesn't look like the MySQL PDO driver returns anything useful for rowCount() on SELECT. That gives us 3 options.
A) Stop using db_num_rows() on result sets. (It's a database-specific feature in the first place.) In places where we use it, use a separate count(*) query instead. That's the more database-agnostic method, and is what the PHP manual recommends[3]. If we go this route, it will involve removing the db_num_rows() function from the existing mysql and postgres drivers and refactoring core queries accordingly.
B) Instead, have the PDO wrapper do a full ->fetchAll() on the result set. That gives an array of array or object records (specified in the fetchAll()), which can then be simply sizeof()ed in the PDO/db_num_rows() implementation. The downside here is that you need to specify in the fetchAll() whether you want objects or arrays. To not horribly break any existing APIs (I'm trying to minimize the footprint for now; we can break everything later), we'd need to therefore fetchAll() as an array in the PDO driver and then cast back to an object in db_fetch_object() (or vice versa). That feels quite nasty to me, honestly, and is probably a not-unnoticeable performance hit.
C) Drop the PDO idea. It should come as no surprise that this is my least favorite option. :-(
As my goal is that in some mythical future when we can require PHP 5 PDO becomes our main database backend (and we can leverage it even more for a cleaner API, faster/more stable prepared statements, etc.), I would favor slimming down and generalizing our database calls now (option A) so that we're better prepared in the future. (It would also make adding support for other database engines like Oracle or MS SQL easier.) I am, however, open to input and suggestions.
I am hoping I can get this into Drupal 6, but time will tell if I can get it ready in time.
[1] http://www.php.net/pdo [2] http://www.garfieldtech.com/blog/drupalcon-php5 [3] http://us.php.net/manual/en/function.PDOStatement-rowCount.php
Option A. Go for it, I say. Maybe we need to form a database team? In addition to PDO support, there's the referential integrity and transactions which David described. Additionally, just general database optimization and configuration recommendations could be described. Killes did a lot of work on making drupal.org work better. While there's room in the PHP code for performance improvements, probably some of the biggest and easiest to obtain are in the database. It'd be nice to have a couple of database layers to choose from, so as to support the widest variety of installations. That is, for example, a layer which supports PHP 4 and MyISAM, for small sites on cheap hosts, to a layer which supports PDO and transactional engines only (no MyISAM) for high-performance sites on high-end hardware.
David Strauss wrote:
As long as we're discussing major database overhauls, my main concern is
please use own thread. your goal has merit, but you haven't presented it well. let's use good list manners. we have a 1000+ person list now. we do shockingly little "list etiquette" on this list and on issue reviews and i think it hurts our signal vs. noise. i'm going to do a bit more policing in those places, unless someone else volunteers to assume that role. please contact me privately if interested. thanks much, moshe
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Larry Garfield schrieb:
This is an RFC of sorts. :-)
At Drupalcon, I was shopping around the idea of a PDO[1] backend for Drupal. For brevity, I'll just reference the blog entry I made on why I think PDO is a good idea[2]. The goal is that PHP 5 users get a nice speed bump and we get a start on eventually shifting Drupal's database layer from the 1997 APIs to the more robust 21st century PHP standards. Most people I talked to liked
Andrew Kirkham has also been working on PDOs.
the idea, so after getting a nod from Dries I've been working on it since the Hackfest and made decent progress. However, I've run into a few snags I want to get a 2nd opinion on (or 3rd, or 4th, or whatever).
1) It turns out that PDO from PECL running under PHP 5.1.6, at least, has issues. Specifically, segfault issues on otherwise perfectly sane queries. This is apparently a known issue with 5.1.6, and one of the reasons 5.2 exists. :-) At present, I have no solution for it other than saying "well, the PDO support is only if you're running 5.2, otherwise use the existing MySQL/PostgreSQL drivers, deal". Does anyone have a problem with that, and if so, an alternate solution?
No problem with that. Fits in line with the idea to get people to use higher versions of PHP.
2) Like any wrapper, PDO, while it offers some really nice features with a common API (like C-level prepared statements, which are what I'm mainly after), has some "lowest common denominator" issues. The main one I've run into so far is that there is no reliable equivalent of mysql_num_rows() for SELECT statements, only for data-changing statements[3]. In testing it doesn't look like the MySQL PDO driver returns anything useful for rowCount() on SELECT. That gives us 3 options.
A) Stop using db_num_rows() on result sets. (It's a database-specific feature in the first place.) In places where we use it, use a separate count(*) query instead. That's the more database-agnostic method, and is what the PHP manual recommends[3]. If we go this route, it will involve removing the db_num_rows() function from the existing mysql and postgres drivers and refactoring core queries accordingly.
Sounds ok to me. Cheers, Gerhard -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFGE3Lqfg6TFvELooQRAmCJAJ9+7D5+7JpAUcUw45MP6wc9xXZYugCfTEeW 21xgTFKTIt1d5W9GbqNIcSQ= =9BZ1 -----END PGP SIGNATURE-----
On 4/4/07, Larry Garfield <larry@garfieldtech.com> wrote:
This is an RFC of sorts. :-)
1) It turns out that PDO from PECL running under PHP 5.1.6, at least, has issues. Specifically, segfault issues on otherwise perfectly sane queries. This is apparently a known issue with 5.1.6, and one of the reasons 5.2 exists. :-) At present, I have no solution for it other than saying "well, the PDO support is only if you're running 5.2, otherwise use the existing MySQL/PostgreSQL drivers, deal". Does anyone have a problem with that, and if so, an alternate solution?
Which distro? I use Ubuntu 6.10 and if you install PHP 5, 5.1.6 is the default you get. This makes it harder to use 5.2 (custom compile, ...etc.) If you have working code, I can verify whether I have the same problem or not on my test server. 2) Like any wrapper, PDO, while it offers some really nice features with a
common API (like C-level prepared statements, which are what I'm mainly after), has some "lowest common denominator" issues. The main one I've run into so far is that there is no reliable equivalent of mysql_num_rows() for SELECT statements, only for data-changing statements[3]. In testing it doesn't look like the MySQL PDO driver returns anything useful for rowCount() on SELECT. That gives us 3 options.
In core HEAD, there are 43 occurrences in 25 files. Haven't checked contrib. includes/common.inc: includes/database.mysqli.inc: includes/database.mysql.inc: includes/database.pgsql.inc: includes/locale.inc: includes/session.inc: modules/aggregator/aggregator.module: modules/block/block.module: modules/blog/blog.module: modules/book/book.module: modules/comment/comment.module: modules/drupal/drupal.module: modules/forum/forum.module: modules/node/content_types.inc: modules/node/node.module: modules/path/path.module: modules/ping/ping.module: modules/statistics/statistics.module: modules/system/system.install: modules/system/system.module: modules/taxonomy/taxonomy.module: modules/user/user.module: A) Stop using db_num_rows() on result sets. (It's a database-specific
feature in the first place.) In places where we use it, use a separate count(*) query instead. That's the more database-agnostic method, and is what the PHP manual recommends[3]. If we go this route, it will involve removing the db_num_rows() function from the existing mysql and postgres drivers and refactoring core queries accordingly.
My concern here is that all db_num_rows() cases become two queries for people using MySQL (the vast majority) as well as PostgreSQL (it has pg_num_rows()). If these are index only queries, then it may not be a big deal, but anything more (which we have a lot of), would incur a significant performance penalty. It also affects large sites using InnoDB which is notorious for very slow SELECT COUNT(*). See a benchmark here: http://2bits.com/articles/mysql-innodb-performance-gains-as-well-as-some-pit... Not sure what else can be done here. Is there a way to sneek in a pass thru function (mysql_num_rows() in this case) like ODBC for example allows? B) Instead, have the PDO wrapper do a full ->fetchAll() on the result set.
That gives an array of array or object records (specified in the fetchAll()), which can then be simply sizeof()ed in the PDO/db_num_rows() implementation. The downside here is that you need to specify in the fetchAll() whether you want objects or arrays. To not horribly break any existing APIs (I'm trying to minimize the footprint for now; we can break everything later), we'd need to therefore fetchAll() as an array in the PDO driver and then cast back to an object in db_fetch_object() (or vice versa). That feels quite nasty to me, honestly, and is probably a not-unnoticeable performance hit.
I don't think we can do that safely at all. The result set may exhaust available memory, or tax other resources if it is rather large. C) Drop the PDO idea. It should come as no surprise that this is my least
favorite option. :-(
Option D) : File a feature request against PDO to include a numRows() method, but that is not a quick solution. I am hoping I can get this into Drupal 6, but time will tell if I can get it
ready in time.
Ubuntu Feisty (due later in April) will have PHP 5.2.1. http://packages.ubuntu.com/feisty/web/php5 But, what about other distros? If they are on PHP4 or 5.1, then this will be a show stopper. -- 2bits.com http://2bits.com Drupal development, customization and consulting.
A) Stop using db_num_rows() on result sets.
Yes, but most of the time we do not need the exact count, we just want to see whether there is a row or not. That can be replace with a query limited to one row: db_result(db_query_range(' ', 0, 1)) and check for === FALSE . There is only one case in core where we use the exact count, and that's the online users block.
Ubuntu Feisty (due later in April) will have PHP 5.2.1. http://packages.ubuntu.com/feisty/web/php5
Good. Drupal 6 will be out in the summer. Debian Etch has 5.2.0. However, RHEL 5 has only 5.1.6 paackaged. I would say, just go ahead and let the world play catch up. For example you could issue some warning on the requirements page that if PDO is used then "it's strongly recommended" to run 5.2 Rock on! NK
On 04 Apr 2007, at 08:05, Larry Garfield wrote:
A) Stop using db_num_rows() on result sets. (It's a database- specific feature in the first place.) In places where we use it, use a separate count(*) query instead. That's the more database-agnostic method, and is what the PHP manual recommends[3]. If we go this route, it will involve removing the db_num_rows() function from the existing mysql and postgres drivers and refactoring core queries accordingly.
I'd go for A. :) If we don't need the exact count, we might be able to replace db_num_rows() with something like db_table_is_empty(). Let us know if you run into issues that need further attention/ discussion. -- Dries Buytaert :: http://www.buytaert.net/
On Wed, 4 Apr 2007 21:33:49 +0200, Dries Buytaert <dries.buytaert@gmail.com> wrote:
On 04 Apr 2007, at 08:05, Larry Garfield wrote:
A) Stop using db_num_rows() on result sets. (It's a database- specific feature in the first place.) In places where we use it, use a separate count(*) query instead. That's the more database-agnostic method, and is what the PHP manual recommends[3]. If we go this route, it will involve removing the db_num_rows() function from the existing mysql and postgres drivers and refactoring core queries accordingly.
I'd go for A. :)
If we don't need the exact count, we might be able to replace db_num_rows() with something like db_table_is_empty().
Seems most people are OK with A, so A it is. :-) I'll keep an eye on the queries as I'm fixing them to try and avoid double-queries when possible. I don't know if db_result_set_is_empty() is a good solution or not. I guess I'll find out.
Let us know if you run into issues that need further attention/ discussion.
One other FYI that is probably not going to break TOO much, I hope... Since this affects db_query(), this patch will also involve moving db_query into each database.*.inc file rather than database.inc. For the moment I'm just looking at copying and pasting them into the mysql and pgsql implementations, which so far has caused no issues. We may, later, want to then refactor the relationship between db_query and _db_query(), but for the moment that is not something I'm worrying about. (I suspect the pdo db_query() won't even have a _db_query(), as I'll just merge them. TBD.) --Larry Garfield
Database abstraction is generally a good thing - you get to write one set of data access layer code and once you follow the best practices, you can let the vendor focus on optimizing the driver for their database engine. So in theory with PDO you could call $rowset->rowCount() and MySQL could implement this as a call to a C-Level native API and not even run any SQL code. You get support for more platforms, optimization by the people who know the most about it, and much less data access code. You also get a the ability to implement transactions transparently, and you get closer to being able to run transactions which enlist other apps. Goodness +4 However there are hazards. ADO worked really well on Windows because it only ran on Windows, and there was one binary standard for Windows which was COM and one standard for data access which was OLE DB/ODBC and every database vendor had to play by those rules. I don't know how mature PDO is wrt being able to talk to different databases on different architectures. It would really suck if you rewrote your data-access code and then had to thrown in kludges to work around specific issues with specific versions of different databases which is what you were trying to get away from in the first place. Also all modules would have to rewritten to use the new data access layer - the performance benefits of DOs are inside methods like rowCount(), you don't gain anything by passing in a raw SQL statement to execute - It's in methods like setFetchmode, prepare(), execute() where the advanyages are to be found. All db_query()s would have to be exorcised, trying to run with a mix of DO and raw SQL code would be a maintenance nightmare Dropping support for MyISAM means that Drupal newbies may need to take extra steps when creating their database - right now MySQL and MyISAM are the lowest common denominator - just about anybody can run it. On 04/04/07, Larry Garfield <larry@garfieldtech.com> wrote:
On Wed, 4 Apr 2007 21:33:49 +0200, Dries Buytaert <dries.buytaert@gmail.com> wrote:
On 04 Apr 2007, at 08:05, Larry Garfield wrote:
A) Stop using db_num_rows() on result sets. (It's a database- specific feature in the first place.) In places where we use it, use a separate count(*) query instead. That's the more database-agnostic method, and is what the PHP manual recommends[3]. If we go this route, it will involve removing the db_num_rows() function from the existing mysql and postgres drivers and refactoring core queries accordingly.
I'd go for A. :)
If we don't need the exact count, we might be able to replace db_num_rows() with something like db_table_is_empty().
Seems most people are OK with A, so A it is. :-) I'll keep an eye on the queries as I'm fixing them to try and avoid double-queries when possible. I don't know if db_result_set_is_empty() is a good solution or not. I guess I'll find out.
Let us know if you run into issues that need further attention/ discussion.
One other FYI that is probably not going to break TOO much, I hope... Since this affects db_query(), this patch will also involve moving db_query into each database.*.inc file rather than database.inc. For the moment I'm just looking at copying and pasting them into the mysql and pgsql implementations, which so far has caused no issues. We may, later, want to then refactor the relationship between db_query and _db_query(), but for the moment that is not something I'm worrying about. (I suspect the pdo db_query() won't even have a _db_query(), as I'll just merge them. TBD.)
--Larry Garfield
-- Muppet Show > Monty Python
I think you're misunderstanding what PDO is. PDO is not an ActiveRecord implementation. It's a unification of the disparate mysql_*, pgsql_*, odbc_*, etc. functions in PHP, which has always been ugly. It provides a prepared statement emulation layer for databases that don't support them directly as well as a transaction implementation that still works transparently (albeit with no effect) on systems that don't natively support transactions. It does not provide any sort of abstraction for SQL statements themselves. http://www.php.net/pdo At this point I'm not looking to change the entire database API. At least not until Drupal 7. :-) For now I'm just trying to provide an alternate backend in addition to the existing mysql and pgsql backends because PDO's prepared statement support is way better than ours could ever be, since it's all happening down in C code. That should (I believe) give a nice performance boost for those running PHP 5.2 where PDO is included natively (and give a nice "hey, stop using PHP 4 would you!" nudge without removing support for PHP 4 from Drupal itself). I'm trying to keep the API changes as minimal as possible for now. ActiveRecord and object relational mapping (what you're talking about here) in PHP 4 is ugly, nasty, and evil. In PHP 5, it *can*, if done right, be very thin and very powerful and very cool. (I've been doing some research on that separately from Drupal.) The question of using an ActiveRecord/ORM in Drupal is really something that is not worth thinking about until we know we can demand PHP 5, which I figure is at least another year away. Database table type (MyISAM vs. InnoDB) is also completely unrelated to what I'm doing. I will, however, agree with you that Muppet Show > Monty Python. :-) --Larry Garfield On Fri, 6 Apr 2007 09:54:18 -0400, "Allister Beharry" <allister.beharry@gmail.com> wrote:
Database abstraction is generally a good thing - you get to write one set of data access layer code and once you follow the best practices, you can let the vendor focus on optimizing the driver for their database engine. So in theory with PDO you could call $rowset->rowCount() and MySQL could implement this as a call to a C-Level native API and not even run any SQL code. You get support for more platforms, optimization by the people who know the most about it, and much less data access code. You also get a the ability to implement transactions transparently, and you get closer to being able to run transactions which enlist other apps. Goodness +4
However there are hazards. ADO worked really well on Windows because it only ran on Windows, and there was one binary standard for Windows which was COM and one standard for data access which was OLE DB/ODBC and every database vendor had to play by those rules. I don't know how mature PDO is wrt being able to talk to different databases on different architectures. It would really suck if you rewrote your data-access code and then had to thrown in kludges to work around specific issues with specific versions of different databases which is what you were trying to get away from in the first place.
Also all modules would have to rewritten to use the new data access layer - the performance benefits of DOs are inside methods like rowCount(), you don't gain anything by passing in a raw SQL statement to execute - It's in methods like setFetchmode, prepare(), execute() where the advanyages are to be found. All db_query()s would have to be exorcised, trying to run with a mix of DO and raw SQL code would be a maintenance nightmare
Dropping support for MyISAM means that Drupal newbies may need to take extra steps when creating their database - right now MySQL and MyISAM are the lowest common denominator - just about anybody can run it.
On 04/04/07, Larry Garfield <larry@garfieldtech.com> wrote:
On Wed, 4 Apr 2007 21:33:49 +0200, Dries Buytaert
<dries.buytaert@gmail.com> wrote:
On 04 Apr 2007, at 08:05, Larry Garfield wrote:
A) Stop using db_num_rows() on result sets. (It's a database- specific feature in the first place.) In places where we use it, use a separate count(*) query instead. That's the more database-agnostic method, and is what the PHP manual recommends[3]. If we go this route, it will involve removing the db_num_rows() function from the existing mysql and postgres drivers and refactoring core queries accordingly.
I'd go for A. :)
If we don't need the exact count, we might be able to replace db_num_rows() with something like db_table_is_empty().
Seems most people are OK with A, so A it is. :-) I'll keep an eye on the queries as I'm fixing them to try and avoid double-queries when possible. I don't know if db_result_set_is_empty() is a good solution or not. I guess I'll find out.
Let us know if you run into issues that need further attention/ discussion.
One other FYI that is probably not going to break TOO much, I hope... Since this affects db_query(), this patch will also involve moving db_query into each database.*.inc file rather than database.inc. For the moment I'm just looking at copying and pasting them into the mysql and pgsql implementations, which so far has caused no issues. We may, later, want to then refactor the relationship between db_query and _db_query(), but for the moment that is not something I'm worrying about. (I suspect the pdo db_query() won't even have a _db_query(), as I'll just merge them. TBD.)
--Larry Garfield
--
Muppet Show > Monty Python
On 06/04/07, Larry Garfield <larry@garfieldtech.com> wrote:
I think you're misunderstanding what PDO is. PDO is not an ActiveRecord implementation. It's a unification of the disparate mysql_*, pgsql_*, odbc_*, etc. functions in PHP, which has always been ugly. It provides a prepared statement emulation layer for databases that don't support them directly as well as a transaction implementation that still works transparently (albeit with no effect) on systems that don't natively support transactions. It does not provide any sort of abstraction for SQL statements themselves.
"PDO provides a data-access abstraction layer, which means that, regardless of which database you're using, you use the same functions to issue queries and fetch data. PDO does not provide a database abstraction; it doesn't rewrite SQL or emulate missing features. You should use a full-blown abstraction layer if you need that facility." ohh...ok *face is red*
At this point I'm not looking to change the entire database API. At least not until Drupal 7. :-) For now I'm just trying to provide an alternate backend in addition to the existing mysql and pgsql backends because PDO's prepared statement support is way better than ours could ever be, since it's all happening down in C code. That should (I believe) give a nice performance boost for those running PHP 5.2 where PDO is included natively (and give a nice "hey, stop using PHP 4 would you!" nudge without removing support for PHP 4 from Drupal itself). I'm trying to keep the API changes as minimal as possible for now.
Ok, sounds like a plan and it will reap some of the benefits I mentioned. Hmm..I think I just volunteered to help.
ActiveRecord and object relational mapping (what you're talking about here) in PHP 4 is ugly, nasty, and evil. In PHP 5, it *can*, if done right, be very thin and very powerful and very cool. (I've been doing some research on that separately from Drupal.) The question of using an ActiveRecord/ORM in Drupal is really something that is not worth thinking about until we know we can demand PHP 5, which I figure is at least another year away.
Actually I wasn't specifically talking about the ActiveRecord pattern or ORM, I meant any type of unified data access layer - Windows ADO came to mind because the methods in PDO reminded me of it. But my point is moot because PDO isn't trying to do database abstraction like ADO.
Database table type (MyISAM vs. InnoDB) is also completely unrelated to what I'm doing.
I had a brain fart there, what I meant to write was PHP 4 and MyISAM are the lowest common denominator; and can run practically everywhere; whereas if Drupal started to require more advanced database functionality to support PDO and transactions and what not, it would raise the bar out of reach of many people. Ok, no more posting to MLs before breakfast.
I will, however, agree with you that Muppet Show > Monty Python. :-)
The work continues...
On Fri, 6 Apr 2007 09:54:18 -0400, "Allister Beharry" <allister.beharry@gmail.com> wrote:
Database abstraction is generally a good thing - you get to write one set of data access layer code and once you follow the best practices, you can let the vendor focus on optimizing the driver for their database engine. So in theory with PDO you could call $rowset->rowCount() and MySQL could implement this as a call to a C-Level native API and not even run any SQL code. You get support for more platforms, optimization by the people who know the most about it, and much less data access code. You also get a the ability to implement transactions transparently, and you get closer to being able to run transactions which enlist other apps. Goodness +4
However there are hazards. ADO worked really well on Windows because it only ran on Windows, and there was one binary standard for Windows which was COM and one standard for data access which was OLE DB/ODBC and every database vendor had to play by those rules. I don't know how mature PDO is wrt being able to talk to different databases on different architectures. It would really suck if you rewrote your data-access code and then had to thrown in kludges to work around specific issues with specific versions of different databases which is what you were trying to get away from in the first place.
Also all modules would have to rewritten to use the new data access layer - the performance benefits of DOs are inside methods like rowCount(), you don't gain anything by passing in a raw SQL statement to execute - It's in methods like setFetchmode, prepare(), execute() where the advanyages are to be found. All db_query()s would have to be exorcised, trying to run with a mix of DO and raw SQL code would be a maintenance nightmare
Dropping support for MyISAM means that Drupal newbies may need to take extra steps when creating their database - right now MySQL and MyISAM are the lowest common denominator - just about anybody can run it.
On 04/04/07, Larry Garfield <larry@garfieldtech.com> wrote:
On Wed, 4 Apr 2007 21:33:49 +0200, Dries Buytaert
<dries.buytaert@gmail.com> wrote:
On 04 Apr 2007, at 08:05, Larry Garfield wrote:
A) Stop using db_num_rows() on result sets. (It's a database- specific feature in the first place.) In places where we use it, use a separate count(*) query instead. That's the more database-agnostic method, and is what the PHP manual recommends[3]. If we go this route, it will involve removing the db_num_rows() function from the existing mysql and postgres drivers and refactoring core queries accordingly.
I'd go for A. :)
If we don't need the exact count, we might be able to replace db_num_rows() with something like db_table_is_empty().
Seems most people are OK with A, so A it is. :-) I'll keep an eye on the queries as I'm fixing them to try and avoid double-queries when possible. I don't know if db_result_set_is_empty() is a good solution or not. I guess I'll find out.
Let us know if you run into issues that need further attention/ discussion.
One other FYI that is probably not going to break TOO much, I hope... Since this affects db_query(), this patch will also involve moving db_query into each database.*.inc file rather than database.inc. For the moment I'm just looking at copying and pasting them into the mysql and pgsql implementations, which so far has caused no issues. We may, later, want to then refactor the relationship between db_query and _db_query(), but for the moment that is not something I'm worrying about. (I suspect the pdo db_query() won't even have a _db_query(), as I'll just merge them. TBD.)
--Larry Garfield
--
Muppet Show > Monty Python
participants (9)
-
Allister Beharry -
Chris Johnson -
David Strauss -
Dries Buytaert -
Gerhard Killesreiter -
Karoly Negyesi -
Khalid Baheyeldin -
Larry Garfield -
Moshe Weitzman