Last modified: 2014-09-07 19:49:09 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 64043 - change in default schema for MSSQL complicates installation with PostgreSQL
change in default schema for MSSQL complicates installation with PostgreSQL
Status: ASSIGNED
Product: MediaWiki
Classification: Unclassified
Installer (Other open bugs)
1.23.0
All All
: High normal (vote)
: 1.23.x release
Assigned To: Ryan Schmidt
:
Depends on:
Blocks: postgres 70340
  Show dependency treegraph
 
Reported: 2014-04-17 05:31 UTC by Jeff Janes
Modified: 2014-09-07 19:49 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---
legoktm.wikipedia: Backport_to_Stable+


Attachments

Description Jeff Janes 2014-04-17 05:31:32 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.
Comment 1 Jeff Janes 2014-06-16 21:46:23 UTC
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?
Comment 2 Andre Klapper 2014-06-17 09:36:14 UTC
CC'ing Legoktm and Skizzerz who wrote/review the patch
Comment 3 Kunal Mehta (Legoktm) 2014-06-17 10:15:59 UTC
(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.
Comment 4 Ryan Schmidt 2014-06-17 13:30:44 UTC
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.
Comment 5 Gerrit Notification Bot 2014-06-27 01:19:29 UTC
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
Comment 6 Gerrit Notification Bot 2014-07-09 00:20:39 UTC
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
Comment 7 Gerrit Notification Bot 2014-07-09 00:36:28 UTC
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
Comment 8 Kunal Mehta (Legoktm) 2014-07-09 01:33:23 UTC
Merged into master.

Skizzerz, can you prepare a backport for the REL1_23 branch?
Comment 9 Jeff Janes 2014-07-16 21:19:44 UTC
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.
Comment 10 Andre Klapper 2014-08-05 22:24:27 UTC
Skizzerz: Could you shed some light on this and reply to comment 9, please?
Comment 11 Ryan Schmidt 2014-08-09 03:03:38 UTC
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.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links