-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
MariaDB Incompatibility #33157
Comments
@cruftex the truth is, prestashop uses MariaDB for its official demo: https://demo.prestashop.com/#/en/front Database information But I'm with you. It should be clear the compatibility between PS and MySQL and / or MariaDB. Pretty much like Wordpress does... Based on your PHP version, I assume you're running PS8 already. My personal feeling (beeing a PS user for over 10 years) is to keep the lowest version for the longest time possible. This is sad... but also true. |
What's your MariaDB version @cruftex ? I'm using 10.10, and I can run all the tests correctly.
That's an odd thing to write, especially in the issue pointing to the Behat tests :) I've been using PrestaShop for over 15 years, and I've never had any issues between the core and MariaDB :) the only things I remember were related to MySQL 8 migration :) |
That was just a comment. Not related to the issue itself... ;) |
A very inappropriate comment. A comment that undermines the great work that all project members and contributors from the community are doing. The latest versions of PrestaShop are great. If you don't want to use them, don't use them. But don't write that it's the best option because that's nonsense. |
Well, I'm entitled to my opinion... just as you. A real prestashop store tipically has third party modules, so the decision has two consider at least two factors: those third party modules to have been correctly upgraded to new PS major version version and the prestashop core itself should be mature. I'm not even talking about third party integrations with prestashop API, etc... In the end of the day this is a political/management decision and I keep my philosophy: keeping the most mature and supported version for the longest time possible while preparing to the next mature branch. This is way off topic and no one tried to undermine nothing here. Cheers and keep up. |
I am using
Did you run the test in There are only a few occurrences where the alias with delete is used. |
hi @cruftex In MariaDB, you can use aliases when performing a delete statement by following this syntax:
As stated in MySQL docs:
I believe you have identified an issue in the following code. PrestaShop/src/Adapter/CartRule/Repository/CartRuleRepository.php Lines 516 to 525 in 2f9be0d
|
You are speaking of MariaDB but referring to the MySQL documentation. The MariaDB documentation does not state the alias support: |
Hi @cruftex MariaDB uses the same documentation, also from the link you posted:
Did you try that command I posted? It works when we add the table alias. |
Here you have more info related to this: |
Attention, that applies for "RETURNING" section of the delete statement only not for the delete itself. |
If the following command works on your MariaDB there is no much left to say about this, and it means alias are valid to use:
I left doctrine related info where the issue resides if you want to learn. But this is an issue that sure the PrestaShop will figure out easily. |
found this and requested - https://jira.mariadb.org/browse/MDEV-33988 |
Hi, @cruftex Do you have a solution to fix the problem? |
@valentin-harrang I can find code that has the issue, which is not covered by the CI tests. I could change this code and create a pull request, but it is unclear how to properly test it. Its not a good idea to change queries and never execute them. So also it is easy to make the change, quality assurance is a problem. I was speaking to the PrestaShop maintainers about this on the PrestaShop dev conference. There is no plan to do regular testing with MariaDB. So, only MySQL is the supported database. As long as there is no regular integration testing and no indication that maintainers support MariaDB, the most correct "solution" would be not to use MariaDB and state in the installation documentation that MariaDB is not supported. |
Looking at this more. The original exception was Doctrine\DBAL - and @ChillCode listed the few Doctrine issues that corrected this. So it wasn't in Prestashop. Other issues also seem to relate to Doctrine integration. On QA Doctrine does test the range of MariaDB releases: https://github.com/doctrine/dbal/blob/4.0.x/.github/workflows/continuous-integration.yml#L273-L291. It would be a shame to drop MariaDB support as for the most part its worked well for users and from the MariaDB side (disclaimer - I'm a MariaDB Foundation employee) we do take pride in our backwards compatibility. cPanel also announced they where installing MariaDB by default soon it seems like bad time to drop support for MariaDB. Would PrestaShop maintainers consider a github nightly action? There is a github action start mariadb forked from the current action that would support it quite easily. |
@cruftex @grooverdan just to give some context: today when someone submits a Pull Request, it launches 35 GitHub Action jobs. A fair number of them are Behat tests and e2e tests that need to run the database. If we modify the CI to test these scenarios with both MySQL and MariaDB, we might increase the number of concurrent jobs to 40 or event 45 (need to be confirmed). This change will create more latency in GH actions running. In the past we've added lot of jobs without checking the numbers and ended in a situation where after a PR was submitted you had to wait for 6 hours before the jobs were finished.
Guys where did you read we are going to drop MariaDB? Please stop jumping to hasty conclusions 🤔 |
The opposite is the case: The official documentation only explicitly lists MySQL as a requirement. It is nowhere written that it works with MariaDB. However, people usually expect that they can use MariaDB as well, which is not the case right now. So, MariaDB, it seems, was actually never supported :) BTW: I am aware that "support" might mean different things to us and context. Here I mean it is on the same level than MySQL and receives same amount of testing and attention. |
@ChillCode I already deleted it yesterday https://github.com/PrestaShop/PrestaShop/pull/36052/files 😉 |
Make sure to delete the alias from the where clause also. Thanks! |
Oh you're right, good catch. |
Now add crc and will work, but doctrine querybuilder doesn't allow to do that, as per references I posted. Maybe I'm completely wrong or not getting what is wrong, but MariaDB, MSSQL and MySQL wants us to do it like this:
Doing this way we can control which rows to delete, in the previous command will delete cart_product, cart and order rows, if we only specify cp will only delete cp and so on. Been working with PS from 2020 and never saw incompatibilities with MariaDB. |
I reopen this issue as we have to decide what to do with MariaDB support. |
Ping @ShaiMagal |
PrestaShop must work with MariaDB. MariaDB is industry standard, it's not Windows vs Linux discussion here.
Same, there might be some specific cases but most of my customers were on MariaDB and no issues at all for years. |
@kpodemski I agree with you, but I can't decide alone. We have to list all the changes to do: documentation, CI, etc. |
For instance I ended up testing, MariaDB and MSSQL have the same behavior, both require the alias to be appended after DELETE, or command fails, MySQL (8.0.16=>) accepts both ways, and Oracle and PostgreSQL fails if be append it. Last time I tested it produced errors, but was an older MySQL before it was resolved, per documentation:
https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-16.html So MariaDB as per @grooverdan report should also allow it. Is not nice developing, also I find it useless, but per standards. |
@nicosomb
The pragmatic solution would be to switch to MariaDB to run the test suite. At the moment, no commands are known that run on MariaDB but not on MySQL. For releases testing the full support matrix makes sense. |
@ChillCode I feel your pain. As you are one of the developers trying to work in this compatibility environment, these are the sorts of compatibility issues that MariaDB wants to hear about. Fixes like the following aren't particularly difficult. |
Hello @nicosomb @kpodemski , I searched for "MySQL" in the documentation, there seems to be about 17 pages. Happy to help in updating github.com/PrestaShop/docs. |
Hello everyone First of all, thank you everybody for the different informations and feedbacks provided here. It's really great to help make informed decisions 👍 There are hundred of GitHub issues so from a maintainer perspective we sometimes are simply flooded with topics to think and we might overlook some of them. So if you think a topic is important, don't hesitate to push attention to it. Just like this was done here. As you can see @nicosomb is checking to solve the problem detected. But as said above, it's only about this particular issue, it does not solve
Just like @kpodemski said, IMO PrestaShop MUST work with MariaDB. MariaDB is industry standard, that's a no-brainer. I'm going to push the groups @PrestaShop/tech-council and @PrestaShop/committers to have a consensus here, and then be able to write down in the devdocs that we CLEARLY support MariaDB too. And we'll find a pragmatic way to detect issues specific to MariaDB without running all CI steps with both database engines to avoid slowing down drastically the CI. |
Hi @matks, is that process for MariaDB support publicly shared somewhere, any link to follow? Any way to support this? :) |
Hi guys, thanks for the ping @robertsilen
As nobody told me NO when I reached out to as many project members as I could, I assume it's a YES 😄 I'm going to submit the PR to update the devdocs right now. |
Here it goes PrestaShop/docs#1807 However please remember: we do not have the intention of running every step of our CI with both MariaDB and MySQL because it would be very expensive while the two SQL implementations are supposed to be very similar. We will find another way to detect MariaDB incompatibilities 😉 |
It's merged, thank you @matks. Just FYI, our README was already OK with MariaDB https://github.com/PrestaShop/PrestaShop/blob/develop/README.md#server-configuration |
@matks when @cruftex reported the "issue", I had a Debian instance running MySQL 5.6 and the command also failed, but upgraded that one to bookworm in January so also upgraded MySQL and then it worked, so if tests were run on minimum MySQL version required, 5.6, those would also fail. So dev who pushed this was not to blame, but if 9.0 was released with this code more than one would complain about compatibility. So I don't get why running tests against 8.0 when minimum required version is 5.6. A related issue for not testing with 5.6: Thanks @grooverdan for continuously making MariaDB compatible with MySQL, and @nicosomb for removing useless code. |
FYI as MySQL and MariaDB are supposed to behave almost identically, we see no benefit in running every CI step with both engines. But we will explore running the nightly test campaign with both. |
Good job @Progi1984 #36295 🔥 |
Prerequisites
Describe the bug and add attachments
I am using MariaDB and tried to run the behavioural test suite and get several errors. Around 12 errors are related to a problem with the database and all of the same kind:
The cause is, that MariaDB is not supporting an alias for the
DELETE
statement. Minimal test:I can find a few places where the alias feature is used, but these seem not covered by the automated tests.
When I search MariaDB in the issues, 19 issues are shown. From this, I conclude, that MariaDB is used with PrestaShop and thought to be working. Looking through the issues briefly, I haven't found an indication that MariaDB is not to be used.
The CI tests run only with MySQL.
Expected behavior
I suggest to do the following, instantly:
If MariaDB should be supported, it needs to be added to the CI tests in the long run I think.
Since the problem seems minor and I know what to do now, I will stay with MariaDB in our environment and send some patches.
Steps to reproduce
Run the behavioural tests with MariaDB
PrestaShop version(s) where the bug happened
develop
PHP version(s) where the bug happened
8.1
If your bug is related to a module, specify its name and its version
No response
Your company or customer's name goes here (if applicable).
No response
The text was updated successfully, but these errors were encountered: