Support enclosing reserved words with brackets.

@Beakerboy thanks for testing Drupal 9 prior to release. It's great to get real world testing of #2986452: Database reserved keywords need to be quoted as per the ANSI standard prior to it landing. Especially real world testing that reveals a flaw in the design where this was supposed to make things much easier for contrib drivers like sqlsrv. I'll work on this this weekend if no-one beats me to it.

Here are two ways to fix this. One is less of an API change than the other. Personally I prefer the -alt patch which changes the protected function \Drupal\Core\Database\Connection::identifierQuote() return value to be an array. This function only exists within 9.0.x and is in a very low level part of the system and is protected.

After sleeping on this I've realised that using a method to contain the identifier quotes is more than a little bit awkward. You should do something functional is that method - so we can make it a property but still keep the deprecation and means no one is wondering about \Drupal\Core\Database\Connection::quoteIdentifiers() vs \Drupal\Core\Database\Connection::identifierQuote()/\Drupal\Core\Database\Connection::identifierQuotes().

More assert() magic for enforcing the contract.

Straight reroll no changes.

Patch looks good. My preference is the alternative solution.
My review for the patch from comment #6.

1. Instead of doing an array of $identifierQuotes, can we do 2 variables: $identifierStartingQuote and $identifierEndingQuote. Less confusion. We do not have to, just an idea.

2. +++ b/core/lib/Drupal/Core/Database/Connection.php
   @@ -413,7 +416,7 @@ public function prefixTables($sql) {
   -    return str_replace(['[', ']'], $this->identifierQuote(), $sql);
   +    return str_replace(['[', ']'], $this->identifierQuotes, $sql);
   

   So for SQL Server, they will be replacing the blockquotes with blockquotes. :)

3. In Drupal\Tests\Core\Database\ConnectionTest can we also add bracket testing in providerEscapeAlias().

What ever solution it will be we shall have to update the CR in https://www.drupal.org/node/3123251.

@daffie good call on the additional test. Personally I prefer a single class property for this.

re $identifierStartingQuote and $identifierEndingQuote I think 1 class property is better in this instance than two. Easier to keep consistent.

I've pondered several times whether we should allow a single element array if the quotes are the same or whether a string of 1 or 2 characters for the same thing.

Seems like we need 9.1.x and 9.0.x patches. Oh yay.

All the code changes look good to me.
Removed the method Drupal\Core\Database\Connection::identifierQuote().
Added the class variable Drupal\Core\Database\Connection::$identifierQuotes.
The change record must be changed for this. The problem is that this can only be done after this issue has landed, because the CR is already published.
For me the patch is RTBC.

Agreed we should just get this in since it's not been in for long in its current state.

Committed/pushed to 9.1.x and 9.0.x, thanks!

edit: CR still needs updating.

Arghh also needs a release notes snippet.

Updated the CR https://www.drupal.org/node/2986894 for this issue.

@daffie the changes to the CR look good.

Added a release note and filled out the issue summary.