User tests: Successful: Unsuccessful:
Install Joomla. Then check in the database all tables are utf8_unicode_ci or ut8mb4_unicode_ci
ok updated the code
updated
the tables or the database default collation?
the tables where already utf8mb4_unicode_ci before this PR.
@andrepereiradasilva I agree with your latest comment, Code looks ok now, but as far as I understand it, it will be only relevant if Joomla! is installed with the otpion to create the new database when it does not exist already, or am I wrong? The default collation of existing databases is not touched by Joomla! as far as I know up to now.
getAlterTableCharacterSet is only used on alterTableCharacterSet, so you should see no changes
since it's in the db main driver, i wonder if this could have impact on other databases .... like postgresql, for instance.
@andrepereiradasilva That's what I asked myself, too, because I also wanted to make a PR for this file, but then not seen where it was used ... but it cannot have any impact on the other database kinds, otherwise this "utf8_general_ci" all the time would also have confused them, or not? The joomla.sql for postgresql and sqlazure not contain any character set stuff, so maybe it is in the main driver but it seems to have an effect only for mysql.
Because this confused me and was less urgent for me than other utf8mb4 problems to be fixed, I have it then up for the moment to understand it.
I mean this is where shit starts to get hard :P getAlterTableCharacterSet
(and related sub method) are new for 3.5 - so I'm guessing they don't work on postgres and mssql - I'll try and verify on my postgres testbed in a bit (or maybe @alikon can advise as the postgres guru). @nikosdion any thoughts as you put in these methods? Maybe we need to do a mysql check at the top of these two methods
For the other methods (for getting the create statement) again I think getCreateDatabaseQuery
needs to have it's second parameter ($utf
) as false on non-mysql databases (as the utf8mb4 collations will not work on postgres afaik)
Labels |
Added:
?
|
Labels |
Added:
?
|
Actually, since there is no concept of utf8 vs utf8mb4 in PostgreSQL and MS SQL Server (as far as I can tell) you can simply implement this simple getAlterTableCharacterSet
method in each of these drivers:
public function getAlterTableCharacterSet($tableName)
{
return array();
}
REALITY CHECK: Then the alterTableCharacterSet method will return false immediately. However, do note that these methods are NOT used anywhere in the CMS.
Regarding the getCreateDatabaseQuery
you should override it in the PostgreSQL and MS SQL Server drivers with this simple thing:
protected function getCreateDatabaseQuery($options, $utf)
{
return 'CREATE DATABASE ' . $this->quoteName($options->db_name);
}
Please do remember that the special syntax to add a character only makes sense in MySQL (mysql, mysqli and pdomysql). In other db drivers it doesn't make sense.
REALITY CHECK: The core code as it was before my changes (last modified June 2nd 2013 by @mbabker) would still fail on non-MySQL databases when $utf was true. We never bumped into this problem because you can only create a database if you have root privileges in your database. Remember that this is never the case unless You Are Doing Something Completely Stupid™. For 2 1/2 years nobody reported a problem ergo it's not the "OMG! The sky is falling! Release blocker!" issue you made of it. It's just a long standing pesky bug which is there to remind us that either nobody creates databases with JDatabaseDriver or nobody uses Joomla! on non-MySQL hosts (or both, which is the evidence I have been collecting the last 2 years through Akeeba Backup but nobody is willing to listen to me, you had to reproduce stats collection and then wait another 1-2 years to make sense of them – but I digress).
So, don't panic! Implement two one-liner methods in four classes (JDatabaseDriverPostgresql, JDatabaseDriverSqlsrv, JDatabaseDriverSqlazure, JDatabaseDriverOracle) and you're done. Why four classes? If you are changing code which is NOT used anywhere in the CMS just for correctness' sake you should address all of the additional databases supported by JDatabase, not just the two that came first to your mind. And no, none of these is actually a release blocker since the code isn't actually used anywhere in the CMS.
Labels |
Removed:
?
|
Milestone |
Added: |
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-03-06 23:36:34 |
Closed_By | ⇒ | wilsonge |
imo just needed the override of those methods for non-mysql drivers
Changing collations for both utf8mb4 and utf8 cases to _unicode_ci was agreed with @wilsonge and @aschkenasy