Commit 81b4b3a9 authored by catch's avatar catch
Browse files

Issue #3126658 by alexpott, daffie, Beakerboy: Support enclosing reserved words with brackets

parent 824ef2ea
......@@ -265,7 +265,7 @@ protected function getTableIndexes(Connection $connection, $table, &$definition)
*/
protected function getTableCollation(Connection $connection, $table, &$definition) {
// Remove identifier quotes from the table name. See
// \Drupal\Core\Database\Driver\mysql\Connection::identifierQuote().
// \Drupal\Core\Database\Driver\mysql\Connection::$identifierQuotes.
$table = trim($connection->prefixTables('{' . $table . '}'), '"');
$query = $connection->query("SHOW TABLE STATUS WHERE NAME = :table_name", [':table_name' => $table]);
$data = $query->fetchAssoc();
......
......@@ -2,6 +2,7 @@
namespace Drupal\Core\Database;
use Drupal\Component\Assertion\Inspector;
use Drupal\Core\Database\Query\Condition;
/**
......@@ -186,6 +187,17 @@ abstract class Connection {
*/
protected $rootTransactionEndCallbacks = [];
/**
* The identifier quote characters for the database type.
*
* An array containing the start and end identifier quote characters for the
* database type. The ANSI SQL standard identifier quote character is a double
* quotation mark.
*
* @var string[]
*/
protected $identifierQuotes;
/**
* Constructs a Connection object.
*
......@@ -198,6 +210,12 @@ abstract class Connection {
* - Other driver-specific options.
*/
public function __construct(\PDO $connection, array $connection_options) {
if ($this->identifierQuotes === NULL) {
@trigger_error('In drupal:10.0.0 not setting the $identifierQuotes property in the concrete Connection class will result in an RuntimeException. See https://www.drupal.org/node/2986894', E_USER_DEPRECATED);
$this->identifierQuotes = ['', ''];
}
assert(count($this->identifierQuotes) === 2 && Inspector::assertAllStrings($this->identifierQuotes), '\Drupal\Core\Database\Connection::$identifierQuotes must contain 2 string values');
// Initialize and prepare the connection prefix.
$this->setPrefix(isset($connection_options['prefix']) ? $connection_options['prefix'] : '');
......@@ -330,7 +348,7 @@ protected function setPrefix($prefix) {
$this->prefixes = ['default' => $prefix];
}
$identifier_quote = $this->identifierQuote();
[$start_quote, $end_quote] = $this->identifierQuotes;
// Set up variables for use in prefixTables(). Replace table-specific
// prefixes first.
$this->prefixSearch = [];
......@@ -340,8 +358,8 @@ protected function setPrefix($prefix) {
$this->prefixSearch[] = '{' . $key . '}';
// $val can point to another database like 'database.users'. In this
// instance we need to quote the identifiers correctly.
$val = str_replace('.', $identifier_quote . '.' . $identifier_quote, $val);
$this->prefixReplace[] = $identifier_quote . $val . $key . $identifier_quote;
$val = str_replace('.', $end_quote . '.' . $start_quote, $val);
$this->prefixReplace[] = $start_quote . $val . $key . $end_quote;
}
}
// Then replace remaining tables with the default prefix.
......@@ -349,9 +367,9 @@ protected function setPrefix($prefix) {
// $this->prefixes['default'] can point to another database like
// 'other_db.'. In this instance we need to quote the identifiers correctly.
// For example, "other_db"."PREFIX_table_name".
$this->prefixReplace[] = $identifier_quote . str_replace('.', $identifier_quote . '.' . $identifier_quote, $this->prefixes['default']);
$this->prefixReplace[] = $start_quote . str_replace('.', $end_quote . '.' . $start_quote, $this->prefixes['default']);
$this->prefixSearch[] = '}';
$this->prefixReplace[] = $identifier_quote;
$this->prefixReplace[] = $end_quote;
// Set up a map of prefixed => un-prefixed tables.
foreach ($this->prefixes as $table_name => $prefix) {
......@@ -361,20 +379,6 @@ protected function setPrefix($prefix) {
}
}
/**
* Returns the identifier quote character for the database type.
*
* The ANSI SQL standard identifier quote character is a double quotation
* mark.
*
* @return string
* The identifier quote character for the database type.
*/
protected function identifierQuote() {
@trigger_error('In drupal:10.0.0 this method will be abstract and contrib and custom drivers will have to implement it. See https://www.drupal.org/node/2986894', E_USER_DEPRECATED);
return '';
}
/**
* Appends a database prefix to all tables in a query.
*
......@@ -414,7 +418,7 @@ public function prefixTables($sql) {
* This method should only be called by database API code.
*/
public function quoteIdentifiers($sql) {
return str_replace(['[', ']'], $this->identifierQuote(), $sql);
return str_replace(['[', ']'], $this->identifierQuotes, $sql);
}
/**
......@@ -580,7 +584,7 @@ public function makeSequenceName($table, $field) {
$sequence_name = $this->prefixTables('{' . $table . '}_' . $field . '_seq');
// Remove identifier quotes as we are constructing a new name from a
// prefixed and quoted table name.
return str_replace($this->identifierQuote(), '', $sequence_name);
return str_replace($this->identifierQuotes, '', $sequence_name);
}
/**
......@@ -1064,7 +1068,8 @@ public function condition($conjunction) {
*/
public function escapeDatabase($database) {
$database = preg_replace('/[^A-Za-z0-9_]+/', '', $database);
return $this->identifierQuote() . $database . $this->identifierQuote();
[$start_quote, $end_quote] = $this->identifierQuotes;
return $start_quote . $database . $end_quote;
}
/**
......@@ -1107,10 +1112,10 @@ public function escapeTable($table) {
public function escapeField($field) {
if (!isset($this->escapedFields[$field])) {
$escaped = preg_replace('/[^A-Za-z0-9_.]+/', '', $field);
$identifier_quote = $this->identifierQuote();
[$start_quote, $end_quote] = $this->identifierQuotes;
// Sometimes fields have the format table_alias.field. In such cases
// both identifiers should be quoted, for example, "table_alias"."field".
$this->escapedFields[$field] = $identifier_quote . str_replace('.', $identifier_quote . '.' . $identifier_quote, $escaped) . $identifier_quote;
$this->escapedFields[$field] = $start_quote . str_replace('.', $end_quote . '.' . $start_quote, $escaped) . $end_quote;
}
return $this->escapedFields[$field];
}
......@@ -1131,7 +1136,8 @@ public function escapeField($field) {
*/
public function escapeAlias($field) {
if (!isset($this->escapedAliases[$field])) {
$this->escapedAliases[$field] = $this->identifierQuote() . preg_replace('/[^A-Za-z0-9_]+/', '', $field) . $this->identifierQuote();
[$start_quote, $end_quote] = $this->identifierQuotes;
$this->escapedAliases[$field] = $start_quote . preg_replace('/[^A-Za-z0-9_]+/', '', $field) . $end_quote;
}
return $this->escapedAliases[$field];
}
......
......@@ -63,6 +63,11 @@ class Connection extends DatabaseConnection {
*/
const MIN_MAX_ALLOWED_PACKET = 1024;
/**
* {@inheritdoc}
*/
protected $identifierQuotes = ['"', '"'];
/**
* Constructs a Connection object.
*/
......@@ -228,15 +233,6 @@ public function queryTemporary($query, array $args = [], array $options = []) {
return $tablename;
}
/**
* {@inheritdoc}
*/
protected function identifierQuote() {
// The database is using the ANSI option on set up so use ANSI quotes and
// not MySQL's custom backtick quote.
return '"';
}
public function driver() {
return 'mysql';
}
......
......@@ -49,6 +49,11 @@ class Connection extends DatabaseConnection {
'NOT REGEXP' => ['operator' => '!~*'],
];
/**
* {@inheritdoc}
*/
protected $identifierQuotes = ['"', '"'];
/**
* Constructs a connection object.
*/
......@@ -201,13 +206,6 @@ public function queryTemporary($query, array $args = [], array $options = []) {
return $tablename;
}
/**
* {@inheritdoc}
*/
protected function identifierQuote() {
return '"';
}
public function driver() {
return 'pgsql';
}
......
......@@ -55,6 +55,11 @@ class Connection extends DatabaseConnection {
*/
public $tableDropped = FALSE;
/**
* {@inheritdoc}
*/
protected $identifierQuotes = ['"', '"'];
/**
* Constructs a \Drupal\Core\Database\Driver\sqlite\Connection object.
*/
......@@ -375,13 +380,6 @@ public function databaseType() {
return 'sqlite';
}
/**
* {@inheritdoc}
*/
protected function identifierQuote() {
return '"';
}
/**
* Overrides \Drupal\Core\Database\Connection::createDatabase().
*
......
......@@ -5,6 +5,7 @@
use Drupal\Core\Database\Connection;
use Drupal\Core\Database\Query\Condition;
use Drupal\Core\Database\Query\PlaceholderInterface;
use Drupal\Tests\Core\Database\Stub\StubConnection;
use Drupal\Tests\Core\Database\Stub\StubPDO;
use Drupal\Tests\UnitTestCase;
use Prophecy\Argument;
......@@ -195,14 +196,7 @@ class_alias('MockCondition', $mocked_namespace);
$mockPdo = $this->createMock(StubPDO::class);
$connection = $this->getMockBuilder(Connection::class)
->setConstructorArgs([$mockPdo, $options])
->setMethods(['identifierQuote'])
->getMockForAbstractClass();
// @todo In drupal:10.0.0 this function will be abstract and the mock
// builder will automatically create it. This can be
// can be removed at that time.
$connection->method('identifierQuote')->willReturn(NULL);
$connection = new StubConnection($mockPdo, $options);
$condition = $connection->condition('AND');
$this->assertSame('MockCondition', get_class($condition));
}
......
......@@ -78,7 +78,7 @@ public function providerTestPrefixTables() {
'SELECT * FROM test_table',
'test_',
'SELECT * FROM {table}',
'',
['', ''],
],
[
'SELECT * FROM "first_table" JOIN "second"."thingie"',
......@@ -88,6 +88,16 @@ public function providerTestPrefixTables() {
],
'SELECT * FROM {table} JOIN {thingie}',
],
[
'SELECT * FROM [first_table] JOIN [second].[thingie]',
[
'table' => 'first_',
'thingie' => 'second.',
],
'SELECT * FROM {table} JOIN {thingie}',
['[', ']'],
],
];
}
......@@ -96,7 +106,7 @@ public function providerTestPrefixTables() {
*
* @dataProvider providerTestPrefixTables
*/
public function testPrefixTables($expected, $prefix_info, $query, $quote_identifier = '"') {
public function testPrefixTables($expected, $prefix_info, $query, array $quote_identifier = ['"', '"']) {
$mock_pdo = $this->createMock('Drupal\Tests\Core\Database\Stub\StubPDO');
$connection = new StubConnection($mock_pdo, ['prefix' => $prefix_info], $quote_identifier);
$this->assertEquals($expected, $connection->prefixTables($query));
......@@ -275,7 +285,8 @@ public function providerEscapeTables() {
return [
['nocase', 'nocase'],
['camelCase', 'camelCase'],
['backtick', '`backtick`', '`'],
['backtick', '`backtick`', ['`', '`']],
['brackets', '[brackets]', ['[', ']']],
['camelCase', '"camelCase"'],
['camelCase', 'camel/Case'],
// Sometimes, table names are following the pattern database.schema.table.
......@@ -290,7 +301,7 @@ public function providerEscapeTables() {
* @covers ::escapeTable
* @dataProvider providerEscapeTables
*/
public function testEscapeTable($expected, $name, $identifier_quote = '"') {
public function testEscapeTable($expected, $name, array $identifier_quote = ['"', '"']) {
$mock_pdo = $this->createMock(StubPDO::class);
$connection = new StubConnection($mock_pdo, [], $identifier_quote);
......@@ -307,9 +318,10 @@ public function testEscapeTable($expected, $name, $identifier_quote = '"') {
*/
public function providerEscapeAlias() {
return [
['!nocase!', 'nocase', '!'],
['`backtick`', 'backtick', '`'],
['nocase', 'nocase', ''],
['!nocase!', 'nocase', ['!', '!']],
['`backtick`', 'backtick', ['`', '`']],
['nocase', 'nocase', ['', '']],
['[brackets]', 'brackets', ['[', ']']],
['"camelCase"', '"camelCase"'],
['"camelCase"', 'camelCase'],
['"camelCase"', 'camel.Case'],
......@@ -320,7 +332,7 @@ public function providerEscapeAlias() {
* @covers ::escapeAlias
* @dataProvider providerEscapeAlias
*/
public function testEscapeAlias($expected, $name, $identifier_quote = '"') {
public function testEscapeAlias($expected, $name, array $identifier_quote = ['"', '"']) {
$mock_pdo = $this->createMock(StubPDO::class);
$connection = new StubConnection($mock_pdo, [], $identifier_quote);
......@@ -337,15 +349,16 @@ public function testEscapeAlias($expected, $name, $identifier_quote = '"') {
*/
public function providerEscapeFields() {
return [
['/title/', 'title', '/'],
['`backtick`', 'backtick', '`'],
['test.title', 'test.title', ''],
['/title/', 'title', ['/', '/']],
['`backtick`', 'backtick', ['`', '`']],
['test.title', 'test.title', ['', '']],
['"isDefaultRevision"', 'isDefaultRevision'],
['"isDefaultRevision"', '"isDefaultRevision"'],
['"entity_test"."isDefaultRevision"', 'entity_test.isDefaultRevision'],
['"entity_test"."isDefaultRevision"', '"entity_test"."isDefaultRevision"'],
['"entityTest"."isDefaultRevision"', '"entityTest"."isDefaultRevision"'],
['"entityTest"."isDefaultRevision"', 'entityTest.isDefaultRevision'],
['[entityTest].[isDefaultRevision]', 'entityTest.isDefaultRevision', ['[', ']']],
];
}
......@@ -353,7 +366,7 @@ public function providerEscapeFields() {
* @covers ::escapeField
* @dataProvider providerEscapeFields
*/
public function testEscapeField($expected, $name, $identifier_quote = '"') {
public function testEscapeField($expected, $name, array $identifier_quote = ['"', '"']) {
$mock_pdo = $this->createMock(StubPDO::class);
$connection = new StubConnection($mock_pdo, [], $identifier_quote);
......@@ -370,10 +383,11 @@ public function testEscapeField($expected, $name, $identifier_quote = '"') {
*/
public function providerEscapeDatabase() {
return [
['/name/', 'name', '/'],
['`backtick`', 'backtick', '`'],
['testname', 'test.name', ''],
['/name/', 'name', ['/', '/']],
['`backtick`', 'backtick', ['`', '`']],
['testname', 'test.name', ['', '']],
['"name"', 'name'],
['[name]', 'name', ['[', ']']],
];
}
......@@ -381,11 +395,41 @@ public function providerEscapeDatabase() {
* @covers ::escapeDatabase
* @dataProvider providerEscapeDatabase
*/
public function testEscapeDatabase($expected, $name, $identifier_quote = '"') {
public function testEscapeDatabase($expected, $name, array $identifier_quote = ['"', '"']) {
$mock_pdo = $this->createMock(StubPDO::class);
$connection = new StubConnection($mock_pdo, [], $identifier_quote);
$this->assertEquals($expected, $connection->escapeDatabase($name));
}
/**
* @covers ::__construct
* @expectedDeprecation In drupal:10.0.0 not setting the $identifierQuotes property in the concrete Connection class will result in an RuntimeException. See https://www.drupal.org/node/2986894
* @group legacy
*/
public function testIdentifierQuotesDeprecation() {
$mock_pdo = $this->createMock(StubPDO::class);
new StubConnection($mock_pdo, [], NULL);
}
/**
* @covers ::__construct
*/
public function testIdentifierQuotesAssertCount() {
$this->expectException(\AssertionError::class);
$this->expectExceptionMessage('\Drupal\Core\Database\Connection::$identifierQuotes must contain 2 string values');
$mock_pdo = $this->createMock(StubPDO::class);
new StubConnection($mock_pdo, [], ['"']);
}
/**
* @covers ::__construct
*/
public function testIdentifierQuotesAssertString() {
$this->expectException(\AssertionError::class);
$this->expectExceptionMessage('\Drupal\Core\Database\Connection::$identifierQuotes must contain 2 string values');
$mock_pdo = $this->createMock(StubPDO::class);
new StubConnection($mock_pdo, [], [0, '1']);
}
}
......@@ -25,9 +25,6 @@ class OrderByTest extends UnitTestCase {
protected function setUp() {
$connection = $this->getMockBuilder('Drupal\Core\Database\Connection')
->disableOriginalConstructor()
// Prevent deprecation message being triggered by
// Connection::identifierQuote().
->setMethods(['identifierQuote'])
->getMockForAbstractClass();
$this->query = new Select($connection, 'test', NULL);
}
......
......@@ -20,13 +20,6 @@ class StubConnection extends Connection {
*/
public $driver = 'stub';
/**
* The identifier quote character. Can be set in the constructor for testing.
*
* @var string
*/
protected $identifierQuote = '';
/**
* Constructs a Connection object.
*
......@@ -34,21 +27,14 @@ class StubConnection extends Connection {
* An object of the PDO class representing a database connection.
* @param array $connection_options
* An array of options for the connection.
* @param string $identifier_quote
* The identifier quote character. Defaults to an empty string.
* @param string[]|null $identifier_quotes
* The identifier quote characters. Defaults to an empty strings.
*/
public function __construct(\PDO $connection, array $connection_options, $identifier_quote = '') {
$this->identifierQuote = $identifier_quote;
public function __construct(\PDO $connection, array $connection_options, $identifier_quotes = ['', '']) {
$this->identifierQuotes = $identifier_quotes;
parent::__construct($connection, $connection_options);
}
/**
* {@inheritdoc}
*/
protected function identifierQuote() {
return $this->identifierQuote;
}
/**
* {@inheritdoc}
*/
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment