Page MenuHomePhabricator

"Special:ActiveUsers" throws database query error with sql_mode=only_full_group_by
Closed, ResolvedPublic

Description

Setup

  • MediaWiki 1.27.1 (8c593e7) 22:49, 26 October 2016
  • PHP 7.0.15-0ubuntu0.16.04.4 (apache2handler)
  • MySQL 5.7.17-0ubuntu0.16.04.1

Issue
When trying to access "Special:ActiveUsers" I get the following error:

A database query error has occurred. This may indicate a bug in the software.

Query:

SELECT  user_name,user_id,COUNT(*) AS `recentedits`,qcc_title  FROM `querycachetwo`,`user`,`recentchanges`   WHERE qcc_type = 'activeusers' AND qcc_namespace = '2' AND (user_name = qcc_title) AND (rc_user_text = qcc_title) AND (rc_type != '5') AND (rc_type != '6') AND (rc_log_type IS NULL OR rc_log_type != 'newusers') AND (rc_timestamp >= '20170210201050') AND (NOT EXISTS (SELECT  1  FROM `ipblocks`   WHERE (ipb_user=user_id) AND ipb_deleted = '1'  ))  GROUP BY qcc_title ORDER BY qcc_title LIMIT 301

Function:

IndexPager::buildQueryInfo (ActiveUsersPager)

Error:

1055 Expression #1 of SELECT list is not in GROUP BY clause and contains nonaggregated column 'DB0231020151216.user.user_name' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by (localhost)

Backtrace:

#0 /.../w/includes/db/Database.php(901): DatabaseBase->reportQueryError('Expression #1 o...', 1055, 'SELECT  user_na...', 'IndexPager::bui...', false)
#1 /.../w/includes/db/Database.php(1234): DatabaseBase->query('SELECT  user_na...', 'IndexPager::bui...')
#2 /.../w/includes/pager/IndexPager.php(365): DatabaseBase->select(Array, Array, Array, 'IndexPager::bui...', Array, Array)
#3 /.../w/includes/pager/IndexPager.php(219): IndexPager->reallyDoQuery('', 301, true)
#4 /.../w/includes/pager/IndexPager.php(419): IndexPager->doQuery()
#5 /.../w/includes/specials/SpecialActiveusers.php(77): IndexPager->getBody()
#6 /.../w/includes/specialpage/SpecialPage.php(479): SpecialActiveUsers->execute(NULL)
#7 /.../w/includes/specialpage/SpecialPageFactory.php(576): SpecialPage->run(NULL)
#8 /.../w/includes/MediaWiki.php(282): SpecialPageFactory::executePath(Object(Title), Object(RequestContext))
#9 /.../w/includes/MediaWiki.php(745): MediaWiki->performRequest()
#10 /.../w/includes/MediaWiki.php(519): MediaWiki->main()
#11 /.../w/index.php(43): MediaWiki->run()
#12 {main}

Since 1.27 is LTS ...

Event Timeline

Kghbln created this task.Mar 12 2017, 8:21 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 12 2017, 8:21 PM
Kghbln updated the task description. (Show Details)Mar 12 2017, 8:22 PM
Reedy added a subscriber: Reedy.Mar 12 2017, 8:31 PM

I suspect you want to disable that mysql mode

You mean sql_mode=only_full_group_by? I must however note that this server is using the MySQL 5.7+ default settings.

Reedy added a comment.Mar 12 2017, 8:43 PM

And it's seemingly a breaking change made in a point release, which is weird. Quick glance suggests it's the same in mariadb et al in new enough versions

Is this a new install? Or a recent upgrade of mysql?

I'm imagining there might be a lot more broken queries, and needs more work than just fixing this special page...

Reedy renamed this task from "Special:ActiveUsers" throws database query error to "Special:ActiveUsers" throws database query error with sql_mode=only_full_group_by.Mar 12 2017, 8:43 PM
Ciencia_Al_Poder added a subscriber: Ciencia_Al_Poder.

As of MySQL 5.7.5, the default SQL mode includes ONLY_FULL_GROUP_BY: https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sqlmode_only_full_group_by

Even if this is a compatibility issue that can be "fixed" by changing SQL mode, it's actually non-standard, and conceptually erroneous, to not specify all non-aggregate fields in the GROUP BY clause. Consider it a flaw in mysql to not check against this. It would error out on mssql server as well, for example. This is an easy fix as well.

Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptMar 12 2017, 8:59 PM

The wiki as such is about 9 years old. The upgrade history is as follows (minor version updates excluded)

20.12.15MySQL5.1.73 to 5.6.27PHP5.3.3 to 5.6.11MW1.24.1 to 1.25.4
13.12.16MySQL5.6.31 to 5.7.16PHP5.6.11 to 7.0.8MW04.11.16 / 1.25.6 to 1.27.1

