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=Trac kerItemEdit&tracker_item_id=18384

Things to watch out for a version-dependent features and clumsy implementations.

Thanks

Thom Brown

--
Sent via pgsql-php mailing list (pgsql-php@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-php

Re: PostgreSQL driver for Joomla review

Hi Thom,

I would like to review it, but I get "403 - Forbidden" when clicking:

http://downloads.joomlacode.org/trackeritem/4/5/0/45041/post gresql.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 ?

Cheers,
Csaba.

On Tue, 2009-10-20 at 14:02 +0200, Thom Brown 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=Trac kerItemEdit&tracker_item_id=18384
>
> Things to watch out for a version-dependent features and clumsy implementations.
>
> Thanks
>
> Thom Brown
>


--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

Re: [GENERAL] PostgreSQL driver for Joomla review

--0023545bd064e6123104765d7297
Content-Type: text/plain; charset=ISO-8859-1

2009/10/20 Csaba Nagy :
> Hi Thom,
>
> I would like to review it, but I get "403 - Forbidden" when clicking:
>
> http://downloads.joomlacode.org/trackeritem/4/5/0/45041/post gresql.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

--0023545bd064e6123104765d7297
Content-Type: application/octet-stream; name="postgresql.php"
Content-Disposition: attachment; filename="postgresql.php"
Content-Transfer-Encoding: base64
X-Attachment-Id: f_g10nutma0

PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iaXNvLTg4NTktMSI/Pgo8
IURPQ1RZUEUgaHRtbCBQVUJMSUMgIi0vL1czQy8vRFREIFhIVE1MIDEuMCBU
cmFuc2l0aW9uYWwvL0VOIgogICAgICAgICAiaHR0cDovL3d3dy53My5vcmcv
VFIveGh0bWwxL0RURC94aHRtbDEtdHJhbnNpdGlvbmFsLmR0ZCI+CjxodG1s
IHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5L3hodG1sIiB4bWw6bGFu
Zz0iZW4iIGxhbmc9ImVuIj4KIDxoZWFkPgogIDx0aXRsZT40MDMgLSBGb3Ji
aWRkZW48L3RpdGxlPgogPC9oZWFkPgogPGJvZHk+CiAgPGgxPjQwMyAtIEZv
cmJpZGRlbjwvaDE+CiA8L2JvZHk+CjwvaHRtbD4K

--0023545bd064e6123104765d7297
Content-Type: text/plain
Content-Disposition: inline
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable


--=20
Sent via pgsql-php mailing list (pgsql-php@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-php

--0023545bd064e6123104765d7297--

Re: [GENERAL] PostgreSQL driver for Joomla review

--0023545bd06465813804765dd9dc
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

2009/10/20 Reid Thompson :
> your attachment contains this...
>
>
> > =A0 =A0 =A0 =A0 "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
>
> =A0
> =A0
> =A0
> =A0
> =A0

403 - Forbidden

Re: PostgreSQL driver for Joomla review

On Tue, Oct 20, 2009 at 8:02 AM, Thom Brown wrote:
> If anyone has a moment, could they review the PostgreSQL driver I
> wrote for Joomla's next major release? =A0The 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=3DTr ackerItemEdit=
&tracker_item_id=3D18384
>
> Things to watch out for a version-dependent features and clumsy implement=
ations.

*) why bother with the version check in queryBatch? why not just do
begin->commit always?
*) no ability to delete?
*) looks like typo on line 713


keep up the good work...

merlin

--=20
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

Re: PostgreSQL driver for Joomla review

2009/10/20 Merlin Moncure :
> On Tue, Oct 20, 2009 at 8:02 AM, Thom Brown wrote:
>> If anyone has a moment, could they review the PostgreSQL driver I
>> wrote for Joomla's next major release? =A0The 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_threa=
d/thread/1382dc6f4af56278#msg_9b95648941ef6fa7
>> for this development)
>>
>> My submission can be found at:
>> http://joomlacode.org/gf/project/joomla/tracker/?action=3DTr ackerItemEdi=
t&tracker_item_id=3D18384
>>
>> Things to watch out for a version-dependent features and clumsy implemen=
tations.
>
> *) why bother with the version check in queryBatch? =A0why 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

--=20
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

Re: PostgreSQL driver for Joomla review

On 20 Oct 2009, at 14:02, Thom Brown 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=Trac kerItemEdit&tracker_item_id=18384
>
> Things to watch out for a version-dependent features and clumsy
> implementations.


You miss a keyword in your query in renameTable; it should be " RENAME
TO ", not " TO ".

Wouldn't it be convenient to have an EXPLAIN ANALYSE version of explain
()? Maybe with a boolean parameter?

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

Regards,
Alban Hertroys

--
Screwing up is the best way to attach something to the ceiling.


!DSPAM:737,4addf36611682031315238!



--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

Re: PostgreSQL driver for Joomla review

2009/10/20 Alban Hertroys :
> 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

--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

Re: PostgreSQL driver for Joomla review

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

Cheers,
Csaba.


On Tue, 2009-10-20 at 15:28 +0200, Thom Brown wrote:
> 2009/10/20 Reid Thompson :
> > your attachment contains this...
> >
> >
> > > > "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
> >
> >
> >
> >
> >
> >

403 - Forbidden

Re: PostgreSQL driver for Joomla review

2009/10/21 Csaba Nagy :
> 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

--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general