Piwik compatible with MySQL mode: ONLY_FULL_GROUP_BY

[anonymous-matomo-user]

What is the 'problem', well its mysql. In short its about this

Our and other mysql users use the best (valid) sql modes to get the best query's etc. One sql mode is ONLY_FULL_GROUP_BY You need to add all columns you select/use in the GROUP BY

SELECT name, address, MAhot smileyage) FROM t GROUP BY name; ERROR 1055 (42000): 't.address' isn't in GROUP BY

You do this to tackle some mysql problems. http://dev.mysql.com/doc/refman/5.0/en/sql-mode.html#sqlmode_only_full_group_by

But piwik don't use this way so upgrading/installing failes. To fix this we need manual add a sql command in https://github.com/piwik/piwik/blob/master/core/Db/Adapter/Pdo/Mysql.php

public function getConnection() { if ($this->_connection) { return $this->_connection; }

$this->_connect(); $this->_connection->setAttribute(PDO::ATTR_EMULATE_PREPARES, true);

SQL COMMAND -> "SET sql_mode='';"

return $this->_connection; }

Maybe its possible to make this an option or something, the other option is to add all columns to your group by query's confused smiley

Forum: http://forum.piwik.org/read.php?3,115060

[mattab]

is something broken when ONLY_FULL_GROUP_BY is enabled?

What is the actual error that you get please?

I think the best course of action would be:

• Set ONLY_FULL_GROUP_BY during the core + integration + plugin tests, so that our Continuous integration server shows us problems as early as possible

[lourdas]

I'm attaching a screenshot of my piwik database where ONLY_FULL_GROUP_BY is enabled. This needs work and it might not be a simple case (from personal experience, we had to rewrite some sql queries for ONLY_FULL_GROUP_BY to work for a project of ours).

[mattab]

Tests are now running with ONLY_FULL_GROUP_BY but unfortunately no tests are failing. Maybe the set global sqlmode is not working as expected?

UPDATE: Actually tests are failing! see https://travis-ci.org/piwik/piwik/jobs/33112799

[mattab]

If you experience this issue with Piwik please comment in this thread! If more users comment we will schedule it.

[gcmartijn]

Mysql has support for non-standard grouping. The fix is to group by all non-aggregate selected columns:

GROUP BY [all columns]

In mysql only (all other databases will raise an exception), grouping by fewer than all non-aggregated columns results in a random row for each unique combination of those columns that are grouped by.

That's why to set ONLY_FULL_GROUP_BY to make valid database query's so it will work everywhere. And more important, don't get random results.

[mattab]

That will help us for tickets like #2593 and #3914 Sqlite support and #500 Postgresql support

[mattab]

The goal of this issue is to enable ONLY_FULL_GROUP_BY in our CI setup (revert https://github.com/piwik/piwik/commit/49c4863788d96091b6d2949f14ad5360560756fa) and then implement the change in all SQL queries that make the build fail.

[rvullriede]

With MySQL 5.7 ONLY_FULL_GROUP_BY will become the default and a lot more user will experience this: http://mysqlserverteam.com/mysql-5-7-only_full_group_by-improved-recognizing-functional-dependencies-enabled-by-default/

[sander]

I’m experiencing this issue with MySQL 5.7. Most Piwik displays show:

Mysqli prepare error: Expression #1 of SELECT list is not in GROUP BY clause and contains nonaggregated column 'withCounter.idaction' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by

Is there a workaround?

[lourdas]

rant mode on Welcome to the wonderful world of MySQL and non SQL strictness. Enjoy. rant mode off

If by any chance Piwik is supposed to use in the future other database backends, like PostgreSQL or Oracle or MS SQL Server maybe, the Piwik developers must correct all these problematic SQL queries. IMHO, the fact that MySQL 5.7 will by default enable ONLY_FULL_GROUP_BY tells us nothing. Any serious database application developer should set their own sane (thus SQL strict) options by default.

[gcmartijn]

I'm the anonymous-piwik-user that posted this topic. The 'patch' is to add those line's in the file.

I only can say, don't use piwik because they don't want to create SQL strict (safe) query's. The first topic about this, was on a other forum more the a year ago. I guess that they don't know how important this is, and probably they don't want 100% correct output data.

My opinion is that, a program that collects/rapports statistics need to use the strict possible query's.

And loardas is right, when they want to move to a real database they will need to fix this ;)

