This repository has been archived by the owner. It is now read-only.

Improvement for PostgreSQL database. #1460

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
6 participants
Contributor

gpongelli commented Aug 17, 2012

These changes come from 2.5 CMS' Postgresql pull request to let CMS works on this database.

+ $result = str_replace('\0\0\0', chr(0).'*'.chr(0), $result);
+
+ return $result;
+
@realityking

realityking Aug 17, 2012

Contributor

Nitpick: Useless new line. Can you remove it? Thanks.

Contributor

realityking commented Aug 17, 2012

Could you explain a bit why these changes are needed for postgres?

Contributor

gpongelli commented Aug 17, 2012

Line removed.

About str_replace, these changes come from here.

Contributor

realityking commented Aug 17, 2012

Thanks.

Maybe I should be a bit more specific, the question I have is what's different about postgres that we need those changes. I'd just like to understand so we can avoid them in the future.

Contributor

gpongelli commented Aug 18, 2012

Sincerely I don't remember which was the error, try asking to @elkuku .
The only thing I remember is that without that code I wasn't able to correctly use joomla on Postgresql.

Committed code style changes.

Member

elkuku commented Aug 19, 2012

Hum, let me try - first I should state that I am no database expert at all ;)

There are two issues here. They exist both on Postgresql and on SQLite, a driver I am playing with..

When inserting new records the PK should be "NULL" not "0".

The db engine considers 0 a perfect valid integer, so it will insert a record with the primary key set to 0 instead of auto incrementing.

Mysql does not care and auto increments both on 0 and NULL
M$Sql - not tested.

Not sure how to hande this on a db specific level, so I opted for doing it in JTable.

(scratching my head.. what was that about.....)

I believe it was about a strange NULL character PHP generates when serializing stuff. The dabase engine can not handle them and will simply truncate the strings when those characters appear.

I also was not able to porperly escape those in the db driver so I opted for str_replaceIng back and forth in JSessionStorageDatabase.

Hope that makes some sense,
Nikolai

Contributor

pasamio commented Sep 2, 2012

Re 1:
That's a good addition, I've done something similar in the past on an SQLite project where the same behaviour exists. I think MySQL is probably the exception in letting zero values for auto-incrementing columns work like a null. MSSQL identity fields I think will have an issue with either zero or null so perhaps it'd be better to actually not include the field at all if it's empty in the insertObject (e.g. unset $k) which should work fine for all four of those platforms since it'll be assumed to be null if not specified. But we can look at that in another pull request.

Re 2:
From the PHP web site there's this:

Note: Object's private members have the class name prepended to the member name; protected members have a '*' prepended to the member name. These prepended values have null bytes on either side.

Which appears to cause PostgreSQL some issues as noted in the comments as well:
http://www.php.net/manual/en/function.serialize.php#96504

I don't actually think this is enough since if the null character is the problem, serialising an object with private members will also break. The change will only work for protected members however as soon as an object with private members is put into the session it will break again. I assume that single quotes were used to prevent PHP from turning the \0 into a literal null character by design.

Contributor

eddieajau commented Mar 16, 2013

I'm not seeing a clear decision here. If there are still fixes to be made please issue a pull request against the CMS repo. If it proves stable, we can incorporate it into the Framework.

Thanks.

@eddieajau eddieajau closed this Mar 16, 2013

Contributor

gpongelli commented Mar 16, 2013

As you can see, these changes are related to platform's files, so why have I to make a pull request to CMS repository ?

Contributor

dongilbert commented Mar 16, 2013

The CMS is taking the current platform code back into it's repo to ease maintenance of the legacy platform. The platform has been upgraded to the Joomla Framework; a package based, composer integrated platform. And, since there wasn't a clear decision on what to do here, we felt it best for the CMS to handle the change as they see fit, since it will effect them more than the framework. Does that help?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.