Page MenuHomePhabricator
Create Task
Maniphest T147852

MW unable to bypass MySQL strict mode by setting $wgSQLMode to blank
Closed, ResolvedPublic
Actions

Description

Today I noticed that the code added as part of https://gerrit.wikimedia.org/r/#/c/314474/ is breaking the update script on my dev wiki. Later, I noticed that SecurePoll has stopped working for me as well. In both cases, an INSERT or UPDATE query that was trying to add/modify data without specifying all columns was causing a MYSQL error 1364 (Field ... doesn't have a default value). Checking the DB schema showed those columns truly did not have a default value (examples are el_owner column of securepoll_elections table, or iw_api column of interwiki table).

I am using MW on a machine with MySQL using out-of-the-box settings. It appears that MySQL now uses STRICT_TRANS_TABLES mode by default. That means unless MW does something about it, strict mode will be enforced and all columns without default values must be specified in INSERT/UPDATE queries. That will break tons of code in MW core and MW extensions.

My assumption was that setting $wgSQLMode = '' in LocalSettings.php would fix that, but somehow it is not fixing the problem either.

The real bug we need to fix is "fix all bad SQL code"! But that is a huge undertaking. Until then, we should at least make sure MW disables strict mode for MySQL.

Details

SubjectRepoBranchLines +/-
Call parent::__construct() in MysqlBase AFTER transferring specific parametersmediawiki/coremaster+2 -2
Customize query in gerrit

Related Objects

Event Timeline

Huji created this task.Oct 11 2016, 2:28 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 11 2016, 2:28 PM
Huji triaged this task as High priority.Oct 11 2016, 2:29 PM
Huji added a project: MediaWiki-libs-Rdbms.
Huji updated the task description. (Show Details)
Huji updated the task description. (Show Details)Oct 11 2016, 2:31 PM
Huji updated the task description. (Show Details)
Zppix subscribed.Oct 11 2016, 2:32 PM

Could T147756 be related?

Reedy added a comment.Oct 11 2016, 2:33 PM
In T147852#2705715, @Zppix wrote:

Could T147756 be related?

No

Reedy renamed this task from Strict mode in MySQL breaks MediaWiki and its exntesions to Strict mode in MySQL breaks MediaWiki and its extensions.Oct 11 2016, 2:34 PM
Reedy added a subscriber: aaron.
Nikerabbit subscribed.Oct 11 2016, 2:43 PM

I have been running with STRICT_TRANS_TABLES for quite a long time and it generally works. Please file more detailed issues as sub tasks of T108255: Enable MariaDB/MySQL's Strict Mode if they are not there yet.

Huji added a comment.Oct 11 2016, 4:40 PM

@Nikerabbit: will add subtasks.

However, I have two questions for you:

  1. Why doesn't setting $wgSQLMode = '' fix the issue for me? I had to edit my.cnf and restart MySQL to disable the strict mode.
  1. Is there anyway we could have jenkins bot test the code to ensure it runs in strict mode? I cannot think of a way, but I don't know jenkins well enough.
Reedy added a comment.Oct 11 2016, 4:41 PM

Whether user/app options override server options isn't MW specific

Nikerabbit added a comment.Oct 11 2016, 4:45 PM

For what I know it's not even enabled by default.

ack wgSQLMODE -i includes/ --php
includes/DefaultSettings.php
1835:$wgSQLMode = '';
Reedy added a comment.Oct 11 2016, 7:24 PM
In T147852#2706107, @Nikerabbit wrote:

For what I know it's not even enabled by default.

ack wgSQLMODE -i includes/ --php
includes/DefaultSettings.php
1835:$wgSQLMode = '';
In T147852#2706098, @Huji wrote:
  1. Why doesn't setting $wgSQLMode = '' fix the issue for me? I had to edit my.cnf and restart MySQL to disable the strict mode.

I suspect setting no sql mode on connection doesn't override a setting.. You'd need to set some no strict mode, surely?

Krenair subscribed.Oct 11 2016, 7:38 PM
Huji added a comment.Oct 11 2016, 7:58 PM

Well, that is not true. Running the same query via MySQL command line, preceded by SET GLOBAL sql_mode = '' works.

Reedy added a comment.Oct 11 2016, 7:58 PM
In T147852#2707077, @Huji wrote:

Well, that is not true. Running the same query via MySQL command line, preceded by SET GLOBAL sql_mode = '' works.

Does MW send that if sql_mode = '' ?

Huji added a comment.Oct 11 2016, 8:00 PM
In T147852#2707080, @Reedy wrote:
In T147852#2707077, @Huji wrote:

Well, that is not true. Running the same query via MySQL command line, preceded by SET GLOBAL sql_mode = '' works.

Does MW send that if sql_mode = '' ?

Not sure.

Reedy added a comment.Oct 11 2016, 8:00 PM
		// Set SQL mode, default is turning them all off, can be overridden or skipped with null
		if ( is_string( $this->sqlMode ) ) {
			$set[] = 'sql_mode = ' . $this->addQuotes( $this->sqlMode );
		}
Huji added a comment.Oct 11 2016, 8:13 PM

@Reedy when I add $wgSQLMode = '' to 'LocalSettings.php` and then check the line of code you pasted above (from DatabaseMysqlBase.php I notice that the condition is not met. $this->sqlMode is not a string in this situation. Exploring why.

Huji added a comment.Oct 11 2016, 8:31 PM

@Reedy seems out that line of code (and in fact, the entire open() function seems to be executed before the __construct function is run. Therefore $this->sqlMode is not even populated when it is being checked.

Huji renamed this task from Strict mode in MySQL breaks MediaWiki and its extensions to MW unable to bypass MySQL strict mode by setting $wgSQLMode to blank.Oct 11 2016, 8:32 PM
Reedy added a comment.Oct 11 2016, 8:33 PM
In T147852#2707172, @Huji wrote:

@Reedy seems out that line of code (and in fact, the entire open() function seems to be executed before the __construct function is run. Therefore $this->sqlMode is not even populated when it is being checked.

Considering you're saying this is broken recently... I wonder if it's related to the refactoring @aaron has done in the last few weeks?

gerritbot subscribed.Oct 11 2016, 9:29 PM

Change 315423 had a related patch set uploaded (by Reedy):
Call parent::__construct() in MysqlBase AFTER transferring specific parameters

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

gerritbot added a project: Patch-For-Review.Oct 11 2016, 9:29 PM
gerritbot added a comment.Oct 12 2016, 4:49 PM

Change 315423 merged by jenkins-bot:
Call parent::__construct() in MysqlBase AFTER transferring specific parameters

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

Huji closed this task as Resolved.Oct 12 2016, 5:07 PM
Huji claimed this task.
ReleaseTaggerBot added projects: MW-1.28-release (WMF-deploy-2016-10-25_(1.28.0-wmf.23)), MW-1.28-release-notes.Oct 12 2016, 6:00 PM
Huji removed a project: Patch-For-Review.Nov 5 2017, 2:17 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL