-
Notifications
You must be signed in to change notification settings - Fork 40
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
Retroactive transliteration of file names broken with MySQL 8.0.22+ #4931
Comments
@jlfranklin Many thanks for reporting this issue. My suspicion is that it has something to do with some stricter query handling in MySQL 8 (and eventually recent MariaDB). I don't think it's related to utf8-general vs. utf8-mb4. The query looks a bit strange anyway...
I mean the First of all: I suspect, not only the test, but also the functionality is broken. I can't actually test locally, as I don't have MySQL 8 here on my dev. But you can. Steps to verify:
Is the file with camel-case name listed there? (I doubt it.) |
Although I'm currently not able to reproduce (no MySQL 8 here), I created a PR with a slightly different syntax. @jlfranklin does that patch change anything for your Ubuntu setup? |
Ah, there it is: https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-22.html " A solution for both, MySQL 5 and 8 will be tricky. The older does not support the case sensitive switch within the regex But maybe CAST or CONVERT could be used to get a case sensitive charset? @jlfranklin can you try the latest variant from my PR, please? |
I believe I can use MySQL 8 with lando on my local. I'll try this soon as I can get to it. |
The PR passes on MySQL 8. (Also passes on PostgreSQL, FWIW. SQLite never passed that test, so I have no expectation of it working with this PR.) |
@jlfranklin good to know. I was a bit concerned regarding PostgreSQL.
@klonos cool, many thanks! We still need to verify my suspicion that not only the test, but also the functionality is currently broken with MySQL 8 and gets fixed by the PR. |
OK, I've tested this:
Yes it was 😅 |
Many thanks for testing. 👍 Unfortunately... according to their changelog the breaking change came with MySQL 8.0.22. @klonos I suppose, lando doesn't ship with that version yet? |
They say that they do not officially support specifying patch versions, but it is possible: https://docs.lando.dev/config/mysql.html#supported-versions Gimme a few mins to try that... |
...nope. That didn't work 😅 ...I need to rest, and will return to it later today/tonight. |
Hey, no need to rush! @jlfranklin should be able to verify, as he obviously has an affected version. (The version seems to be 8.0.23-0ubuntu0.20.04.1.) |
You called it. The list is empty on my Ubuntu 20.04 VM: For reference:
|
@jlfranklin many thanks for verifying! I'll need to get a dev setup with MySQL 8.0.22+. That's not difficult, just a little time consuming - I need to setup an extra VM for that. Yes, that Exception is the same as in the failing test "...cannot be used in conjunction with "binary" in call to regexp_like...". |
I think the code looks fine. Doesn't look like it's stopping us from doing future abstraction if necessary. By the way, I was confused about the mention of testing with Postgres and Sqlite. Backdrop officially only supports MySQL (and MariaDB) so it's officially ok if it breaks other dbs. |
I can finally test this with Lando now!... For anyone else that wants to know how, please see: lando/lando#2948 (comment) |
I've installed a vanilla Backdrop instance, using:
I then tried the steps to reproduce provided by @indigoxela, but when I navigated to
I then destroyed the instance, and reinstalled with the changes in the PR. When I tried the steps to reproduce, I did not hit that PDOException, but the list in I then destroyed the instance once again, and installed again, this time with mysql 8.0.23 + the changes from the PR. This time when visiting [edit] please ignore my previous report re upload errors after deleting and trying to re-upload the transliterated file. This seems to be a pre-existing issue, and mot specific to mysql: #5424. So to summarize:
As such, I'm setting this back to "needs work". If the empty list in |
I'm a bit confused. @klonos you already realized that the problem you encountered is unrelated to the problem we try to fix here. And still you reject this fix? Mind to explain, why? Or am I missing the point? Anyway, rebased. I didn't change the lables and leave it to someone else to decide. |
Sorry for the confusion @indigoxela ...I've set it back to "needs work" because of this:
...but as I explained:
|
I tried to switch the GHA workflow to the exact version (8.0.22), but that's not supported. The currently installed 8 version with that action is 8.0.27, so I run the functional tests with that one. (Cool GHA!) The transliteration test passes with version 8.0.27, but there's another test failing, which points to another problem with either the test or the functionality behind it: DatabaseTemporaryQueryTestCase TBH, currently my motivation to keep on working on this PR is rather low. Even lower if I have to hunt some possible bug in an outdated MySQL version (8.0.22). The effort to install specific mysql versions is rather high for me and personally, I don't have any websites out there running on version 8. If anyone wants to take over, please do! I guess, DatabaseTemporaryQueryTestCase needs a new issue - that problem is unrelated here. |
@indigoxela do you agree that this issue still needs work or is it ready (and testing results as expected)? I had trouble following some of this. I agree about making a new issue for anything else. |
@indigoxela it is now (relatively) easy to install any patch-level version of mysql v8.0.x with Lando, but I'm assuming that your local tooling/workflow may be different. In any case, if you do get motivated to try that, then:
You should now have an instance with the version of mysql specified in step 2 above. To test with another version:
|
@klonos many thanks for the detailed instructions, but I never used Lando, I don't even do local development (on localhost) - I use remote webservers. So still someone else should carry it over the finish line.
@herbdool I can't actually tell, as I didn't verify the problem with version 8.0.22 (that exact version). |
Fair enough 👍🏼
It was initially mentioned by @jlfranklin back in Feb.:
And I was able to confirm now. |
I think, we should remove the milestone here. The existing PR unsuccessfully waited for review for months, and it's now obsolete anyway. |
Fine by me. |
@herbdool Which of it? 😏 Removing milestone or closing PR, or something else? |
Removing milestone and closing PR. Someone can always find the PR later if needed. |
By chance I had a VM with PHP 8.1 and MySQL 8.0.31 available and did a quick test run. Still a problem. See the verbose test result
|
Description of the bug
One test fails under Ubuntu 20.04 that does not fail under Debian 9 (php 7.0, MariaDB 10.1.47).
It may be a configuration or database issue. Database is using
default char set 'utf8'
, notutf8mb4
, andutf8mb4
is not enabled insettings.php
.Steps To Reproduce
To reproduce the behavior:
Actual behavior
Additional information
Add any other information that could help, such as:
The text was updated successfully, but these errors were encountered: