Skip to content

Conversation

@szaimen
Copy link
Contributor

@szaimen szaimen commented Aug 21, 2022

Signed-off-by: szaimen [email protected]

@szaimen
Copy link
Contributor Author

szaimen commented Aug 21, 2022

Cc @nickvergessen @ArtificialOwl it seems like tests fail with thr latest mariadb10.6 version: ERROR 1140 (42000) at line 2: Mixing of GROUP columns (MIN(),MAX(),COUNT(),...) with no GROUP columns is illegal if there is no GROUP BY clause. What can we do to fix this?

I made it running by not adding the sql-mode command from the drone config in #33607 and #33606...

@nickvergessen
Copy link
Member

I made it running by not adding the sql-mode command from the drone config

Tests failed, so we don't run them.... 🤕

If the query is an issue it needs to be adjusted

@szaimen
Copy link
Contributor Author

szaimen commented Aug 21, 2022

If the query is an issue it needs to be adjusted

And where can I find this query and why does it fail only with this new mariadb10.6 version and not with an old mariadb10.6 version?

@nickvergessen
Copy link
Member

Maybe because they fixed something in a maintenance release?

I can't even see the query nor anything as CI just died with a timeout

@szaimen
Copy link
Contributor Author

szaimen commented Aug 21, 2022

Maybe because they fixed something in a maintenance release?

possible

I can't even see the query nor anything as CI just died with a timeout

same

@szaimen
Copy link
Contributor Author

szaimen commented Aug 21, 2022

cc @dr-m maybe?

Are there some known issues with the latest mariadb10.6 release and

- --sql-mode=ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION
?

@dr-m
Copy link

dr-m commented Aug 22, 2022

@szaimen I only really know the InnoDB storage engine. On a quick read of the comments on this pull request, sql_mode=ONLY_FULL_GROUP_BY is what should cause the error to be reported for such invalid SQL. https://mariadb.com/kb/en/group-by/ does not say when the default setting of that might have been changed.

@szaimen
Copy link
Contributor Author

szaimen commented Aug 22, 2022

I only really know the InnoDB storage engine

Ah I see. Sorry for pinging you then!

On a quick read of the comments on this pull request, sql_mode=ONLY_FULL_GROUP_BY is what should cause the error to be reported for such invalid SQL. https://mariadb.com/kb/en/group-by/ does not say when the default setting of that might have been changed.

Thanks! We'll have a look!

@szaimen
Copy link
Contributor Author

szaimen commented Aug 22, 2022

Seems like removing ONLY_FULL_GROUP_BY indeed makes it work.

@nickvergessen since this was added intentionally, how can we debug and fix this?

@szaimen szaimen force-pushed the enh/noid/mariadb10.6-master branch from 6bcbda5 to 4e83a8c Compare August 22, 2022 08:13
@nickvergessen
Copy link
Member

nickvergessen commented Aug 22, 2022

Maybe the docker image itself is incompatible with the flag? I don't know. It fails outside of the nextcloud code, so should be good/easy to report to the maria docker people?

At least our tests need to run in this mode, I don't really care how it gets enabled

@szaimen
Copy link
Contributor Author

szaimen commented Aug 22, 2022

Maybe the docker image itself is incompatible with the flag?

I'll try that out locally.

@szaimen
Copy link
Contributor Author

szaimen commented Aug 22, 2022

Yes, this fails with the same log:

docker run -it --rm \
--name mariadb \
-e MYSQL_ROOT_PASSWORD=owncloud \
-e MYSQL_USER=oc_autotest \
-e MYSQL_PASSWORD=owncloud \
-e MYSQL_DATABASE=oc_autotest \
ghcr.io/nextcloud/continuous-integration-mariadb-10.6:latest \
--sql-mode=ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION

@szaimen
Copy link
Contributor Author

szaimen commented Aug 22, 2022

Reported in https://jira.mariadb.org/browse/MDEV-29347

@szaimen
Copy link
Contributor Author

szaimen commented Oct 10, 2022

Waiting for mariadb10.6.11 release which fixes the issue. https://hub.docker.com/_/mariadb/tags?page=1&name=10.6

@szaimen szaimen force-pushed the enh/noid/mariadb10.6-master branch from 44962c5 to fe2b851 Compare November 11, 2022 20:34
@szaimen szaimen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 11, 2022
@szaimen szaimen marked this pull request as ready for review November 11, 2022 20:35
@szaimen szaimen changed the title [WIP] use the updated mariadb10.6 container in drone use the updated mariadb10.6 container in drone Nov 11, 2022
@szaimen szaimen removed the blocked label Nov 11, 2022
@szaimen szaimen requested review from a team and nickvergessen November 11, 2022 20:35
@szaimen szaimen requested review from PVince81, blizzz and icewind1991 and removed request for a team November 11, 2022 20:35
@szaimen
Copy link
Contributor Author

szaimen commented Nov 11, 2022

MariaDB 10.6.11 is finally out that brings the fix. So this should finally be ready for review :)

@szaimen
Copy link
Contributor Author

szaimen commented Nov 13, 2022

/backport to stable25

@szaimen
Copy link
Contributor Author

szaimen commented Nov 15, 2022

CI failure unrelated

@szaimen szaimen merged commit c3d0b71 into master Nov 15, 2022
@szaimen szaimen deleted the enh/noid/mariadb10.6-master branch November 15, 2022 12:26
@backportbot-nextcloud
Copy link

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

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

Labels

3. to review Waiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants