Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
- The database connection array in settings.php can specify an ['init_commands']['sql_mode'] that overrides the default that's provided at the end of
Drupal\Core\Database\Driver\mysql\Connection::open()
. - However,
Drupal\Core\Database\Driver\mysql\Connection::$identifierQuotes
is hard-coded to double quotes. - As a result, if the connection info array overrides the sql_mode to not include ANSI or ANSI_QUOTES, queries throw exceptions, because MySQL doesn't accept a double quote as an identifier quote.
Steps to reproduce
Proposed resolution
- Add a constructor to
Drupal\Core\Database\Driver\mysql\Connection
to fall back to the non-ANSI backtick if the SQL mode doesn't include ANSI quotes.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#25 | sql_mode-3218978-25.patch | 927 bytes | teodyseguin |
#17 | interdiff.txt | 892 bytes | effulgentsia |
#17 | sql_mode-17.patch | 3.37 KB | effulgentsia |
Comments
Comment #2
effulgentsia CreditAttribution: effulgentsia at Acquia commentedComment #4
mcdruidThis is a great fallback / safety net.
However, should we consider simply changing the identifierQuotes to backticks by default? Perhaps there would need to be a deprecation dance (although not sure what that would look like in this case).
We went with backticks in D7 - having originally intended to backport the double quotes implementation - in order to avoid the risk of contrib or custom code conflicting with the ANSI_QUOTES requirement or similar (e.g. #3172388: backup_migrate resets sql_mode causing problems with D7's MySQL 8 support).
Comment #5
effulgentsia CreditAttribution: effulgentsia at Acquia commentedGood question. Asked in #2966523-78: MySQL 8 Support.
Comment #6
effulgentsia CreditAttribution: effulgentsia at Acquia commentedMeanwhile, I'm curious what else breaks without our other default sql_mode options. Here's a patch to find that out for all of our kernel and functional tests.
Comment #8
effulgentsia CreditAttribution: effulgentsia at Acquia commentedOnly 18 failures in #6, that's fewer than I expected. I'll open separate issues for them.
In the meantime, back to the scope of this issue. This goes back to the patch in #2, but removes the call to
str_contains()
, since that's a PHP 8 only function.This does not address #4 yet, since I'd like others to weigh in on that.
Comment #9
mcdruidLooks like the double quotes are very hard-coded in D8:
https://git.drupalcode.org/project/drupal/-/blob/8.9.16/core/lib/Drupal/...
I don't see any mention of case (in)sensitivity in the MySQL docs, but a quick experiment suggests that the sql_mode value is not case sensitive.
Perhaps we should cater for the (perhaps somewhat unlikely) scenario of a custom sql_mode not being all uppercase?
Comment #10
Wim LeersSeems like this problem is even worse when database or tables names consists solely of digits:
— https://dev.mysql.com/doc/refman/8.0/en/identifiers.html
IOW: On MySQL, you should always quote identifiers, either using
"
whenANSI_QUOTES
is on, otherwise using`
.That makes this at least major IMHO, bordering on critical. Any site that turns off
ANSI_QUOTES
which uses a fully numerical database name will get a SQL error, and fail to work.Worse, counter to the docs, I can confirm that even database names that just start with a digit reproduce this behavior:
⇒
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 '101.foo f ON f.id = u.uid' 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 '73829e0c4090444f85fbae2e92f8925a.foo f ON f.id = u.uid' at line 1
e
(the letter "e") in the database name, which makes MySQL interpret it as a number in exponential notation! 🤪🤯🤯🤯🤯🤯🤯🤯🤯🤯🤯 Check yourself:⇒
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 '1e0.foo f ON f.id = u.uid' at line 1
… yet
1a0
works fine!To reproduce:
foo
table like so in each:⚠️ Databases named using only digits (unlikely) or named with a hash (kinda likely) that happen to contain the letter "e" (unlikely) will fail hard! ⚠️
Comment #11
daffie CreditAttribution: daffie commentedThe test for 'ANSI' only works because there are only 2 sql_modes that have the text 'ANSI' in it and they are the 2 we are testing for. Maybe we should add a comment for that.
Can we change this code to:
As I would very much like to test that backticks are being used for MySQL.
Comment #12
effulgentsia CreditAttribution: effulgentsia at Acquia commentedComment #13
effulgentsia CreditAttribution: effulgentsia at Acquia commented#12 addresses #9 and #11.
Comment #14
daffie CreditAttribution: daffie commentedTestbot is failing for PostgreSQL and SQLite. Maybe doing:
$info['default']['init_commands']['sql_mode'] = "SET sql_mode = ''";
only for MySQL.Comment #15
effulgentsia CreditAttribution: effulgentsia at Acquia commentedThis skips the test for non-mysql.
Comment #16
daffie CreditAttribution: daffie commentedThe testbot is still not happy.
Comment #17
effulgentsia CreditAttribution: effulgentsia at Acquia commentedComment #18
daffie CreditAttribution: daffie commentedBugfix with testing.
For me it is RTBC.
Comment #19
Wim LeersNice work! 😊
Comment #22
catchCommitted/pushed to 9.3.x and cherry-picked to 9.2.x, thanks!
Comment #24
teodyseguin CreditAttribution: teodyseguin as a volunteer and commentedI was still able to reproduce this one. After upgrading to 9.2.9 it started to throw an error message
SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '"users_field_data" SET "access"='1638221696' WHERE "uid" = '1'' at line 1: UPDATE "users_field_data" SET "access"=:db_update_placeholder_0 WHERE "uid" = :db_condition_placeholder_0; Array ( [:db_update_placeholder_0] => 1638221696 [:db_condition_placeholder_0] => 1 ) in Drupal\user\UserStorage->updateLastAccessTimestamp()
Site is broken because of this.
I got this while running in a
lando
setup and the version of mysql server is (mysql Ver 14.14 Distrib 5.7.29).Comment #25
teodyseguin CreditAttribution: teodyseguin as a volunteer and commentedComment #26
mcdruid@teodyseguin what do you have your
sql_mode
set to when your site has problems? (It's typically a key/value pair underinit_commands
in the$databases
configuration in settings.php)Comment #27
teodyseguin CreditAttribution: teodyseguin as a volunteer and commentedHi @mcdruid.. I don't have the
init_commands
being set the from thesettings.php