MariaDB Incompatibility

[cruftex]

Prerequisites

• [X] I understand and accept the project's code of conduct.
• [X] I have already searched in existing issues and found no previous report of this bug.

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:

001 Scenario: one product in cart, quantity 1, can apply only the cart rule which is restricted to selected carrier # Features/Scenario/Cart/CartRule/FrontOffice/carrier.feature:160
      And I save all the restrictions for cart rule cartrule7                                                       # Features/Scenario/Cart/CartRule/FrontOffice/carrier.feature:173
        An exception occurred while executing 'DELETE FROM ps_cart_rule_carrier crc WHERE crc.id_cart_rule = ?' with params [3]:

        SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'crc WHERE crc.id_cart_rule = '3'' at line 1 (Doctrine\DBAL\Exception\SyntaxErrorException)

The cause is, that MariaDB is not supporting an alias for the DELETE statement. Minimal test:

MariaDB [test_prestashop]> DELETE FROM ps_prodcut xy WHERE id_product=123123;
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'xy WHERE id_product=123123' at line 1

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:

• update the documentation stating that: "Only MySQL is fully supported and tested. MariaDB users could run into issues and should report a bug"
• Add a field in the bug report with the database type and version

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

[seederp2p]

@cruftex the truth is, prestashop uses MariaDB for its official demo:

https://demo.prestashop.com/#/en/front

Database information MySQL version: 10.5.19-MariaDB-0+deb11u2

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.

[kpodemski]

What's your MariaDB version @cruftex ? I'm using 10.10, and I can run all the tests correctly.

@seederp2p

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.

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 :)

[seederp2p]

What's your MariaDB version @cruftex ? I'm using 10.10, and I can run all the tests correctly.

@seederp2p

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.

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... ;)

[kpodemski]

@seederp2p

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.

[seederp2p]

@seederp2p

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. I partner with some "so called" premium agencies with whom, on very recent conversations, all of them said to avoid upgrading for the moment.

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.

[cruftex]

@kpodemski

What's your MariaDB version @cruftex ? I'm using 10.10, and I can run all the tests correctly.

I am using 10.6.12-MariaDB shipped with Ubuntu 22.04. A quick test with the latest MariaDB (10.11) shows the same behaviour:

# docker run --detach --env MARIADB_ROOT_PASSWORD=my-secret-pw --name=temptest mariadb:latest
# docker exec -it temptest  mysql --password=my-secret-pw mysql --version
mysql  Ver 15.1 Distrib 10.11.2-MariaDB, for debian-linux-gnu (x86_64) using  EditLine wrapper
# docker exec -it temptest  mysql --password=my-secret-pw mysql -e 'delete from slow_log xy where xy.insert_id=123;'
ERROR 1064 (42000) at line 1: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'xy where xy.insert_id=123' at line 1

Did you run the test in develop branch?

There are only a few occurrences where the alias with delete is used.

[ChillCode]

hi @cruftex

In MariaDB, you can use aliases when performing a delete statement by following this syntax:

docker exec -it temptest mysql --password=my-secret-pw mysql -e 'delete xy from slow_log xy where xy.insert_id=123;'

As stated in MySQL docs:

If you declare an alias for a table, you must use the alias when referring to the table:

I believe you have identified an issue in the following code.

https://github.com/PrestaShop/PrestaShop/blob/2f9be0dd2792308f12ec8e15aa7a21402b19d624/src/Adapter/CartRule/Repository/CartRuleRepository.php#L516-L525

[cruftex]

@ChillCode

In MariaDB, you can use aliases when performing a delete statement by following this syntax...

You are speaking of MariaDB but referring to the MySQL documentation. The MariaDB documentation does not state the alias support:

https://mariadb.com/kb/en/delete/

[ChillCode]

Hi @cruftex

MariaDB uses the same documentation, also from the link you posted:

Any of SQL expression that can be calculated from a single row fields is allowed. Subqueries are allowed. The AS keyword is allowed, so it is possible to use aliases.

Did you try that command I posted? It works when we add the table alias.

[ChillCode]

Here you have more info related to this:

https://github.com/doctrine/dbal/pull/4472

https://github.com/doctrine/dbal/issues/4448

https://github.com/doctrine/dbal/issues/2297

[cruftex]

@ChillCode

Any of SQL expression that can be calculated from a single row fields is allowed. Subqueries are allowed. The AS keyword is allowed, so it is possible to use aliases.

Attention, that applies for "RETURNING" section of the delete statement only not for the delete itself.

[ChillCode]

@cruftex

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:

docker exec -it temptest mysql --password=my-secret-pw mysql -e 'delete xy from slow_log xy where xy.insert_id=123;'

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.

[grooverdan]

found this and requested - https://jira.mariadb.org/browse/MDEV-33988

[valentin-harrang]

Hi, @cruftex

Do you have a solution to fix the problem?

[cruftex]

@valentin-harrang The incompatibility issue can be fixed by slightly changing the queries. You need to remove the alias and replace it with the full table name. Interestingly, also now a MariaDB issue is open to fix it on their side, see above. But:

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.

[grooverdan]

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.

[matks]

@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.

So, only MySQL is the supported database.

it seems like bad time to drop support for MariaDB

Guys where did you read we are going to drop MariaDB? Please stop jumping to hasty conclusions 🤔

[ChillCode]

The issue reported, as mentioned is because this code:

     private function removeRestrictionsByName(CartRuleId $cartRuleId, string $entityName) 
     { 
         $this->connection->createQueryBuilder() 
             ->delete($this->dbPrefix . 'cart_rule_' . $entityName, 'crc') 
             ->where('crc.id_cart_rule = :cartRuleId') 
             ->setParameter('cartRuleId', $cartRuleId->getValue()) 
             ->execute() 
         ; 
     } 
 } 

There is no need to use alias.

So the following code would work:

     private function removeRestrictionsByName(CartRuleId $cartRuleId, string $entityName) 
     { 
         $this->connection->createQueryBuilder() 
             ->delete($this->dbPrefix . 'cart_rule_' . $entityName) 
             ->where('id_cart_rule = :cartRuleId') 
             ->setParameter('cartRuleId', $cartRuleId->getValue()) 
             ->execute() 
         ; 
     } 
 } 

If someone want to create a PR and solve this specific case this error will be gone.

[nicosomb]

I play with CI to launch integration tests on MariaDB https://github.com/PrestaShop/PrestaShop/pull/36049/files

[nicosomb]

@ChillCode If I'm not wrong I found 2 usages of aliases.

I opened a draft PR: https://github.com/PrestaShop/PrestaShop/pull/36052

[ChillCode]

Yes, also this one:

removeRestrictedCartRules

Searched through all PS code and those are the only ones using alias.

Thanks @nicosomb

[cruftex]

@matks

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.

[nicosomb]

Yes, also this one:

removeRestrictedCartRules

Searched through all PS code and those are the only ones using alias.

@ChillCode I already deleted it yesterday https://github.com/PrestaShop/PrestaShop/pull/36052/files 😉

[ChillCode]

Make sure to delete the alias from the where clause also.

Thanks!

[nicosomb]

Oh you're right, good catch.

[nicosomb]

This PR https://github.com/PrestaShop/PrestaShop/pull/36052 will fix the bug

But we (the project) have to take a decision: do we have to write that PrestaShop supports MariaDB? If yes, we have to adapt our testsuite (like I try here https://github.com/PrestaShop/PrestaShop/pull/36049). If we do that, the testsuite will be longer to execute ...

ping @PrestaShop/committers

[Hlavtox]

Interesting find, but yeah, it really happens. :-)))

Plesk Obsidian 18.0.60 10.2.44-MariaDB

[ChillCode]

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:

        $this->connection->executeQuery('DELETE `cp`,`c`,`o` FROM `' . $this->dbPrefix . 'cart_product` AS `cp` LEFT JOIN `' . $this->dbPrefix . 'cart` AS `c` ON `c`.id_cart = `cp`.id_cart LEFT JOIN `' . $this->dbPrefix . 'orders` AS `o` ON `cp`.id_cart = `o`.id_cart WHERE id_order = ' . $orderId);

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.

[nicosomb]

I reopen this issue as we have to decide what to do with MariaDB support.

[Hlavtox]

Ping @ShaiMagal

[kpodemski]

I reopen this issue as we have to decide what to do with MariaDB support.

PrestaShop must work with MariaDB. MariaDB is industry standard, it's not Windows vs Linux discussion here.

Been working with PS from 2020 and never saw incompatibilities with MariaDB.

Same, there might be some specific cases but most of my customers were on MariaDB and no issues at all for years.

[nicosomb]

@kpodemski I agree with you, but I can't decide alone.

We have to list all the changes to do: documentation, CI, etc.

[ChillCode]

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:

For consistency with the SQL standard and other RDBMS, table aliases are now supported in single-table as well as multi-table DELETE statements. (Bug #27455809)

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.

[cruftex]

@nicosomb Thanks for the PR and taking action! I was not daring to do a PR because it seemed that nobody feels this is actually an issue.

But we (the project) have to take a decision: do we have to write that PrestaShop supports MariaDB? If yes, we have to adapt our testsuite (like I try here https://github.com/PrestaShop/PrestaShop/pull/36049). If we do that, the testsuite will be longer to execute ...

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.

[grooverdan]

@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.

https://github.com/MariaDB/server/pull/3237

[robertsilen]

@kpodemski I agree with you, but I can't decide alone.

We have to list all the changes to do: documentation, CI, etc.

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.

[matks]

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

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 :)

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.

[robertsilen]

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

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 :)

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? :)

[matks]

Hi guys, thanks for the ping @robertsilen

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.

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.

[matks]

Here it goes https://github.com/PrestaShop/docs/pull/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 😉

[nicosomb]

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

[ChillCode]

@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:

https://github.com/PrestaShop/PrestaShop/issues/33021

Thanks @grooverdan for continuously making MariaDB compatible with MySQL, and @nicosomb for removing useless code.