Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL-Error for getRecentActivities() caused by GROUP BY #488

Closed
lduer opened this issue Jan 7, 2019 · 6 comments
Closed

SQL-Error for getRecentActivities() caused by GROUP BY #488

lduer opened this issue Jan 7, 2019 · 6 comments

Comments

@lduer
Copy link
Contributor

lduer commented Jan 7, 2019

Describe the bug
The DB-Query, generated in App\Repository\ActivityRepository::getRecentActivities() causes the following error on my production server (only production, not dev-machine)

An exception has been thrown during the rendering of a template ("An exception occurred while executing '
SELECT DISTINCT 
k0_.id AS id_0, k0_.start_time AS start_time_1, k0_.end_time AS end_time_2, k0_.duration AS duration_3, k0_.description AS description_4, k0_.rate AS rate_5, k0_.fixed_rate AS fixed_rate_6, k0_.hourly_rate AS hourly_rate_7, 
k1_.id AS id_8, k1_.name AS name_9, k1_.comment AS comment_10, k1_.visible AS visible_11, k1_.fixed_rate AS fixed_rate_12, k1_.hourly_rate AS hourly_rate_13, k2_.id AS id_14, k2_.name AS name_15, k2_.order_number AS order_number_16, k2_.comment AS comment_17, k2_.visible AS visible_18, k2_.budget AS budget_19, k2_.fixed_rate AS fixed_rate_20, k2_.hourly_rate AS hourly_rate_21, 
k3_.id AS id_22, k3_.name AS name_23, k3_.number AS number_24, k3_.comment AS comment_25, k3_.visible AS visible_26, k3_.company AS company_27, k3_.contact AS contact_28, k3_.address AS address_29, k3_.country AS country_30, k3_.currency AS currency_31, k3_.phone AS phone_32, k3_.fax AS fax_33, k3_.mobile AS mobile_34, k3_.mail AS mail_35, k3_.homepage AS homepage_36, k3_.timezone AS timezone_37, k3_.fixed_rate AS fixed_rate_38, k3_.hourly_rate AS hourly_rate_39, 
k0_.user AS user_40, k0_.activity_id AS activity_id_41, k0_.project_id AS project_id_42, k1_.project_id AS project_id_43, k2_.customer_id AS customer_id_44 
FROM kimai2_timesheet k0_ 
INNER JOIN kimai2_activities k1_ ON k0_.activity_id = k1_.id 
INNER JOIN kimai2_projects k2_ ON k0_.project_id = k2_.id 
INNER JOIN kimai2_customers k3_ ON k2_.customer_id = k3_.id 
WHERE k0_.end_time IS NOT NULL AND k1_.visible = 1 AND k2_.visible = 1 AND k3_.visible = 1 AND k0_.user = ? AND k0_.start_time > ? 
GROUP BY k1_.id, k0_.id 
ORDER BY k0_.end_time DESC 
LIMIT 10' 

with params [1, "2018-12-08 08:52:59"]:

SQLSTATE[42000]: Syntax error or access violation: 1055 'DBNAME.k0_.start_time' isn't in GROUP BY").

My prod-env:

  • Plesk: Plesk Onyx 17.8.11 Update Nr. 35
  • on OS: CentOS Linux 7.6.1810
  • MariaDB: 10.2.21-MariaDB
  • PHP: 7.2.13

My dev-env:

  • OS: Maverics 10.11.6
  • PHP: 5.7.18
  • MySQL: 7.2.9

Expected behavior
After the login, some of my custom additions to kimai didn't load (were working fine in dev, on my local machine) and the prod.log gave me this sql error.

Additional context
I also tried to update the setting config/packages/doctrine.yaml -> doctrine.dbal.server_version to mariadb-10.2.21 (as described here: https://symfony.com/doc/current/reference/configuration/doctrine.html#doctrine-dbal-configuration)

@lduer
Copy link
Contributor Author

lduer commented Jan 7, 2019

Solution

After doing some research and updating the settings on my prod-server, I've found my way:

Some articles (e.g. this) describe the new default settings of mysql 5.7 as "strict" which needs some adaptions.

The corresponding settings in MariaDB (see this article) allows me to adapt the sql-mode (ONLY_FULL_GROUP_BY,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION) and remove ONLY_FULL_GROUP_BY from the list.

SET @@GLOBAL.SQL_MODE = REPLACE(@@GLOBAL.SQL_MODE, 'ONLY_FULL_GROUP_BY', '');

I don't know how this affects the security or other settings on my server, but kimai works fine this way.

Note: I found this solution before creating the issue, but due to some misconfigurations, I haven't bring it to work with the right sql-mode settings. After all, I think that this Information needs to be documented.

Documentation

Regarding the install-documentation and kimai requirements, I suggest to add some sentences about the STRICT-mode and how to remove the ONLY_FULL_GROUP_BY-tag from the sql-mode in MariaDB and MySQL.

Doctrine Configuration

Maybe my solution is not the best way. As far as I can see, this could/should also be done in doctrine settings, a listener or something similar to set the sql-mode for the current connection. (The doctrine configuration for symfony allows to add a wrapper class: https://symfony.com/doc/current/reference/configuration/doctrine.html#doctrine-dbal-configuration)

@kevinpapst
Copy link
Member

Thanks for posting the solution. I will check if I can reproduce this.
Probably we can fix the SQL instead of having to reconfigure the SQL server, would be the better solution for most users ;-)

@kevinpapst
Copy link
Member

kevinpapst commented Feb 9, 2019

I did some research, as the flag ONLY_FULL_GROUP_BY was new to me and it makes sense. After reconfiguring my dev DB I was able to reproduce the error.
After hours looking for a good solution I could only come up with one, which gives proper results, see PR #550 ... but I don't like it performance wise. It gives much better results than before, but it needs 11 queries now: one to fetch all activity-project combinations and then one for each combination to fetch the matching timesheet entry to restart.
If you have any idea how that can be improved I would be very happy to discuss/integrate it.
Limiting the "recent activities" menu to 5 entries would at least shrink the amount fo queries down to 6 ... but thats a poor mans solution ;-)

@lduer
Copy link
Contributor Author

lduer commented Feb 9, 2019

For my current prod-setup, I removed the ONLY_FULL_GROUP_BY flag from the sql_mode=[...] line in the my.cnf. (to change it permanentely; source: https://blog.gabriela.io/2016/03/03/group-by-are-you-sure-you-know-it/)

For the long time path of kimai, I think it's important to fix this, but at the moment I'm ok with this - also with the query splitted into subqueries and a foreach.

@kevinpapst
Copy link
Member

That was a fantastic link!
Thanks for sharing, after reading it I was able to reduce the queries down to 2.

@lock
Copy link

lock bot commented Apr 10, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. If you use Kimai on a daily basis, please consider donating to support further development of Kimai.

@lock lock bot locked and limited conversation to collaborators Apr 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants