Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support database name with dots #8664

Merged
merged 12 commits into from
Mar 28, 2024
10 changes: 0 additions & 10 deletions phpstan-baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -8981,11 +8981,6 @@
'count' => 1,
'path' => __DIR__ . '/system/Test/Mock/MockConnection.php',
];
$ignoreErrors[] = [
'message' => '#^Property CodeIgniter\\\\Test\\\\Mock\\\\MockConnection\\:\\:\\$returnValues has no type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/system/Test/Mock/MockConnection.php',
];
$ignoreErrors[] = [
'message' => '#^Return type \\(array\\{code\\: int\\|string\\|null, message\\: string\\|null\\}\\) of method CodeIgniter\\\\Test\\\\Mock\\\\MockConnection\\:\\:error\\(\\) should be covariant with return type \\(array\\<string, int\\|string\\>\\) of method CodeIgniter\\\\Database\\\\ConnectionInterface\\<object\\|resource,object\\|resource\\>\\:\\:error\\(\\)$#',
'count' => 1,
Expand Down Expand Up @@ -12676,11 +12671,6 @@
'count' => 1,
'path' => __DIR__ . '/tests/system/Database/BaseConnectionTest.php',
];
$ignoreErrors[] = [
'message' => '#^Property class@anonymous/tests/system/Database/BaseConnectionTest\\.php\\:121\\:\\:\\$returnValues has no type specified\\.$#',
'count' => 1,
'path' => __DIR__ . '/tests/system/Database/BaseConnectionTest.php',
];
$ignoreErrors[] = [
'message' => '#^Method CodeIgniter\\\\Database\\\\BaseQueryTest\\:\\:provideHighlightQueryKeywords\\(\\) return type has no value type specified in iterable type iterable\\.$#',
'count' => 1,
Expand Down
18 changes: 18 additions & 0 deletions system/Database/BaseConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -1188,6 +1188,24 @@ private function protectDotItem(string $item, string $alias, bool $protectIdenti
return $item . $alias;
}

/**
* Escape the SQL Identifier
*
* This function escapes single identifier.
*
* @param non-empty-string $item
*/
public function escapeIdentifier(string $item): string
{
return $this->escapeChar
. str_replace(
$this->escapeChar,
$this->escapeChar . $this->escapeChar,
$item
)
. $this->escapeChar;
}

/**
* Escape the SQL Identifiers
*
Expand Down
19 changes: 16 additions & 3 deletions system/Database/Forge.php
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,14 @@ public function createDatabase(string $dbName, bool $ifNotExists = false): bool
}

try {
if (! $this->db->query(sprintf($ifNotExists ? $this->createDatabaseIfStr : $this->createDatabaseStr, $dbName, $this->db->charset, $this->db->DBCollat))) {
if (! $this->db->query(
sprintf(
$ifNotExists ? $this->createDatabaseIfStr : $this->createDatabaseStr,
$this->db->escapeIdentifier($dbName),
$this->db->charset,
$this->db->DBCollat
)
)) {
// @codeCoverageIgnoreStart
if ($this->db->DBDebug) {
throw new DatabaseException('Unable to create the specified database.');
Expand Down Expand Up @@ -286,7 +293,9 @@ public function dropDatabase(string $dbName): bool
return false;
}

if (! $this->db->query(sprintf($this->dropDatabaseStr, $dbName))) {
if (! $this->db->query(
sprintf($this->dropDatabaseStr, $this->db->escapeIdentifier($dbName))
)) {
if ($this->db->DBDebug) {
throw new DatabaseException('Unable to drop the specified database.');
}
Expand All @@ -295,7 +304,11 @@ public function dropDatabase(string $dbName): bool
}

if (! empty($this->db->dataCache['db_names'])) {
$key = array_search(strtolower($dbName), array_map('strtolower', $this->db->dataCache['db_names']), true);
$key = array_search(
strtolower($dbName),
array_map('strtolower', $this->db->dataCache['db_names']),
true
);
if ($key !== false) {
unset($this->db->dataCache['db_names'][$key]);
}
Expand Down
2 changes: 1 addition & 1 deletion system/Database/MySQLi/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ public function escapeLikeStringDirect($str)
*/
protected function _listTables(bool $prefixLimit = false, ?string $tableName = null): string
{
$sql = 'SHOW TABLES FROM ' . $this->escapeIdentifiers($this->database);
$sql = 'SHOW TABLES FROM ' . $this->escapeIdentifier($this->database);

if ($tableName !== null) {
return $sql . ' LIKE ' . $this->escape($tableName);
Expand Down
51 changes: 50 additions & 1 deletion system/Database/SQLSRV/Forge.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
namespace CodeIgniter\Database\SQLSRV;

use CodeIgniter\Database\BaseConnection;
use CodeIgniter\Database\Exceptions\DatabaseException;
use CodeIgniter\Database\Forge as BaseForge;
use Throwable;

/**
* Forge for SQLSRV
Expand Down Expand Up @@ -42,7 +44,7 @@ class Forge extends BaseForge
*
* @var string
*/
protected $createDatabaseIfStr = "DECLARE @DBName VARCHAR(255) = '%s'\nDECLARE @SQL VARCHAR(max) = 'IF DB_ID( ''' + @DBName + ''' ) IS NULL CREATE DATABASE ' + @DBName\nEXEC( @SQL )";
protected $createDatabaseIfStr = "DECLARE @DBName VARCHAR(255) = '%s'\nDECLARE @SQL VARCHAR(max) = 'IF DB_ID( ''' + @DBName + ''' ) IS NULL CREATE DATABASE %s'\nEXEC( @SQL )";

/**
* CREATE DATABASE IF statement
Expand Down Expand Up @@ -119,6 +121,53 @@ public function __construct(BaseConnection $db)
$this->dropIndexStr = 'DROP INDEX %s ON ' . $this->db->escapeIdentifiers($this->db->schema) . '.%s';
}

/**
* Create database
*
* @param bool $ifNotExists Whether to add IF NOT EXISTS condition
*
* @throws DatabaseException
*/
public function createDatabase(string $dbName, bool $ifNotExists = false): bool
{
if ($ifNotExists) {
$sql = sprintf(
$this->createDatabaseIfStr,
$dbName,
$this->db->escapeIdentifier($dbName)
);
} else {
$sql = sprintf(
$this->createDatabaseStr,
$this->db->escapeIdentifier($dbName)
);
}

try {
if (! $this->db->query($sql)) {
// @codeCoverageIgnoreStart
if ($this->db->DBDebug) {
throw new DatabaseException('Unable to create the specified database.');
}

return false;
// @codeCoverageIgnoreEnd
}

if (isset($this->db->dataCache['db_names'])) {
$this->db->dataCache['db_names'][] = $dbName;
}

return true;
} catch (Throwable $e) {
if ($this->db->DBDebug) {
throw new DatabaseException('Unable to create the specified database.', 0, $e);
}

return false; // @codeCoverageIgnore
}
}

/**
* CREATE TABLE attributes
*/
Expand Down
5 changes: 4 additions & 1 deletion system/Test/Mock/MockConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
*/
class MockConnection extends BaseConnection
{
/**
* @var array{connect?: mixed, execute?: bool|object}
*/
protected $returnValues = [];

/**
Expand Down Expand Up @@ -84,7 +87,7 @@ public function query(string $sql, $binds = null, bool $setEscapeFlags = true, s
$query->setDuration($startTime);

// resultID is not false, so it must be successful
if ($query->isWriteType()) {
if ($query->isWriteType($sql)) {
return true;
}

Expand Down
50 changes: 50 additions & 0 deletions tests/system/Database/BaseConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -283,4 +283,54 @@ public static function provideProtectIdentifiers(): iterable
],
];
}

/**
* These tests are intended to confirm the current behavior.
*
* @dataProvider provideEscapeIdentifiers
*/
public function testEscapeIdentifiers(string $item, string $expected): void
{
$db = new MockConnection($this->options);

$return = $db->escapeIdentifiers($item);

$this->assertSame($expected, $return);
}

/**
* @return array<string, list<string>>
*/
public static function provideEscapeIdentifiers(): iterable
{
yield from [
// $item, $expected
'simple' => ['test', '"test"'],
'with dots' => ['com.sitedb.web', '"com"."sitedb"."web"'],
];
}

/**
* @dataProvider provideEscapeIdentifier
*/
public function testEscapeIdentifier(string $item, string $expected): void
{
$db = new MockConnection($this->options);

$return = $db->escapeIdentifier($item);

$this->assertSame($expected, $return);
}

/**
* @return array<string, list<string>>
*/
public static function provideEscapeIdentifier(): iterable
{
yield from [
// $item, $expected
'simple' => ['test', '"test"'],
'with dots' => ['com.sitedb.web', '"com.sitedb.web"'],
];
}
}
68 changes: 57 additions & 11 deletions tests/system/Database/Live/ForgeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,40 +54,89 @@
if ($this->db->DBDriver === 'OCI8') {
$this->markTestSkipped('OCI8 does not support create database.');
}

$databaseCreated = $this->forge->createDatabase('test_forge_database');

$this->assertTrue($databaseCreated);
}

public function testCreateDatabaseWithDots(): void

Check warning on line 63 in tests/system/Database/Live/ForgeTest.php

View workflow job for this annotation

GitHub Actions / DatabaseLive (8.2, SQLSRV, 8.0) / tests

Took 0.57s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testCreateDatabaseWithDots
{
if ($this->db->DBDriver === 'OCI8') {
$this->markTestSkipped('OCI8 does not support create database.');
}

$dbName = 'test_com.sitedb.web';

$databaseCreated = $this->forge->createDatabase($dbName);

$this->assertTrue($databaseCreated);

// Checks if tableExists() works.
$config = config(Database::class)->{$this->DBGroup};
$config['database'] = $dbName;
$db = db_connect($config);
$result = $db->tableExists('not_exist');

$this->assertFalse($result);

$db->close();
if ($this->db->DBDriver !== 'SQLite3') {
$this->forge->dropDatabase($dbName);
}
}

public function testCreateDatabaseIfNotExists(): void

Check warning on line 89 in tests/system/Database/Live/ForgeTest.php

View workflow job for this annotation

GitHub Actions / DatabaseLive (8.2, SQLSRV, 8.0) / tests

Took 0.51s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testCreateDatabaseIfNotExists
{
if ($this->db->DBDriver === 'OCI8') {
$this->markTestSkipped('OCI8 does not support create database.');
}

$dbName = 'test_forge_database_exist';

$databaseCreateIfNotExists = $this->forge->createDatabase($dbName, true);

$this->assertTrue($databaseCreateIfNotExists);

if ($this->db->DBDriver !== 'SQLite3') {
$this->forge->dropDatabase($dbName);
}

$this->assertTrue($databaseCreateIfNotExists);
}

public function testCreateDatabaseIfNotExistsWithDb(): void

Check warning on line 106 in tests/system/Database/Live/ForgeTest.php

View workflow job for this annotation

GitHub Actions / DatabaseLive (8.2, SQLSRV, 8.0) / tests

Took 0.52s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testCreateDatabaseIfNotExistsWithDb
{
if ($this->db->DBDriver === 'OCI8') {
$this->markTestSkipped('OCI8 does not support create database.');
}

$dbName = 'test_forge_database_exist';

$this->forge->createDatabase($dbName);
$databaseExists = $this->forge->createDatabase($dbName, true);

$this->assertTrue($databaseExists);

if ($this->db->DBDriver !== 'SQLite3') {
$this->forge->dropDatabase($dbName);
}
}

public function testCreateDatabaseIfNotExistsWithDbWithDots(): void

Check warning on line 124 in tests/system/Database/Live/ForgeTest.php

View workflow job for this annotation

GitHub Actions / DatabaseLive (8.2, SQLSRV, 8.0) / tests

Took 0.51s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testCreateDatabaseIfNotExistsWithDbWithDots
{
if ($this->db->DBDriver === 'OCI8') {
$this->markTestSkipped('OCI8 does not support create database.');
}

$dbName = 'test_forge.database.exist';

$this->forge->createDatabase($dbName);
$databaseExists = $this->forge->createDatabase($dbName, true);

$this->assertTrue($databaseExists);

if ($this->db->DBDriver !== 'SQLite3') {
$this->forge->dropDatabase($dbName);
}
}

public function testDropDatabase(): void
Expand All @@ -106,17 +155,14 @@

public function testCreateDatabaseExceptionNoCreateStatement(): void
{
$this->setPrivateProperty($this->forge, 'createDatabaseStr', false);
if ($this->db->DBDriver !== 'OCI8') {
$this->markTestSkipped($this->db->DBDriver . ' does support drop database.');
}

if ($this->db->DBDriver === 'SQLite3') {
$databaseCreated = $this->forge->createDatabase('test_forge_database');
$this->assertTrue($databaseCreated);
} else {
$this->expectException(DatabaseException::class);
$this->expectExceptionMessage('This feature is not available for the database you are using.');
$this->expectException(DatabaseException::class);
$this->expectExceptionMessage('This feature is not available for the database you are using.');

$this->forge->createDatabase('test_forge_database');
}
$this->forge->createDatabase('test_forge_database');
}

public function testDropDatabaseExceptionNoDropStatement(): void
Expand Down Expand Up @@ -386,7 +432,7 @@
$this->forge->createTable('forge_test_table');
}

public function testRenameTable(): void

Check warning on line 435 in tests/system/Database/Live/ForgeTest.php

View workflow job for this annotation

GitHub Actions / DatabaseLive (8.2, SQLSRV, 8.0) / tests

Took 0.58s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testRenameTable
{
$this->forge->dropTable('forge_test_table_dummy', true);

Expand Down Expand Up @@ -442,7 +488,7 @@
$this->forge->dropTable('', true);
}

public function testForeignKey(): void

Check warning on line 491 in tests/system/Database/Live/ForgeTest.php

View workflow job for this annotation

GitHub Actions / DatabaseLive (8.2, OCI8, 8.0) / tests

Took 3.15s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testForeignKey

Check warning on line 491 in tests/system/Database/Live/ForgeTest.php

View workflow job for this annotation

GitHub Actions / DatabaseLive (8.2, SQLSRV, 8.0) / tests

Took 0.57s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testForeignKey
{
$this->forge->dropTable('forge_test_invoices', true);
$this->forge->dropTable('forge_test_users', true);
Expand Down Expand Up @@ -546,7 +592,7 @@
/**
* @see https://github.com/codeigniter4/CodeIgniter4/issues/4310
*/
public function testCompositeForeignKey(): void

Check warning on line 595 in tests/system/Database/Live/ForgeTest.php

View workflow job for this annotation

GitHub Actions / DatabaseLive (8.2, SQLSRV, 8.0) / tests

Took 0.52s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testCompositeForeignKey
{
$this->forge->dropTable('forge_test_invoices', true);
$this->forge->dropTable('forge_test_users', true);
Expand Down Expand Up @@ -1184,7 +1230,7 @@
$this->forge->dropTable('forge_test_1', true);
}

public function testSetKeyNames(): void

Check warning on line 1233 in tests/system/Database/Live/ForgeTest.php

View workflow job for this annotation

GitHub Actions / DatabaseLive (8.2, OCI8, 8.0) / tests

Took 1.35s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testSetKeyNames
{
$this->forge->dropTable('forge_test_1', true);

Expand Down Expand Up @@ -1532,7 +1578,7 @@
$this->forge->dropTable('forge_test_four', true);
}

public function testDropKey(): void

Check warning on line 1581 in tests/system/Database/Live/ForgeTest.php

View workflow job for this annotation

GitHub Actions / DatabaseLive (8.2, OCI8, 8.0) / tests

Took 1.12s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testDropKey
{
$this->forge->dropTable('key_test_users', true);
$keyName = 'key_test_users_id';
Expand Down Expand Up @@ -1619,7 +1665,7 @@
$this->forge->dropTable('forge_test_users', true);
}

public function testProcessIndexes(): void

Check warning on line 1668 in tests/system/Database/Live/ForgeTest.php

View workflow job for this annotation

GitHub Actions / DatabaseLive (8.2, OCI8, 8.0) / tests

Took 1.72s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testProcessIndexes

Check warning on line 1668 in tests/system/Database/Live/ForgeTest.php

View workflow job for this annotation

GitHub Actions / DatabaseLive (8.2, SQLSRV, 8.0) / tests

Took 0.53s from 0.50s limit to run CodeIgniter\\Database\\Live\\ForgeTest::testProcessIndexes
{
// make sure tables don't exist
$this->forge->dropTable('actions', true);
Expand Down
2 changes: 2 additions & 0 deletions user_guide_src/source/changelogs/v4.5.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ Forge
Others
------

- Added support for database names containing dots (``.``).

Model
=====

Expand Down
3 changes: 2 additions & 1 deletion user_guide_src/source/database/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ Explanation of Values
**password** The password used to connect to the database. (``SQLite3`` does not use this.)
**database** The name of the database you want to connect to.

.. note:: CodeIgniter doesn't support dots (``.``) in the database, table, and column names.
.. note:: CodeIgniter doesn't support dots (``.``) in the table and column names.
Since v4.5.0, database names with dots are supported.
**DBDriver** The database driver name. The case must match the driver name.
You can set a fully qualified classname to use your custom driver.
Supported drivers: ``MySQLi``, ``Postgre``, ``SQLite3``, ``SQLSRV``, and ``OCI8``.
Expand Down
3 changes: 2 additions & 1 deletion user_guide_src/source/database/examples.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ The following page contains example code showing how the database class
is used. For complete details please read the individual pages
describing each function.

.. note:: CodeIgniter doesn't support dots (``.``) in the database, table, and column names.
.. note:: CodeIgniter doesn't support dots (``.``) in the table and column names.
Since v4.5.0, database names with dots are supported.

.. contents::
:local:
Expand Down
3 changes: 2 additions & 1 deletion user_guide_src/source/database/queries.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ Queries
Query Basics
************

.. note:: CodeIgniter doesn't support dots (``.``) in the database, table, and column names.
.. note:: CodeIgniter doesn't support dots (``.``) in the table and column names.
Since v4.5.0, database names with dots are supported.

Regular Queries
===============
Expand Down
Loading
Loading