Indeed the upgrade to MySQL caused several issues with extensions. Hopefully the situation is not too bad.

Even if this is a compatibility issue that can be "fixed" by changing SQL mode

@Ciencia_Al_Poder For me this is not a problem since I can just disable it but think of all the shared hosting users which will inevitably profit from this "cool" mode.

Reedy added a comment.Mar 12 2017, 9:11 PM

Do they disable changing of SQL Mode? I thought it was fairly well accepted that it can be set/changed at connection time by most apps as appropriate

I have never seen a shared hosting provider allowing to change SQL settings. Admittedly I never looked for that. Usually I am happy if I can change a couple of PHP settings myself.

Reedy added a comment.Mar 12 2017, 9:18 PM

I have never seen a shared hosting provider allowing to change SQL settings. Admittedly I never looked for that. Usually I am happy if I can change a couple of PHP settings myself.

Usually it's changed at the application level, with the SQL command it sends the server upon connection, rather than changing the actual sql server settings

Ciencia_Al_Poder added a subscriber: aaron.EditedMar 12 2017, 9:25 PM

It was "incorrectly" fixed for PostgreSQL for T70087, by adding a condition to "only" produce valid SQL statements if the RDBMS supports implicitGroupby(), as a petition of @aaron in gerrit 147647. Quote from @aaron:

Can you only do this if implicitGroupby() is false? that way it want hurt the mysql query plan,

I'm trying to understand how adding those fields in the group by (which should be there, because that's what the results of the query expect, and because it's required for all serious RDBMS) can negatively impact in the query plan. This can probably be tagged as Technical-Debt as well...

Kghbln added a comment.EditedSep 9 2017, 1:10 PM

Anybody up for at least triaging this database query issue?

Still happening with:

  • MediaWiki 1.27.3 (2fd9800) 06:05, 7. Jul. 2017
  • PHP 7.0.22-0ubuntu0.16.04.1 (apache2handler)
  • MySQL 5.7.19-0ubuntu0.16.04.1

@aaron should probably reply T160298#3094400, or somebody just write a patch to remove the $dbr->implicitGroupby() logic, leaving all 3 columns in the group by inconditionally.

Change 377015 had a related patch set uploaded (by Aaron Schulz; owner: Aaron Schulz):
[mediawiki/core@master] Remove use of implicitGroupBy() in ActiveUsersPager

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

Kghbln assigned this task to aaron.Sep 11 2017, 7:50 AM
Kghbln triaged this task as Normal priority.

Note: The same error can be reproduced on mysql/mariadb if someone sets sql_mode to ONLY_FULL_GROUP_BY

Note: The same error can be reproduced on mysql/mariadb if someone sets sql_mode to ONLY_FULL_GROUP_BY

Another note: I am using the MySQL defaults at least for this mode so everybody doing so too will get the error.

Change 377015 merged by jenkins-bot:
[mediawiki/core@master] Remove use of implicitGroupBy() in ActiveUsersPager

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

aaron closed this task as Resolved.Oct 2 2017, 6:09 PM

Change 384549 had a related patch set uploaded (by Krinkle; owner: Aaron Schulz):
[mediawiki/core@REL1_29] Remove use of implicitGroupBy() in ActiveUsersPager

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

Krinkle reopened this task as Open.Oct 16 2017, 4:19 PM
Krinkle added a subscriber: Krinkle.

Per Gerrit:

Can this be packported at least to REL1_27 which is the branch this was originally reported for?

Change 384569 had a related patch set uploaded (by Reedy; owner: Aaron Schulz):
[mediawiki/core@REL1_28] Remove use of implicitGroupBy() in ActiveUsersPager

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

Change 384571 had a related patch set uploaded (by Reedy; owner: Aaron Schulz):
[mediawiki/core@REL1_27] Remove use of implicitGroupBy() in ActiveUsersPager

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

Reedy added a comment.Oct 16 2017, 6:04 PM

^ And there's two more cherry picked

Appropriate RELEASE-NOTES should be added and then the patches merged

Change 384569 merged by jenkins-bot:
[mediawiki/core@REL1_28] Remove use of implicitGroupBy() in ActiveUsersPager

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

Change 384549 merged by jenkins-bot:
[mediawiki/core@REL1_29] Remove use of implicitGroupBy() in ActiveUsersPager

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

Change 384571 merged by jenkins-bot:
[mediawiki/core@REL1_27] Remove use of implicitGroupBy() in ActiveUsersPager

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

Reedy closed this task as Resolved.Oct 16 2017, 7:25 PM
Reedy removed a project: Patch-For-Review.
Kghbln added a comment.EditedOct 17 2017, 2:19 PM

Awesome to get the backports! Thanks a lot! Please do not be angry with me but I just updated the wiki to use:

  • MediaWiki 1.27.3 (5b97740) 20:03, 16 October 2017
  • PHP 7.0.22-0ubuntu0.16.04.1 (apache2handler)
  • MySQL 5.7.19-0ubuntu0.16.04.1

... and I am still getting the very same error.

Kghbln reopened this task as Open.Oct 17 2017, 2:22 PM

Changing to open until this is revisited somehow.

aaron removed aaron as the assignee of this task.Oct 18 2017, 6:20 PM

It will really be nice to get a fix for this into 1.31 LTS. I know that this is apparently not a front-burner thing but an LTS is always a good opportunity. :)

@CCicalese_WMF FYI

Current broken setup:

  • MediaWiki 1.27.4 (3aae510) 22:32, 29 November 2017
  • PHP 7.0.22-0ubuntu0.16.04.1 (apache2handler)
  • MySQL 5.7.20-0ubuntu0.16.04.1
Reedy added a comment.Dec 8 2017, 11:47 PM

Can you post the sql it's failing on now? As with that patch... there should've been some difference (and aliasing user_name as qcc_title)

Reedy added a comment.EditedDec 8 2017, 11:51 PM

Or not, in 1.27/1.28... Why did the field alias disappear? (does it mean this is actually fixed in REL1_29/REL1_30?)

Change 396551 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@REL1_27] Copy in Field changes from Id8963924f036e9ed3fcdde3a8e54b7126b72356e

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

That's the current situation. Have not checked whether this is different now:

A database query error has occurred. This may indicate a bug in the software.

Query:

SELECT  user_name,user_id,COUNT(*) AS `recentedits`,qcc_title  FROM `querycachetwo`,`user`,`recentchanges`   WHERE qcc_type = 'activeusers' AND qcc_namespace = '2' AND (user_name = qcc_title) AND (rc_user_text = qcc_title) AND (rc_type != '5') AND (rc_type != '6') AND (rc_log_type IS NULL OR rc_log_type != 'newusers') AND (rc_timestamp >= '20171108235347') AND (NOT EXISTS (SELECT  1  FROM `ipblocks`   WHERE (ipb_user=user_id) AND ipb_deleted = '1'  ))  GROUP BY qcc_title ORDER BY qcc_title LIMIT 301

Function:

IndexPager::buildQueryInfo (ActiveUsersPager)

Error:

1055 Expression #1 of SELECT list is not in GROUP BY clause and contains nonaggregated column 'DB0231020151216.user.user_name' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by (localhost)

Backtrace:

#0 /../w/includes/db/Database.php(901): DatabaseBase->reportQueryError('Expression #1 o...', 1055, 'SELECT  user_na...', 'IndexPager::bui...', false)
#1 /../w/includes/db/Database.php(1234): DatabaseBase->query('SELECT  user_na...', 'IndexPager::bui...')
#2 /../w/includes/pager/IndexPager.php(365): DatabaseBase->select(Array, Array, Array, 'IndexPager::bui...', Array, Array)
#3 /../w/includes/pager/IndexPager.php(221): IndexPager->reallyDoQuery('', 301, true)
#4 /../w/includes/pager/IndexPager.php(419): IndexPager->doQuery()
#5 /../w/includes/specials/SpecialActiveusers.php(77): IndexPager->getBody()
#6 /../w/includes/specialpage/SpecialPage.php(479): SpecialActiveUsers->execute(NULL)
#7 /../w/includes/specialpage/SpecialPageFactory.php(577): SpecialPage->run(NULL)
#8 /../w/includes/MediaWiki.php(282): SpecialPageFactory::executePath(Object(Title), Object(RequestContext))
#9 /../w/includes/MediaWiki.php(735): MediaWiki->performRequest()
#10 /../w/includes/MediaWiki.php(509): MediaWiki->main()
#11 /../w/index.php(43): MediaWiki->run()
#12 {main}
Reedy added a comment.Dec 9 2017, 12:03 AM

Fancy testing that patch?

Fancy testing that patch?

@Reedy Works for me! Thank you!

Reedy closed this task as Resolved.Dec 9 2017, 12:13 AM
Reedy claimed this task.
Reedy removed a project: Patch-For-Review.

Change 396551 merged by jenkins-bot:
[mediawiki/core@REL1_27] Copy in Field changes from Id8963924f036e9ed3fcdde3a8e54b7126b72356e

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

Change 407695 had a related patch set uploaded (by Ejegg; owner: Reedy):
[mediawiki/core@fundraising/REL1_27] Copy in Field changes from Id8963924f036e9ed3fcdde3a8e54b7126b72356e

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

Change 407695 merged by jenkins-bot:
[mediawiki/core@fundraising/REL1_27] Copy in Field changes from Id8963924f036e9ed3fcdde3a8e54b7126b72356e

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