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

[Radiological review module] because sql_mode=only_full_group_by on mysql 5.7 (redmine 11703) #2494

Merged
merged 2 commits into from
Jan 20, 2017

Conversation

gluneau
Copy link
Contributor

@gluneau gluneau commented Jan 3, 2017

This fixes this error:

Error Code: 1055. Expression #1 of SELECT list is not in GROUP BY clause and contains nonaggregated column 'fi.FileID' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by

@gluneau gluneau added the Bug PR or issue introducing/requiring bug fixes (not mutually exclusive with the Feature label) label Jan 3, 2017
@gluneau gluneau added this to the 17.0 milestone Jan 3, 2017
@xlecours
Copy link
Contributor

xlecours commented Jan 3, 2017

I think the whole GROUP BY and GROUP CONCAT can be removed instead.
Can there be more than one recond in the tarchive table with the same substring_index(t.sourcelocation, '/', -1) result for the same Patient_Name? If so, let's delete some code :)

@@ -190,7 +190,8 @@ class NDB_Form_final_radiological_review extends NDB_Form
LEFT JOIN tarchive t ON (upper(t.PatientName)
LIKE CONCAT(upper(c.PSCID), '_', upper(s.CandID), '_', upper(s.visit_label), '%'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is costy... (my 2 cents)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you commented on the wrong line? (It's showing up as a comment on a line that doesn't contain any changes..)

@gluneau
Copy link
Contributor Author

gluneau commented Jan 6, 2017

Found that in my project, there are cases where there are multiple tarchive files, this would make a query without group by return more than one result.

Group concat takes care of showing the multiple source locations.

@alisterdev alisterdev added the Passed Manual Tests PR has undergone proper testing by at least one peer label Jan 19, 2017
@alisterdev
Copy link
Contributor

Successfully resurrected the module on MySQL 5.7 👍 🎖

@gluneau
Copy link
Contributor Author

gluneau commented Jan 20, 2017

@alisterdev If your manual tests passed, can you approve this pull request?

@alisterdev
Copy link
Contributor

@gluneau I though approving was for code review and I have no idea whether its good or not (especially given discussion above)

Copy link
Contributor

@alisterdev alisterdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manual tests worked!

@driusan driusan merged commit b7a2223 into aces:17.0-dev Jan 20, 2017
ridz1208 pushed a commit to ridz1208/Loris that referenced this pull request Jan 23, 2017
…ysql 5.7 (redmine 11703) (aces#2494)

* fix group by for mysql 5.7

* Prevents Subquery to returns more than 1 row
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug PR or issue introducing/requiring bug fixes (not mutually exclusive with the Feature label) Passed Manual Tests PR has undergone proper testing by at least one peer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants