[RTM] Quote reserved words in database queries #1262
Conversation
|
Is there a reason we don't copy the reserved words from Contao 3.5 to 4.4? I mean it should be very little effort to also fix that version? |
|
IIRC we didn’t want to add support for using Contao 4.4 with PHP 5 together with MariaDB 10.2.4+, see contao/installation-bundle#70 (comment) |
| */ | ||
| public static function quoteColumnName($strName) | ||
| { | ||
| return \System::getContainer()->get('database_connection')->getDatabasePlatform()->quoteSingleIdentifier($strName); |
There was a problem hiding this comment.
I wonder if we should just return '`' . $strName . '`' instead? Contao does not work with anything but MySQL and quoteColumnName() is potentially executed often, so we might be able to optimize performance here?
There was a problem hiding this comment.
I think we should definitely use quoteSingleIdentifier() from the doctrine database platform, but maybe we can store the DatabasePlatform object in a static variable so that we don’t call the whole \System::getContainer()->get('database_connection')->getDatabasePlatform() every time?
There was a problem hiding this comment.
We can simply use quoteIdentifier() instead of quoteSingleIdentifier() (see 130fc00). Then we don't have to fix the findInSet() method, either.
|
Thank you @ausi. |
Even though we are now quoting database fields, the changes in MariaDB 10.2 also broke Doctrine. And Doctrine will only fix the issue in version 2.7, which requires PHP 7.1+, so this effectively means that Contao 4 with PHP 5 will never be compatible with MariaDB 10.2.7+. |
|
From the current ER2 outage I have learned that some developers are manually quoting identifiers in their code. I had to add this fix: 541a8ca |
From release announce:
4.4.9 (2017/12/14)
* Fixes several minor PHP 7.2 related issues.
4.4.10 (2017/12/27)
* Fixes a few minor issues including a problem with the comments bundle.
4.4.11 (2017/12/28)
* Reverts the identifier quoting changes.
MySQL 8 and MariaDB 10.2
In MySQL 8.0.2 and MariaDB 10.2.7 rows has been added as reserved
word. Since we are using a field named rows in the tl_layout
table, Contao is no longer compatible with the mentioned database
versions.
To avoid renaming the field in Contao and potentially causing
theme import issues, we have added identifier quoting in version
4.4.10(*1). Unfortunately this led to several unforeseen
problems(*2), so we had to revert the changes in version 4.4.11.
(*1) contao/core-bundle#1262
(*2) contao/core#8813 (comment)
4.4.12 (2018/1/3)
* Optimizes adding pages to the search index and fixes a few minor issues.
Description ----------- | Q | A | -----------------| --- | Fixed issues | Fixes #1262 | Docs PR or issue | - Commits ------- f2cb3f11 Add a 5 seconds timeout to the CAPTCHA widget 481076df Only unset the "required" attribute via JS
This is the port of contao/core#8813.
Also see #1106 and contao/installation-bundle#70