Page MenuHomePhabricator

change in default schema for MSSQL complicates installation with PostgreSQL
Closed, ResolvedPublic

Description

Change: rMWf7174057a4ec7b9c74babfe74290bb000204a581 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.


Version: 1.23.0
Severity: normal

Details

Reference
bz64043

Related Objects

View Standalone Graph
This task is connected to more than 200 other tasks. Only direct parents and subtasks are shown here. Use View Standalone Graph to show more of the graph.

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 3:13 AM
bzimport set Reference to bz64043.
Jjanes created this task.Apr 17 2014, 5:31 AM

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.

skizzerz wrote:

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?

skizzerz wrote:

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.

Can this be closed or is the still a problem

Paladox set Security to None.Jul 6 2015, 12:03 AM
Paladox added a subscriber: Aklapper.
Jjanes added a comment.Jul 6 2015, 6:14 AM

It is fixed in 1.25.1 and 1.26alpha

It is not fixed in 1.23.9, but I don't think it is worth fixing so late in the release life-time.

Well 1.23 is a lts release. So it still has maybe 2 to 3 years left of updates. If you could find the patch that fixed the problem I can upload it to MediaWiki 1.23.

saper added a subscriber: saper.Oct 24 2015, 2:13 AM

@Paladox would be great, thanks!

saper assigned this task to Paladox.Oct 24 2015, 2:17 AM

@saper what should I do to fix the problem. Since I am not sure where the patch is that fixed the problem.

Change 248613 had a related patch set uploaded (by Paladox):
Change $wgDBmwschema back to mediawiki instead of null

https://gerrit.wikimedia.org/r/248613

Paladox added a comment.EditedOct 24 2015, 11:14 AM

In mediawiki 1.27 I see that it is done like this

public function getGlobalDefaults() {

		// The default $wgDBmwschema is null, which breaks Postgres and other DBMSes that require
		// the use of a schema, so we need to set it here
		return array_merge( parent::getGlobalDefaults(), array(
			'wgDBmwschema' => 'mediawiki',
		) );

}

In MssqlInstaller.php file.

It was added in http://git.wikimedia.org/commitdiff/mediawiki%2Fcore/30a82aae9cf7e5536eef761637a1277b7c18017e

But this needs to be backported.

Change 248613 abandoned by Paladox:
Change $wgDBmwschema back to mediawiki instead of null

https://gerrit.wikimedia.org/r/248613

Change 248614 had a related patch set uploaded (by Paladox):
Set the default database schema to "mediawiki" so as not to break the CLI installer.

https://gerrit.wikimedia.org/r/248614

Change 248614 merged by jenkins-bot:
Set the default database schema to "mediawiki" so as not to break the CLI installer.

https://gerrit.wikimedia.org/r/248614

Can we close this as resolved now. Since the patch was backported and merged in mediawiki 1.23?

Aklapper closed this task as Resolved.Aug 11 2016, 12:11 PM

Looks like. Thanks everybody for the patience, help, and attention for detail!

Closing as resolved.

Paladox removed a subscriber: gerritbot.
Jdforrester-WMF added a subscriber: Jdforrester-WMF.

Migrating from the old tracking task to a tag for PostgreSQL-related tasks.