Do not cast the order by expression for MySQL drivers as casting a TE…

[andysh-uk]

Fixes #1971 - the CAST to TEXT causes an error on MySQL.

This follows similar logic implemented in src/Doctrine/Query/Cast.php.

Getting the backend driver name (e.g. mysql) is a long chain of method calls but I cannot see any other way. It also uses getDatabasePlatform()->getName() instead of the getDriver()->getName() call in Cast.php as the docblock on that method suggests it is deprecated (but no alternative suggested.)

[bobdenotter]

This looks like a correct fix, but in the last two weeks this has been changed back and forth..

@Wieter , do you know of a way to make it work on Postgres, without breaking MySQL?

[Wieter]

@bobdenotter this fix seems incorrect to me, as the queries here are built against Doctrine DRM and not SQL.
Therefore, this CAST will not be 'native SQL', but should fetched by Doctrine in src/Doctrine/Query/Cast.php.
There, it is either translated into an SQL cast, or omitted (by the driver check).
I'll install mariaDB and run some tests of my own to see what the $backend_driver yields.

[andysh-uk]

My findings were that the line in src/Doctrine/Query/Cast.php was never hit. I had a var_dump just after $backend_driver (before the if condition) followed by an exit(), and I was still getting the exception.

Therefore it felt like the CAST in OrderDirective.php was getting passed as-is to MariaDB, and not being filtered out by the condition in Cast.php.

My $backend_driver does indeed yield “mysql”.

[Wieter]

My .env was just erased by d2da6b9, it will take a bit more time to get all connections back as I didn't bother to save the dev passwords elsewhere.. :)

[Wieter]

@andysh-uk when I insert in src/Doctrine/Query/Cast.php on line 24: exit(print($backend_driver)); I don't get an error but I do get, in my case, 'pdo_pgsql' returned and the page stops rendering. So the src/Doctrine/Query/Cast.php was hit.
I still have to look into how to test mariadb without breaking mysql dependencies.. I guess this has to be yet another VM :)

[andysh-uk]

@Wieter could there be something in Doctrine that it knows how to render the SQL for CAST against MySQL, that it doesn’t with Postgres, so it calls your function for Postgres but not MySQL?

If you can provide me with your IP address I’m more than happy to give you access to my MariaDB dev server, if that helps? If you don’t want to disclose it here, you can send it through my website

[Wieter]

@andysh-uk thanks for the offer, but I managed. My ip address is temporarily super-dynamic as we still have troubles with the ISP ('unlimited' 4G to the rescue..).

I was not able to reproduce the error.
Steps taken (roughly):

• install MariaDB: Server version: 10.3.22-MariaDB-1ubuntu1 Ubuntu 20.04
• install php 7.4 with necessary extensions, php-curl php-xml php-pgsql php-sqlite3 php-mysql
• clone https://github.com/Wieter/core/tree/development-postgres and checkout development-postgres
• edit .env -> DATABASE_URL=mysql://bolt_dev:"..."@localhost:3306/bolt_dev
• the usual composer/npm install procedure
• add your content-type 1:1 as described in Setcontent with order by causes SQL exception in 4.1 #1971
• add some dummy reviews (first 4, later >10), some without name or content or date.
• implemented {% setcontent reviews = 'reviews' limit 6 orderby '-date' %} within public/theme/skeleton/listing.twig with corresponding dump, loop, and dump-per-review.
  all works flawlessly
  The file src/Doctrine/Query/Cast.php is called correctly, and the $backend_driver = pdo_mysql.

I wonder.. what happens with a composer-only fetch and install, through the official release channels. I'll try that later.

[Wieter]

I managed to get the error now and will look into it. #7878 contains the exact steps to reproduce.

[Wieter]

@bobdenotter I think I found the culprit.

using composer install creates an config/packages/doctrine.yaml within the users project folder.
In there, Doctrine>Orm>dql: CAST: DoctrineExtensions\Query\Mysql\Cast
This apparently loads ./vendor/beberlei/doctrineextensions/src/Query/Mysql/Cast.php
and this in turn collides with our Cast.php.

I see more is missing from the doctrine config ( config/packages/doctrine.yaml ) it should be:

doctrine:
    orm:
        dql:
            string_functions:
                JSON_EXTRACT: Bolt\Doctrine\Functions\JsonExtract
                JSON_GET_TEXT: Scienta\DoctrineJsonFunctions\Query\AST\Functions\Postgresql\JsonGetText
                CAST: Bolt\Doctrine\Query\Cast

[andysh-uk]

Thanks @Wieter. Is that why I couldn’t get the line to be hit in src/Doctrine/Query/Cast.php?

That would make better sense than my proposed fix in this PR, knowing now that the query I changed is DQL, not pure SQL.

[Wieter]

yes @andysh-uk
Could you try modifying (commenting out the old values) your config/packages/doctrine.yaml to the definitions I gave in my previous comment, and see if that fixes/breaks things for your MariaDB installation?

[andysh-uk]

@Wieter that works perfectly, many thanks 😄

I rolled my change back to make sure I was still getting the exception (I was) and then applied your changes to config/packages/doctrine.yaml and refreshed the page - works great.

I did double-check that src/Doctrine/Query/Cast.php is called and my driver name is "pdo_mysql" - which is a yes to both.

Just FYI, PhpStorm flags the getName() call on the Doctrine Driver object as deprecated - and this PR has removed it.

A better option might be to use getDatabasePlatform()->getName() instead of getDriver()->getName() for when this method is removed. This method returns mysql instead of pdo_mysql.

[Wieter]

Good to hear it works.
Also; this means that PostgreSQL people will have troubles installing Bolt 4.1 as well using composer install..

Good finding, indeed getDriver() should be changed to getDatabasePlatform() should be changed. Feel free to implement and run some tests ;)

[andysh-uk]

@Wieter thanks, I'll implement and raise a separate PR.