Skip to content

Group on PHP level as MariaDB can not handle this correctly #7031

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.

Sign up for GitHub

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

Jump to bottom
Merged
nickvergessen merged 5 commits into from
Mar 22, 2022
Merged

Group on PHP level as MariaDB can not handle this correctly #7031

nickvergessen merged 5 commits into from
Mar 22, 2022

Conversation

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Mar 21, 2022 •
edited
Loading

MySQL docs:

Reject queries for which the select list, HAVING condition, or ORDER BY
list refer to nonaggregated columns that are neither named in the GROUP BY
clause nor are functionally dependent on (uniquely determined by) GROUP BY columns.

MariaDB docs:

For SELECT ... GROUP BY queries, disallow SELECTing columns which are
not referred to in the GROUP BY clause, unless they are passed to an
aggregate function like COUNT() or MAX(). Produce a 1055 error.

@nickvergessen nickvergessen marked this pull request as draft March 21, 2022 08:21
@nickvergessen nickvergessen added this to the 💛 Next Major (24) milestone Mar 21, 2022
@nickvergessen nickvergessen self-assigned this Mar 21, 2022
@nickvergessen nickvergessen added 2. developing feature: api 🛠️ OCS API for conversations, chats and participants labels Mar 21, 2022
@nickvergessen nickvergessen force-pushed the bugfix/noid/try-setting-ONLY_FULL_GROUP_BY-option branch from dcf8db1 to acb1942 Compare March 21, 2022 08:30
@nickvergessen
Set ONLY_FULL_GROUP_BY option for MySQL
9e4f5cf 
Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
Add a unit test for getParticipantsByNotificationLevel
3af97fc 
Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
Group on PHP level as MariaDB can not handle this correctly:
9d5cfbd 
MySQL docs:
Reject queries for which the select list, HAVING condition, or ORDER BY
list refer to nonaggregated columns that are neither named in the GROUP BY
clause nor are functionally dependent on (uniquely determined by) GROUP BY columns.

MariaDB docs:
For SELECT ... GROUP BY queries, disallow SELECTing columns which are
not referred to in the GROUP BY clause, unless they are passed to an
aggregate function like COUNT() or MAX(). Produce a 1055 error.

Ref https://jira.mariadb.org/browse/MDEV-11588

Similarly Oracle seems to fail with the query before

Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen nickvergessen force-pushed the bugfix/noid/try-setting-ONLY_FULL_GROUP_BY-option branch from 8fc6919 to 9d5cfbd Compare March 21, 2022 10:09
@nickvergessen nickvergessen marked this pull request as ready for review March 21, 2022 10:09
@nickvergessen
Copy link
Member Author

nickvergessen commented Mar 21, 2022

/backport to stable23

@nickvergessen
Copy link
Member Author

nickvergessen commented Mar 21, 2022

/backport to stable22

@nickvergessen nickvergessen changed the title Set ONLY_FULL_GROUP_BY option for MySQL Group on PHP level as MariaDB can not handle this correctly Mar 21, 2022
@nickvergessen
Copy link
Member Author

nickvergessen commented Mar 21, 2022

More problems found in #7032
And we need to port them to this backportable branch.
Due to the drone config change #7032 can not be backported automatically

@nickvergessen
Do not load the session in getParticipantByAttendeeId() as no caller …
c27a6b3 
…needs it

Also fixes a problem with MariaDB ONLY_FULL_GROUP_BY and Oracle

Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
Do not load the session in getParticipantByActor() as no caller needs it
2e26a23 
Also fixes a problem with MariaDB ONLY_FULL_GROUP_BY and Oracle

Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen nickvergessen mentioned this pull request Mar 21, 2022
Merged
4 tasks
@nickvergessen
Copy link
Member Author

nickvergessen commented Mar 22, 2022

Please review very soon @vitormattos

@nickvergessen nickvergessen merged commit 9326949 into master Mar 22, 2022
@nickvergessen nickvergessen deleted the bugfix/noid/try-setting-ONLY_FULL_GROUP_BY-option branch March 22, 2022 10:54
@backportbot-nextcloud
Copy link

backportbot-nextcloud bot commented Mar 22, 2022

The backport to stable23 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

backportbot-nextcloud bot commented Mar 22, 2022

The backport to stable22 failed. Please do this backport manually.

@nickvergessen
Copy link
Member Author

nickvergessen commented Mar 22, 2022

Backporting in #7036

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danxuliu danxuliu Awaiting requested review from danxuliu

1 more reviewer

@vitormattos vitormattos vitormattos approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@nickvergessen nickvergessen

Labels

3. to review feature: api 🛠️ OCS API for conversations, chats and participants

Projects

None yet

Milestone

v14.0.0-beta.1

Development

Successfully merging this pull request may close these issues.

3 participants

@nickvergessen @vitormattos