Skip to content

Commit

Permalink
Merge pull request #794 from cakephp/schema-reflection
Browse files Browse the repository at this point in the history 
Start switching to cakephp/database for schema reflection
  • Loading branch information
@markstory
markstory authored Dec 31, 2024
2 parents 81604f9 + f4af7bf commit 730654f
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 122 deletions.
42 changes: 30 additions & 12 deletions src/Db/Adapter/SqliteAdapter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,19 @@ public function rollbackTransaction(): void
*/
public function quoteTableName($tableName): string
{
return str_replace('.', '`.`', $this->quoteColumnName($tableName));
$driver = $this->getConnection()->getDriver();

return $driver->quoteIdentifier($tableName);
}

/**
* @inheritDoc
*/
public function quoteColumnName($columnName): string
{
return '`' . str_replace('`', '``', $columnName) . '`';
$driver = $this->getConnection()->getDriver();

return $driver->quoteIdentifier($columnName);
}

/**
Expand Down Expand Up @@ -737,7 +741,7 @@ protected function getAddColumnInstructions(Table $table, Column $column): Alter
*/
protected function getDeclaringSql(string $tableName): string
{
$rows = $this->fetchAll("SELECT * FROM sqlite_master WHERE `type` = 'table'");
$rows = $this->fetchAll("SELECT * FROM sqlite_master WHERE \"type\" = 'table'");

$sql = '';
foreach ($rows as $table) {
Expand All @@ -753,7 +757,13 @@ protected function getDeclaringSql(string $tableName): string
$columnNamePattern = "\"$columnName\"|`$columnName`|\\[$columnName\\]|$columnName";
$columnNamePattern = "#([\(,]+\\s*)($columnNamePattern)(\\s)#iU";

$sql = preg_replace($columnNamePattern, "$1`{$column['name']}`$3", $sql);
$sql = preg_replace_callback(
$columnNamePattern,
function ($matches) use ($column) {
return $matches[1] . $this->quoteColumnName($column['name']) . $matches[3];
},
$sql
);
}

$tableNamePattern = "\"$tableName\"|`$tableName`|\\[$tableName\\]|$tableName";
Expand All @@ -773,7 +783,7 @@ protected function getDeclaringSql(string $tableName): string
*/
protected function getDeclaringIndexSql(string $tableName, string $indexName): string
{
$rows = $this->fetchAll("SELECT * FROM sqlite_master WHERE `type` = 'index'");
$rows = $this->fetchAll("SELECT * FROM sqlite_master WHERE \"type\" = 'index'");

$sql = '';
foreach ($rows as $table) {
Expand Down Expand Up @@ -810,7 +820,7 @@ protected function bufferIndicesAndTriggers(AlterInstructions $instructions, str
"SELECT *
FROM sqlite_master
WHERE
(`type` = 'index' OR `type` = 'trigger')
(\"type\" = 'index' OR `type` = 'trigger')
AND tbl_name = ?
AND sql IS NOT NULL
",
Expand Down Expand Up @@ -1304,6 +1314,7 @@ protected function getDropColumnInstructions(string $tableName, string $columnNa
*/
protected function getIndexes(string $tableName): array
{
// TODO could we describe the table and look for constraint in the metadata?
$indexes = [];
$schema = $this->getSchemaName($tableName, true)['schema'];
$indexList = $this->getTableInfo($tableName, 'index_list');
Expand All @@ -1329,6 +1340,7 @@ protected function getIndexes(string $tableName): array
*/
protected function resolveIndex(string $tableName, string|array $columns): array
{
// TODO could we describe the table and look for constraint in the metadata?
$columns = array_map('strtolower', (array)$columns);
$indexes = $this->getIndexes($tableName);
$matches = [];
Expand Down Expand Up @@ -1356,6 +1368,7 @@ public function hasIndex(string $tableName, string|array $columns): bool
*/
public function hasIndexByName(string $tableName, string $indexName): bool
{
// TODO could we describe the table and look for constraint in the metadata?
$indexName = strtolower($indexName);
$indexes = $this->getIndexes($tableName);

Expand All @@ -1375,7 +1388,7 @@ protected function getAddIndexInstructions(Table $table, Index $index): AlterIns
{
$indexColumnArray = [];
foreach ((array)$index->getColumns() as $column) {
$indexColumnArray[] = sprintf('`%s` ASC', $column);
$indexColumnArray[] = sprintf('%s ASC', $this->quoteColumnName($column));
}
$indexColumns = implode(',', $indexColumnArray);
$where = (string)$index->getWhere();
Expand Down Expand Up @@ -1472,6 +1485,7 @@ public function hasPrimaryKey(string $tableName, $columns, ?string $constraint =
*/
protected function getPrimaryKey(string $tableName): array
{
// TODO could we describe the table and look for constraint in the metadata?
$primaryKey = [];

$rows = $this->getTableInfo($tableName);
Expand All @@ -1490,6 +1504,7 @@ protected function getPrimaryKey(string $tableName): array
*/
public function hasForeignKey(string $tableName, $columns, ?string $constraint = null): bool
{
// TODO could we describe the table and look for constraint in the metadata?
if ($constraint !== null) {
return preg_match(
"/,?\s*CONSTRAINT\s*" . $this->possiblyQuotedIdentifierRegex($constraint) . '\s*FOREIGN\s+KEY/is',
Expand All @@ -1499,6 +1514,7 @@ public function hasForeignKey(string $tableName, $columns, ?string $constraint =

$columns = array_map('mb_strtolower', (array)$columns);

// TODO could we describe the table and look for constraint in the metadata by columns
foreach ($this->getForeignKeys($tableName) as $key) {
if (array_map('mb_strtolower', $key) === $columns) {
return true;
Expand All @@ -1516,6 +1532,7 @@ public function hasForeignKey(string $tableName, $columns, ?string $constraint =
*/
protected function getForeignKeys(string $tableName): array
{
// TODO could this be describe() on table and map over foreign keys?
$foreignKeys = [];

$rows = $this->getTableInfo($tableName, 'foreign_key_list');
Expand All @@ -1541,7 +1558,9 @@ protected function getAddPrimaryKeyInstructions(Table $table, string $column): A

$tableName = $table->getName();
$instructions->addPostStep(function ($state) use ($column) {
$matchPattern = "/(`$column`)\s+(\w+(\(\d+\))?)\s+((NOT )?NULL)/";
$quotedColumn = preg_quote($column);
$columnPattern = "`{$quotedColumn}`|\"{$quotedColumn}\"|\[{$quotedColumn}\]";
$matchPattern = "/($columnPattern)\s+(\w+(\(\d+\))?)\s+((NOT )?NULL)/";

$sql = $state['createSQL'];

Expand Down Expand Up @@ -1878,16 +1897,15 @@ protected function getIndexSqlDefinition(Table $table, Index $index): string
} else {
$def = 'INDEX';
}
if (is_string($index->getName())) {
$indexName = $index->getName();
} else {
$indexName = $index->getName();
if (!is_string($indexName)) {
$indexName = $table->getName() . '_';
foreach ((array)$index->getColumns() as $column) {
$indexName .= $column . '_';
}
$indexName .= 'index';
}
$def .= ' `' . $indexName . '`';
$def .= ' ' . $this->quoteColumnName($indexName);

return $def;
}
Expand Down
36 changes: 18 additions & 18 deletions tests/TestCase/Db/Adapter/PhinxAdapterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,12 @@ public function testRollbackTransaction()

public function testQuoteTableName()
{
$this->assertEquals('`test_table`', $this->adapter->quoteTableName('test_table'));
$this->assertEquals('"test_table"', $this->adapter->quoteTableName('test_table'));
}

public function testQuoteColumnName()
{
$this->assertEquals('`test_column`', $this->adapter->quoteColumnName('test_column'));
$this->assertEquals('"test_column"', $this->adapter->quoteColumnName('test_column'));
}

public function testCreateTable()
Expand Down Expand Up @@ -296,10 +296,10 @@ public function testCreateTableWithIndexesAndForeignKey()
$this->assertTrue($this->adapter->hasForeignKey('tbl_child', ['master_id']));

$row = $this->adapter->fetchRow(
"SELECT * FROM sqlite_master WHERE `type` = 'table' AND `tbl_name` = 'tbl_child'"
"SELECT * FROM sqlite_master WHERE \"type\" = 'table' AND \"tbl_name\" = 'tbl_child'"
);
$this->assertStringContainsString(
'CONSTRAINT `fk_master_id` FOREIGN KEY (`master_id`) REFERENCES `tbl_master` (`id`) ON DELETE NO ACTION ON UPDATE NO ACTION',
'CONSTRAINT "fk_master_id" FOREIGN KEY ("master_id") REFERENCES "tbl_master" ("id") ON DELETE NO ACTION ON UPDATE NO ACTION',
$row['sql']
);
}
Expand All @@ -323,10 +323,10 @@ public function testCreateTableWithoutAutoIncrementingPrimaryKeyAndWithForeignKe
$this->assertTrue($this->adapter->hasForeignKey('tbl_child', ['master_id']));

$row = $this->adapter->fetchRow(
"SELECT * FROM sqlite_master WHERE `type` = 'table' AND `tbl_name` = 'tbl_child'"
"SELECT * FROM sqlite_master WHERE \"type\" = 'table' AND \"tbl_name\" = 'tbl_child'"
);
$this->assertStringContainsString(
'CONSTRAINT `fk_master_id` FOREIGN KEY (`master_id`) REFERENCES `tbl_master` (`id`) ON DELETE NO ACTION ON UPDATE NO ACTION',
'CONSTRAINT "fk_master_id" FOREIGN KEY ("master_id") REFERENCES "tbl_master" ("id") ON DELETE NO ACTION ON UPDATE NO ACTION',
$row['sql']
);
}
Expand Down Expand Up @@ -1115,7 +1115,7 @@ public function testDumpCreateTable()
->save();

$expectedOutput = <<<'OUTPUT'
CREATE TABLE `table1` (`id` INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, `column1` VARCHAR NOT NULL, `column2` INTEGER NULL, `column3` VARCHAR NULL DEFAULT 'test');
CREATE TABLE "table1" ("id" INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, "column1" VARCHAR NOT NULL, "column2" INTEGER NULL, "column3" VARCHAR NULL DEFAULT 'test');
OUTPUT;
$actualOutput = join("\n", $this->out->messages());
$this->assertStringContainsString($expectedOutput, $actualOutput, 'Passing the --dry-run option does not dump create table query to the output');
Expand Down Expand Up @@ -1147,9 +1147,9 @@ public function testDumpInsert()
]);

$expectedOutput = <<<'OUTPUT'
INSERT INTO `table1` (`string_col`) VALUES ('test data');
INSERT INTO `table1` (`string_col`) VALUES (null);
INSERT INTO `table1` (`int_col`) VALUES (23);
INSERT INTO "table1" ("string_col") VALUES ('test data');
INSERT INTO "table1" ("string_col") VALUES (null);
INSERT INTO "table1" ("int_col") VALUES (23);
OUTPUT;
$actualOutput = join("\n", $this->out->messages());
$actualOutput = preg_replace("/\r\n|\r/", "\n", $actualOutput); // normalize line endings for Windows
Expand Down Expand Up @@ -1186,7 +1186,7 @@ public function testDumpBulkinsert()
]);

$expectedOutput = <<<'OUTPUT'
INSERT INTO `table1` (`string_col`, `int_col`) VALUES ('test_data1', 23), (null, 42);
INSERT INTO "table1" ("string_col", "int_col") VALUES ('test_data1', 23), (null, 42);
OUTPUT;
$actualOutput = join("\n", $this->out->messages());
$this->assertStringContainsString($expectedOutput, $actualOutput, 'Passing the --dry-run option doesn\'t dump the bulkinsert to the output');
Expand Down Expand Up @@ -1216,8 +1216,8 @@ public function testDumpCreateTableAndThenInsert()
])->save();

$expectedOutput = <<<'OUTPUT'
CREATE TABLE `table1` (`column1` VARCHAR NOT NULL, `column2` INTEGER NULL, PRIMARY KEY (`column1`));
INSERT INTO `table1` (`column1`, `column2`) VALUES ('id1', 1);
CREATE TABLE "table1" ("column1" VARCHAR NOT NULL, "column2" INTEGER NULL, PRIMARY KEY ("column1"));
INSERT INTO "table1" ("column1", "column2") VALUES ('id1', 1);
OUTPUT;
$actualOutput = join("\n", $this->out->messages());
$actualOutput = preg_replace("/\r\n|\r/", "\n", $actualOutput); // normalize line endings for Windows
Expand Down Expand Up @@ -1618,7 +1618,7 @@ public function testForeignKeyReferenceCorrectAfterRenameColumn()
$sql = $row['sql'];
}
}
$this->assertStringContainsString("REFERENCES `{$refTable->getName()}` (`id`)", $sql);
$this->assertStringContainsString("REFERENCES \"{$refTable->getName()}\" (\"id\")", $sql);
}

public function testForeignKeyReferenceCorrectAfterChangeColumn()
Expand All @@ -1645,7 +1645,7 @@ public function testForeignKeyReferenceCorrectAfterChangeColumn()
$sql = $row['sql'];
}
}
$this->assertStringContainsString("REFERENCES `{$refTable->getName()}` (`id`)", $sql);
$this->assertStringContainsString("REFERENCES \"{$refTable->getName()}\" (\"id\")", $sql);
}

public function testForeignKeyReferenceCorrectAfterRemoveColumn()
Expand All @@ -1666,13 +1666,13 @@ public function testForeignKeyReferenceCorrectAfterRemoveColumn()
$this->assertFalse($this->adapter->hasTable("tmp_{$refTable->getName()}"));
$this->assertFalse($this->adapter->hasColumn($refTable->getName(), $refTableColumnToRemove));

$rows = $this->adapter->fetchAll('select * from sqlite_master where `type` = \'table\'');
$rows = $this->adapter->fetchAll('select * from sqlite_master where "type" = \'table\'');
foreach ($rows as $row) {
if ($row['tbl_name'] === $table->getName()) {
$sql = $row['sql'];
}
}
$this->assertStringContainsString("REFERENCES `{$refTable->getName()}` (`id`)", $sql);
$this->assertStringContainsString("REFERENCES \"{$refTable->getName()}\" (\"id\")", $sql);
}

public function testForeignKeyReferenceCorrectAfterChangePrimaryKey()
Expand Down Expand Up @@ -1702,6 +1702,6 @@ public function testForeignKeyReferenceCorrectAfterChangePrimaryKey()
$sql = $row['sql'];
}
}
$this->assertStringContainsString("REFERENCES `{$refTable->getName()}` (`id`)", $sql);
$this->assertStringContainsString("REFERENCES \"{$refTable->getName()}\" (\"id\")", $sql);
}
}
Loading

0 comments on commit 730654f

Please sign in to comment.