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.

Sign up for GitHub

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

Start switching to cakephp/database for schema reflection #794

Merged
merged 4 commits into from
Dec 31, 2024
Merged

Start switching to cakephp/database for schema reflection #794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c897103
Replace table quoting with cakephp/database
markstory Dec 29, 2024
1d2a07c
Replace quoteColumnName with cakephp/database logic
markstory Dec 30, 2024
afa72fb
Use Array cache engine.
markstory Dec 30, 2024
f4af7bf
Fix more failing tests
markstory Dec 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 30 additions & 12 deletions src/Db/Adapter/SqliteAdapter.php
View file
Open in desktop
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
Open in desktop
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
Loading