[lourdas]

What I didn't know and found out the other day, is that ONLY_FULL_GROUP_BY can be enabled per session, so the administrator is not required to have it on globally. The interesting part is that other popular FOSS like Drupal and WordPress already do this (enable it per session). Maybe the Piwik developers should consider this.

[tsteur]

What I didn't know and found out the other day, is that ONLY_FULL_GROUP_BY can be enabled per session

This sounds good @mattab maybe we could give it a try for a quick fix

[mattab]

@tsteur try what? do you mean adding the ONLY_FULL_GROUP_BY on CI so our tests run with it? or "disabling" this ONLY_FULL_GROUP_BY mode?

[tsteur]

I meant "disabling" this ONLY_FULL_GROUP_BY. Not sure if it's a good idea though and whether we should actually do it. Maybe it would be a good temporary solution until we actually get to work on fixing it properly but we all know temporary solutions usually stay forever hehe

[lourdas]

@tsteur is right. While my MariaDB 10 configuration should include ONLY_FULL_GROUP_BY in sql_mode, it doesn't because Piwik throws a bunch of errors. Now it would be a good time for a patch to remove ONLY_FULL_GROUP_BY per session, so that any database admin would not remove ONLY_FULL_GROUP_BY from the global sql_mode variable and this would temporarily fix the upcoming change in defaults for MySQL 5.7. Then, the next logical step is to fix all queries with GROUP BY that are problematic.

On a side note, ONLY_FULL_GROUP_BY is just one MySQL quirk. What about dates like 0000-00-00 which again depending on the server sql_mode are accepted or not...? I just hope Piwik doesn't use dates like that for a null date or something of a special semantics.

[mattab]

Adding to 2.15.0 as this will be a quick fix, and will help prevent some issues.

[lourdas]

Please consider against using a empty sql_mode directive. The one I use at my production server is

sql-mode = STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_AUTO_VALUE_ON_ZERO,NO_ENGINE_SUBSTITUTION,NO_ZERO_DATE,NO_ZERO_IN_DATE

Running the latest version of Piwik (and in total for almost 2 years now) with no issues at all.

[tsteur]

Sounds good, might take a while to check for different modes but I reckon it will be helpful for now when knowing all Piwik installations will have the same SQL mode. We should also check re performance how long it takes to set this (should be very fast)

[tsteur]

Is it possible that someone tries the manual setting of SQL mode?

One needs to patch core/Db.php see https://github.com/piwik/piwik/compare/5124?expand=1#diff-4410ab928da865a7fe6bbb3dea2e48c2R34 and either core/Db/Adapter/Pdo/Mysql.php if PDO is used https://github.com/piwik/piwik/compare/5124?expand=1#diff-3f803240d77a636be35f90d3813d9cf7L12 or core/Db/Adapter/Mysqli.php if Mysqli is used https://github.com/piwik/piwik/compare/5124?expand=1#diff-16483da56b87bf02bb53a48a732958b2L12

I did a quick performance test and it takes about 60micro seconds to set it, both Mysqli and Pdo

[tsteur]

FYI: Once we created this issue, we should move this issue back to mid term or so and create a new issue just for the workaround as we won't be compatible with ONLY_FULL_GROUP_BY afterwards but would be still good to have at some point.

Actually the PR will be the issue, we just need to move this one to mid term afterwards

[lourdas]

@tsteur I've applied the patch to my production Piwik installation (2.14.3), added ONLY_FULL_GROUP_BY to my.cnf, restarted MySQL and Piwik runs fine.

[lourdas]

And link to documentation about MySQL SQL modes: https://dev.mysql.com/doc/refman/5.5/en/sql-mode.html#sql-mode-full (version 5.5)

[tsteur]

Thanks for the feedback!

[mattab]

Therefore could this issue be closed do you think? or do we need to actually add support for ONLY_FULL_GROUP_BY? So far i don't see a reason to work on this, since it should be enough to disable this mode in the current session.

[lourdas]

I consider disabling ONLY_FULL_GROUP_BY a workaround, a hack and not a real solution. As a temporary means to fix this ok, but in the long term, the developers should rewrite SQL queries to be SQL compliant. At least this would help in porting Piwik to use other DBMS, like maybe Postgresql or Oracle or whatever. It's a pity that such a wonderful piece of software is closely tight with only MySQL.