Thread: PostgreSQL driver for Joomla review
If anyone has a moment, could they review the PostgreSQL driver I wrote for Joomla's next major release? The developers at Joomla have listened to the persistent noise created about only having MySQL as an option and are now accepting submissions for alternative database systems. (see http://groups.google.com/group/joomla-dev-cms/browse_thread/thread/1382dc6f4af56278#msg_9b95648941ef6fa7 for this development) My submission can be found at: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=18384 Things to watch out for a version-dependent features and clumsy implementations. Thanks Thom Brown
2009/10/20 Csaba Nagy <nagy@ecircle-ag.com>: > Hi Thom, > > I would like to review it, but I get "403 - Forbidden" when clicking: > > http://downloads.joomlacode.org/trackeritem/4/5/0/45041/postgresql.php > > Not sure what that means, probably I need some kind of login to the > joomla tracker system, and I don't have one, and I would prefer not to > create one... is it possible to access that somehow without full access > to the joomla tracker ? Hi Csaba, I didn't realise it wasn't publicly viewable. I've attached a copy of the php file anyhow. Can you see the tracker page at all? Thom
2009/10/20 Reid Thompson <reid.thompson@ateb.com>: > your attachment contains this... > > <?xml version="1.0" encoding="iso-8859-1"?> > <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" > "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> > <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en"> > <head> > <title>403 - Forbidden</title> > </head> > <body> > <h1>403 - Forbidden</h1> > </body> > </html> > Erk.. that's weird. I got that too even after being logged in. I'm not sure how anyone can review it if no-one has access to it. I've attached my working version which differs only slightly to conform with coding-styles required by Joomla. Apologies Thom
Attachment
2009/10/20 Merlin Moncure <mmoncure@gmail.com>: > On Tue, Oct 20, 2009 at 8:02 AM, Thom Brown <thombrown@gmail.com> wrote: >> If anyone has a moment, could they review the PostgreSQL driver I >> wrote for Joomla's next major release? The developers at Joomla have >> listened to the persistent noise created about only having MySQL as an >> option and are now accepting submissions for alternative database >> systems. (see http://groups.google.com/group/joomla-dev-cms/browse_thread/thread/1382dc6f4af56278#msg_9b95648941ef6fa7 >> for this development) >> >> My submission can be found at: >> http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=18384 >> >> Things to watch out for a version-dependent features and clumsy implementations. > > *) why bother with the version check in queryBatch? why not just do > begin->commit always? To be honest, I hadn't touched that function. That's entirely inherited from the MySQL one, but you're right, for Postgres there's no need to check the version. > *) no ability to delete? The driver has to match that of the MySQL one in Joomla, so I couldn't implement functionality which wouldn't be used by every driver. The only thing I added which differed was a concatentation operator which I put in for future implementation as I believe it will be necessary if they're going to adapt the existing codebase to support multiple database systems. > *) looks like typo on line 713 Yes, that's just left over from me quickly trying to tidy up my comments as I couldn't get the proper version from the site. Please ignore it as it isn't in my actual submission. Thanks Merlin! Thom
2009/10/20 Alban Hertroys <dalroi@solfertje.student.utwente.nl>: > You miss a keyword in your query in renameTable; it should be " RENAME TO ", > not " TO ". Thanks for spotting that. I've made my amendments for next submit. > Wouldn't it be convenient to have an EXPLAIN ANALYSE version of explain()? > Maybe with a boolean parameter? The problem is, I don't see where they're using this function, so if they're using it to EXPLAIN an INSERT, UPDATE or DELETE statement, ANALYZE would execute it... unless I roll it back straight after. > In insertObject() you have a query "SELECT $keyName AS 'id' FROM $table'" - > That line contains two syntax errors: 'id' (You probably meant "id") and > $table' (spurious trailing quote). Again, you're right and well spotted. :) Fixed both of those. Cheers for helping out Alban! :D Thom
2009/10/21 Csaba Nagy <nagy@ecircle-ag.com>: > Hi Thom, > > Sorry for the delay, I got sick in the meantime. I see that others > already did some review, I will do a quick one too, later maybe I'll > actually try it out... so after a quick review: > > * on line 218, the " ENCODING '$DBname')" part feels wrong, you probably > want hardcoded UTF8 encoding there ? > * as Merlin already commented, transactions are always safe in postgres, > this is no mysql ;-) > * again, as Merlin commented, getTableList is getting the data bases, > which doesn't make sense, but maybe you actually wanted to get the > tables - in this case you don't have a typo but you need to change the > query ;-) > > If I'll get some time I'll test it too, but likely not this week... Thanks for the feedback Csaba, Yes, I will change the function which uses transactions, and also replace any conditional UTF8 statements with hard-coded ones. The getTableList function...erm... not sure what I was thinking there. How about: select tablename from pg_tables where schemaname='public'; As for testing it, you won't be able to with the existing Joomla code base as most of the queries will fail as they've been terribly written, and that's because MySQL lets you get away with it for some reason. I imagine the only way to test it is to either get a copy of modified Joomla from me (although it still needs work), or create a test app which uses that class, but you will need the parent JDatabase class from Joomla too... and not sure how deep that rabbit hole goes as that will also in turn inherit from another class. Thanks again. Thom