Last modified: 2014-09-07 19:49:09 UTC
Change-Id: I25df82065a307b9abc30c694f8c8afff0996d7c1 added support for MSSQL It changed $wgDBmwschema in includes/DefaultSettings.php from 'mediawiki' to null, apparently in the belief that no other database engines were using this setting. But PostgreSQL does use that setting, and this change adversely affects the installation process. The user can still successfully install as long they manually specify a schema name. Leaving it as the now-default value of null or empty causes an error. I think this part of the change should be reverted, and some compensatory changes made so that MSSQL still works with the old default value. (Alas, I don't know what those changes in the MSSQL code would need to be, and couldn't test them). Adding a new feature shouldn't get to break the existing behavior of an existing feature for its convenience. Alternatively, we could change the PostgreSQL code so that when given a null $wgDBschema it transparently converts it to "public" (the default schema for PostgreSQL). If starting from scratch I actually would prefer using "public" rather than "mediawiki" as the default schema name, but we are not starting from scratch.
Since 1.23.0 is already released with this unresolved, is this malfeature now carved in stone? The command line installer does not have a way to change the default schema and the current default does not work in postgresql. So the command line installer does not work. If we do not revert this change, what is the next best thing? Add the dbschema to the command line installer (it is already there, just commented out)? Change the parts of the code that use the schema to automatically replace NULL with 'public' for PostgreSQL?
CC'ing Legoktm and Skizzerz who wrote/review the patch
(In reply to Jeff Janes from comment #1) > Since 1.23.0 is already released with this unresolved, is this malfeature > now carved in stone? No :) I'll let Skizzerz comment on the actual technical problems/solutions since I have very little experience in this area, but I marked this as a blocker for the next 1.23 release.
Leaving it at the default value of 'mediawiki' actually broke MySQL, SQLite, and all other DBMSes that did not use a schema after making the changes needed to support MSSQL, which is why the default changed to null. The Postgres installer explicitly adds $wgDBmwschema to LocalSettings.php, and has been doing so since version 1.7 (released over 8 years ago). Since it can be reasonably expected for this variable to be present in LocalSettings.php, modifying the default value shouldn't break any existing wikis. Wikis that have explicitly removed this setting should add it back to LocalSettings.php. I'll look into the CLI installer to see what it's doing later today, from a cursory glance it appears that you can explicitly pass --dbschema to the installer to set the default schema as a workaround. What is likely is that I'll end up hardcoding "mediawiki" as a default schema in the CLI installer in the event that it isn't given one.
Change 142472 had a related patch set uploaded by Skizzerz: (bug 64043) Set the default database schema to "mediawiki" so as not to break the CLI installer. https://gerrit.wikimedia.org/r/142472
Change 142472 had a related patch set uploaded by Legoktm: Set the default database schema to "mediawiki" so as not to break the CLI installer. https://gerrit.wikimedia.org/r/142472
Change 142472 merged by jenkins-bot: Set the default database schema to "mediawiki" so as not to break the CLI installer. https://gerrit.wikimedia.org/r/142472
Merged into master. Skizzerz, can you prepare a backport for the REL1_23 branch?
The merged patch set 4 in 1.24 caused other problems under PostgreSQL: https://gerrit.wikimedia.org/r/142472 Fatal error: Class 'Database' not found in /usr/local/apache2/htdocs/wiki_git/includes/installer/PostgresInstaller.php on line 161 The proposed patch set 2 did not cause this problem. I don't know why patch sets 3 and 4 were so different from sets 1 and 2, so I don't how to investigate this further.
Skizzerz: Could you shed some light on this and reply to comment 9, please?
Patch set 3 was a rework because it broke sqlite, thus breaking every unit test. The fix is incredibly simple; change Database::factory to DatabaseBase::factory on the line mentioned in the error. I am holding off uploading a patch set for the moment as I want to fix bug 69281 at the same time and I do not have the time right now to investigate that one. If I don't have something uploaded to gerrit by next weekend, poke me on IRC as I rarely check bugzilla and bugmail gets lost in all of my emails within a day or so.
