From 1a992fc8fcc9f3ae1ecce6409824d325b32a9ff5 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 16 May 2013 22:55:00 -0400 Subject: [PATCH 01/15] Add/fix docblocks. --- lib/Cake/Database/Schema/MysqlSchema.php | 3 ++- lib/Cake/Database/Schema/PostgresSchema.php | 9 +++++++++ lib/Cake/Database/Schema/SqliteSchema.php | 20 +++++++++---------- .../Database/Schema/SqliteSchemaTest.php | 2 +- 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/lib/Cake/Database/Schema/MysqlSchema.php b/lib/Cake/Database/Schema/MysqlSchema.php index 0d0c7767fc5..47c73fac5fa 100644 --- a/lib/Cake/Database/Schema/MysqlSchema.php +++ b/lib/Cake/Database/Schema/MysqlSchema.php @@ -20,7 +20,7 @@ use Cake\Error; /** - * Schema dialect/support for MySQL + * Schema management/reflection features for MySQL */ class MysqlSchema { @@ -35,6 +35,7 @@ class MysqlSchema { * Constructor * * @param Cake\Database\Driver $driver The driver to use. + * @return void */ public function __construct($driver) { $this->_driver = $driver; diff --git a/lib/Cake/Database/Schema/PostgresSchema.php b/lib/Cake/Database/Schema/PostgresSchema.php index c88a2d4e23a..6656ecdccd4 100644 --- a/lib/Cake/Database/Schema/PostgresSchema.php +++ b/lib/Cake/Database/Schema/PostgresSchema.php @@ -19,6 +19,9 @@ use Cake\Database\Schema\Table; use Cake\Error; +/** + * Schema management/reflection features for Postgres. + */ class PostgresSchema { /** @@ -28,6 +31,12 @@ class PostgresSchema { */ protected $_driver; +/** + * Constructor + * + * @param Cake\Database\Driver\Postgres $driver Driver to use. + * @return void + */ public function __construct($driver) { $this->_driver = $driver; } diff --git a/lib/Cake/Database/Schema/SqliteSchema.php b/lib/Cake/Database/Schema/SqliteSchema.php index 0a093de3f06..ad1ccdb2ece 100644 --- a/lib/Cake/Database/Schema/SqliteSchema.php +++ b/lib/Cake/Database/Schema/SqliteSchema.php @@ -19,6 +19,9 @@ use Cake\Database\Schema\Table; use Cake\Error; +/** + * Schema management/reflection features for Sqlite + */ class SqliteSchema { /** @@ -28,6 +31,12 @@ class SqliteSchema { */ protected $_driver; +/** + * Constructor + * + * @param Cake\Database\Driver\Sqlite $driver Driver to use. + * @return void + */ public function __construct($driver) { $this->_driver = $driver; } @@ -105,15 +114,6 @@ public function describeTableSql($table) { return ["PRAGMA table_info(" . $this->_driver->quoteIdentifier($table) . ")", []]; } -/** - * Additional metadata columns in table descriptions. - * - * @return array - */ - public function extraSchemaColumns() { - return []; - } - /** * Convert field description results into abstract schema fields. * @@ -197,7 +197,7 @@ public function columnSql(Table $table, $name) { } /** - * Generate the SQL fragment for a single index in MySQL + * Generate the SQL fragment for a single index. * * Note integer primary keys will return ''. This is intentional as Sqlite requires * that integer primary keys be defined in the column definition. diff --git a/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php b/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php index 9a5b82389f5..d781387638b 100644 --- a/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php +++ b/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php @@ -481,7 +481,7 @@ public function testCreateTableSql() { /** * Get a schema instance with a mocked driver/pdo instances * - * @return MysqlSchema + * @return Driver */ protected function _getMockedDriver() { $driver = new \Cake\Database\Driver\Sqlite(); From fd50d6a3fa5e39f133ab3775bbd370fa80a4257e Mon Sep 17 00:00:00 2001 From: Mark Story Date: Fri, 17 May 2013 22:14:20 -0400 Subject: [PATCH 02/15] Get many the SQL generation for many column types working. --- lib/Cake/Database/Schema/PostgresSchema.php | 110 +++++++- .../Database/Schema/PostgresSchemaTest.php | 262 ++++++++++++++++++ 2 files changed, 359 insertions(+), 13 deletions(-) diff --git a/lib/Cake/Database/Schema/PostgresSchema.php b/lib/Cake/Database/Schema/PostgresSchema.php index 6656ecdccd4..57c61700b9e 100644 --- a/lib/Cake/Database/Schema/PostgresSchema.php +++ b/lib/Cake/Database/Schema/PostgresSchema.php @@ -145,19 +145,6 @@ public function convertColumn($column) { return ['type' => 'text', 'length' => null]; } -/** - * Get additional column meta data used in schema reflections. - * - * @return array - */ - public function extraSchemaColumns() { - return [ - 'comment' => [ - 'column' => 'comment', - ] - ]; - } - /** * Convert field description results into abstract schema fields. * @@ -197,4 +184,101 @@ public function convertFieldDescription(Table $table, $row, $fieldParams = []) { } } +/** + * Generate the SQL fragment for a single column. + * + * @param Cake\Database\Schema\Table $table The table object the column is in. + * @param string $name The name of the column. + * @return string SQL fragment. + */ + public function columnSql(Table $table, $name) { + $data = $table->column($name); + $out = $this->_driver->quoteIdentifier($name); + $typeMap = [ + 'biginteger' => ' BIGINT', + 'boolean' => ' BOOLEAN', + 'binary' => ' BYTEA', + 'float' => ' FLOAT', + 'decimal' => ' DECIMAL', + 'text' => ' TEXT', + 'date' => ' DATE', + 'time' => ' TIME', + 'datetime' => ' TIMESTAMP', + 'timestamp' => ' TIMESTAMP', + ]; + + if (isset($typeMap[$data['type']])) { + $out .= $typeMap[$data['type']]; + } + + if ($data['type'] === 'integer') { + $type = ' INTEGER'; + if (in_array($name, (array)$table->primaryKey())) { + $type = ' SERIAL'; + unset($data['null'], $data['default']); + } + $out .= $type; + } + + if ($data['type'] === 'string') { + $isFixed = !empty($data['fixed']); + $type = ' VARCHAR'; + if ($isFixed) { + $type = ' CHAR'; + } + if ($isFixed && isset($data['length']) && $data['length'] == 36) { + $type = ' UUID'; + } + $out .= $type; + if (isset($data['length']) && $data['length'] != 36) { + $out .= '(' . (int)$data['length'] . ')'; + } + } + + if ($data['type'] === 'float' && isset($data['precision'])) { + $out .= '(' . (int)$data['precision'] . ')'; + } + + if ($data['type'] === 'decimal' && + (isset($data['length']) || isset($data['precision'])) + ) { + $out .= '(' . (int)$data['length'] . ',' . (int)$data['precision'] . ')'; + } + + if (isset($data['null']) && $data['null'] === false) { + $out .= ' NOT NULL'; + } + if (isset($data['null']) && $data['null'] === true) { + $out .= ' DEFAULT NULL'; + unset($data['default']); + } + if (isset($data['default']) && $data['type'] !== 'timestamp') { + $out .= ' DEFAULT ' . $this->_driver->schemaValue($data['default']); + } + return $out; + } + +/** + * Generate the SQL fragment for a single index. + * + * @param Cake\Database\Schema\Table $table The table object the column is in. + * @param string $name The name of the column. + * @return string SQL fragment. + */ + public function indexSql(Table $table, $name) { + $data = $table->index($name); + } + +/** + * Generate the SQL to create a table. + * + * @param string $table The name of the table. + * @param array $lines The lines (columns + indexes) to go inside the table. + * @return string A complete CREATE TABLE statement + */ + public function createTableSql($table, $lines) { + $content = implode(",\n", array_filter($lines)); + return sprintf("CREATE TABLE \"%s\" (\n%s\n);", $table, $content); + } + } diff --git a/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php b/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php index d574ef99f14..a64a40445ec 100644 --- a/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php +++ b/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php @@ -20,6 +20,7 @@ use Cake\Database\Connection; use Cake\Database\Schema\Collection as SchemaCollection; use Cake\Database\Schema\PostgresSchema; +use Cake\Database\Schema\Table; use Cake\TestSuite\TestCase; /** @@ -289,4 +290,265 @@ public function testDescribeTable() { $this->assertEquals($definition, $result->column($field)); } } + +/** + * Column provider for creating column sql + * + * @return array + */ + public static function columnSqlProvider() { + return [ + // strings + [ + 'title', + ['type' => 'string', 'length' => 25, 'null' => false], + '"title" VARCHAR(25) NOT NULL' + ], + [ + 'title', + ['type' => 'string', 'length' => 25, 'null' => true, 'default' => 'ignored'], + '"title" VARCHAR(25) DEFAULT NULL' + ], + [ + 'id', + ['type' => 'string', 'length' => 32, 'fixed' => true, 'null' => false], + '"id" CHAR(32) NOT NULL' + ], + [ + 'id', + ['type' => 'string', 'length' => 36, 'fixed' => true, 'null' => false], + '"id" UUID NOT NULL' + ], + [ + 'role', + ['type' => 'string', 'length' => 10, 'null' => false, 'default' => 'admin'], + '"role" VARCHAR(10) NOT NULL DEFAULT "admin"' + ], + [ + 'title', + ['type' => 'string'], + '"title" VARCHAR' + ], + // Text + [ + 'body', + ['type' => 'text', 'null' => false], + '"body" TEXT NOT NULL' + ], + // Integers + [ + 'post_id', + ['type' => 'integer', 'length' => 11], + '"post_id" INTEGER' + ], + [ + 'post_id', + ['type' => 'biginteger', 'length' => 20], + '"post_id" BIGINT' + ], + // Decimal + [ + 'value', + ['type' => 'decimal'], + '"value" DECIMAL' + ], + [ + 'value', + ['type' => 'decimal', 'length' => 11], + '"value" DECIMAL(11,0)' + ], + [ + 'value', + ['type' => 'decimal', 'length' => 12, 'precision' => 5], + '"value" DECIMAL(12,5)' + ], + // Float + [ + 'value', + ['type' => 'float'], + '"value" FLOAT' + ], + [ + 'value', + ['type' => 'float', 'length' => 11, 'precision' => 3], + '"value" FLOAT(3)' + ], + // Binary + [ + 'img', + ['type' => 'binary'], + '"img" BYTEA' + ], + // Boolean + [ + 'checked', + ['type' => 'boolean', 'default' => false], + '"checked" BOOLEAN DEFAULT FALSE' + ], + [ + 'checked', + ['type' => 'boolean', 'default' => true, 'null' => false], + '"checked" BOOLEAN NOT NULL DEFAULT TRUE' + ], + // datetimes + [ + 'created', + ['type' => 'datetime'], + '"created" TIMESTAMP' + ], + // Date & Time + [ + 'start_date', + ['type' => 'date'], + '"start_date" DATE' + ], + [ + 'start_time', + ['type' => 'time'], + '"start_time" TIME' + ], + // timestamps + [ + 'created', + ['type' => 'timestamp', 'null' => true], + '"created" TIMESTAMP DEFAULT NULL' + ], + ]; + } + +/** + * Test generating column definitions + * + * @dataProvider columnSqlProvider + * @return void + */ + public function testColumnSql($name, $data, $expected) { + $driver = $this->_getMockedDriver(); + $schema = new PostgresSchema($driver); + + $table = (new Table('articles'))->addColumn($name, $data); + $this->assertEquals($expected, $schema->columnSql($table, $name)); + } + +/** + * Test generating a column that is a primary key. + * + * @return void + */ + public function testColumnSqlPrimaryKey() { + $driver = $this->_getMockedDriver(); + $schema = new PostgresSchema($driver); + + $table = new Table('articles'); + $table->addColumn('id', [ + 'type' => 'integer', + 'null' => false + ]) + ->addIndex('primary', [ + 'type' => 'primary', + 'columns' => ['id'] + ]); + $result = $schema->columnSql($table, 'id'); + $this->assertEquals($result, '"id" SERIAL'); + } + +/** + * Provide data for testing indexSql + * + * @return array + */ + public static function indexSqlProvider() { + return [ + [ + 'primary', + ['type' => 'primary', 'columns' => ['title']], + 'CONSTRAINT "primary" PRIMARY KEY ("title")' + ], + [ + 'unique_idx', + ['type' => 'unique', 'columns' => ['title', 'author_id']], + 'CONSTRAINT "unique_idx" UNIQUE ("title", "author_id")' + ], + ]; + } + +/** + * Test the indexSql method. + * + * @dataProvider indexSqlProvider + */ + public function testIndexSql($name, $data, $expected) { + $driver = $this->_getMockedDriver(); + $schema = new PostgresSchema($driver); + + $table = (new Table('articles'))->addColumn('title', [ + 'type' => 'string', + 'length' => 255 + ])->addColumn('author_id', [ + 'type' => 'integer', + ])->addIndex($name, $data); + + $this->assertEquals($expected, $schema->indexSql($table, $name)); + } + +/** + * Integration test for converting a Schema\Table into MySQL table creates. + * + * @return void + */ + public function testCreateTableSql() { + $driver = $this->_getMockedDriver(); + $connection = $this->getMock('Cake\Database\Connection', array(), array(), '', false); + $connection->expects($this->any())->method('driver') + ->will($this->returnValue($driver)); + + $table = (new Table('articles'))->addColumn('id', [ + 'type' => 'integer', + 'null' => false + ]) + ->addColumn('title', [ + 'type' => 'string', + 'null' => false, + ]) + ->addColumn('body', ['type' => 'text']) + ->addColumn('created', 'datetime') + ->addIndex('primary', [ + 'type' => 'primary', + 'columns' => ['id'] + ]); + + $expected = <<createTableSql($connection); + $this->assertEquals($expected, $result); + } + +/** + * Get a schema instance with a mocked driver/pdo instances + * + * @return Driver + */ + protected function _getMockedDriver() { + $driver = new \Cake\Database\Driver\Postgres(); + $mock = $this->getMock('FakePdo', ['quote', 'quoteIdentifier']); + $mock->expects($this->any()) + ->method('quote') + ->will($this->returnCallback(function ($value) { + return '"' . $value . '"'; + })); + $mock->expects($this->any()) + ->method('quoteIdentifier') + ->will($this->returnCallback(function ($value) { + return '"' . $value . '"'; + })); + $driver->connection($mock); + return $driver; + } + } From 0ce6be0e57135bf3b8fccec4fba92607171eff3b Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 19 May 2013 09:54:10 -0400 Subject: [PATCH 03/15] Requiring the definition of empty methods is annoying. Use method_exists() to make optional methods optional. --- lib/Cake/Database/Schema/Collection.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/Cake/Database/Schema/Collection.php b/lib/Cake/Database/Schema/Collection.php index 6c31cba5189..34c9a8f21f8 100644 --- a/lib/Cake/Database/Schema/Collection.php +++ b/lib/Cake/Database/Schema/Collection.php @@ -89,7 +89,10 @@ public function describe($name) { } $table = new Table($name); - $fieldParams = $this->_dialect->extraSchemaColumns(); + $fieldParams = []; + if (method_exists($this->_dialect, 'extraSchemaColumn')) { + $fieldParams = $this->_dialect->extraSchemaColumns(); + } foreach ($statement->fetchAll('assoc') as $row) { $this->_dialect->convertFieldDescription($table, $row, $fieldParams); } From e677723fe37f082f0f05fea7c820e1a65ad40256 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 19 May 2013 21:34:55 -0400 Subject: [PATCH 04/15] Implement basic indexes for postgres. Only some indexes can be defined during table creation. Other indexes need to be defined after table creation. This also includes table comments, which will need to be handled separately. --- lib/Cake/Database/Schema/PostgresSchema.php | 16 ++++++++++++++++ .../Database/Schema/PostgresSchemaTest.php | 5 +++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/Cake/Database/Schema/PostgresSchema.php b/lib/Cake/Database/Schema/PostgresSchema.php index 57c61700b9e..443aefea422 100644 --- a/lib/Cake/Database/Schema/PostgresSchema.php +++ b/lib/Cake/Database/Schema/PostgresSchema.php @@ -261,12 +261,28 @@ public function columnSql(Table $table, $name) { /** * Generate the SQL fragment for a single index. * + * Handles creating index types that can be defined in a table create + * statement. Primary keys + unique constraints are handled here. + * Other index types are handled in indexSql(). + * * @param Cake\Database\Schema\Table $table The table object the column is in. * @param string $name The name of the column. * @return string SQL fragment. */ public function indexSql(Table $table, $name) { $data = $table->index($name); + $out = 'CONSTRAINT ' . $this->_driver->quoteIdentifier($name); + if ($data['type'] === Table::INDEX_PRIMARY) { + $out = 'PRIMARY KEY '; + } + if ($data['type'] === Table::INDEX_UNIQUE) { + $out .= ' UNIQUE '; + } + $columns = array_map( + [$this->_driver, 'quoteIdentifier'], + $data['columns'] + ); + return $out . '(' . implode(', ', $columns) . ')'; } /** diff --git a/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php b/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php index a64a40445ec..0861b8fb030 100644 --- a/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php +++ b/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php @@ -462,7 +462,7 @@ public static function indexSqlProvider() { [ 'primary', ['type' => 'primary', 'columns' => ['title']], - 'CONSTRAINT "primary" PRIMARY KEY ("title")' + 'PRIMARY KEY ("title")' ], [ 'unique_idx', @@ -522,7 +522,8 @@ public function testCreateTableSql() { "id" SERIAL, "title" VARCHAR NOT NULL, "body" TEXT, -"created" TIMESTAMP +"created" TIMESTAMP, +PRIMARY KEY ("id") ); SQL; $result = $table->createTableSql($connection); From 0a83f4f2970ac29f1e474a36b316ed48424474f4 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 19 May 2013 21:59:55 -0400 Subject: [PATCH 05/15] Separate index + constraint. Conflating the two makes generating the correct SQL harder as some databases have very different ways of including constraints and indexes. --- lib/Cake/Database/Schema/MysqlSchema.php | 16 ++- lib/Cake/Database/Schema/PostgresSchema.php | 17 ++- lib/Cake/Database/Schema/SqliteSchema.php | 20 +++- lib/Cake/Database/Schema/Table.php | 113 +++++++++++++++--- .../Database/Schema/MysqlSchemaTest.php | 12 +- .../Database/Schema/PostgresSchemaTest.php | 10 +- .../Database/Schema/SqliteSchemaTest.php | 14 +-- .../TestCase/Database/Schema/TableTest.php | 52 ++++++-- 8 files changed, 195 insertions(+), 59 deletions(-) diff --git a/lib/Cake/Database/Schema/MysqlSchema.php b/lib/Cake/Database/Schema/MysqlSchema.php index 47c73fac5fa..23d1f282166 100644 --- a/lib/Cake/Database/Schema/MysqlSchema.php +++ b/lib/Cake/Database/Schema/MysqlSchema.php @@ -256,14 +256,15 @@ public function columnSql(Table $table, $name) { return $out; } + /** - * Generate the SQL fragment for a single index in MySQL + * Generate the SQL fragments for defining table constraints. * * @param Cake\Database\Schema\Table $table The table object the column is in. * @param string $name The name of the column. * @return string SQL fragment. */ - public function indexSql(Table $table, $name) { + public function constraintSql(Table $table, $name) { $data = $table->index($name); if ($data['type'] === Table::INDEX_PRIMARY) { $columns = array_map( @@ -295,4 +296,15 @@ public function indexSql(Table $table, $name) { return $out . ' (' . implode(', ', $columns) . ')'; } +/** + * Generate the SQL fragment for a single index in MySQL + * + * @param Cake\Database\Schema\Table $table The table object the column is in. + * @param string $name The name of the column. + * @return string SQL fragment. + */ + public function indexSql(Table $table, $name) { + return ''; + } + } diff --git a/lib/Cake/Database/Schema/PostgresSchema.php b/lib/Cake/Database/Schema/PostgresSchema.php index 443aefea422..e20cf024a17 100644 --- a/lib/Cake/Database/Schema/PostgresSchema.php +++ b/lib/Cake/Database/Schema/PostgresSchema.php @@ -259,17 +259,24 @@ public function columnSql(Table $table, $name) { } /** - * Generate the SQL fragment for a single index. - * - * Handles creating index types that can be defined in a table create - * statement. Primary keys + unique constraints are handled here. - * Other index types are handled in indexSql(). + * Generate the SQL fragment for a single index * * @param Cake\Database\Schema\Table $table The table object the column is in. * @param string $name The name of the column. * @return string SQL fragment. */ public function indexSql(Table $table, $name) { + return ''; + } + +/** + * Generate the SQL fragment for a single constraint + * + * @param Cake\Database\Schema\Table $table The table object the column is in. + * @param string $name The name of the column. + * @return string SQL fragment. + */ + public function constraintSql(Table $table, $name) { $data = $table->index($name); $out = 'CONSTRAINT ' . $this->_driver->quoteIdentifier($name); if ($data['type'] === Table::INDEX_PRIMARY) { diff --git a/lib/Cake/Database/Schema/SqliteSchema.php b/lib/Cake/Database/Schema/SqliteSchema.php index ad1ccdb2ece..197c2320afd 100644 --- a/lib/Cake/Database/Schema/SqliteSchema.php +++ b/lib/Cake/Database/Schema/SqliteSchema.php @@ -197,7 +197,7 @@ public function columnSql(Table $table, $name) { } /** - * Generate the SQL fragment for a single index. + * Generate the SQL fragments for defining table constraints. * * Note integer primary keys will return ''. This is intentional as Sqlite requires * that integer primary keys be defined in the column definition. @@ -206,18 +206,15 @@ public function columnSql(Table $table, $name) { * @param string $name The name of the column. * @return string SQL fragment. */ - public function indexSql(Table $table, $name) { + public function constraintSql(Table $table, $name) { $data = $table->index($name); - if ( count($data['columns']) === 1 && $table->column($data['columns'][0])['type'] === 'integer' ) { return ''; } - - $out = 'CONSTRAINT '; - $out .= $this->_driver->quoteIdentifier($name); + $out = 'CONSTRAINT ' . $this->_driver->quoteIdentifier($name); if ($data['type'] === Table::INDEX_PRIMARY) { $out .= ' PRIMARY KEY'; } @@ -231,6 +228,17 @@ public function indexSql(Table $table, $name) { return $out . ' (' . implode(', ', $columns) . ')'; } +/** + * Generate the SQL fragment for a single index. + * + * @param Cake\Database\Schema\Table $table The table object the column is in. + * @param string $name The name of the column. + * @return string SQL fragment. + */ + public function indexSql(Table $table, $name) { + return ''; + } + /** * Generate the SQL to create a table. * diff --git a/lib/Cake/Database/Schema/Table.php b/lib/Cake/Database/Schema/Table.php index 975d5e4872c..d7533020307 100644 --- a/lib/Cake/Database/Schema/Table.php +++ b/lib/Cake/Database/Schema/Table.php @@ -46,12 +46,19 @@ class Table { protected $_columns = []; /** - * Indexes + Keys in the table. + * Indexes in the table. * * @var array */ protected $_indexes = []; +/** + * Constraints in the table. + * + * @var array + */ + protected $_constraints = []; + /** * The valid keys that can be used in a column * definition. @@ -86,18 +93,27 @@ class Table { * @var array */ protected $_validIndexTypes = [ - self::INDEX_PRIMARY, self::INDEX_INDEX, - self::INDEX_UNIQUE, - self::INDEX_FOREIGN, self::INDEX_FULLTEXT, ]; - const INDEX_PRIMARY = 'primary'; +/** + * Names of the valid constraint types. + * + * @var array + */ + protected $_validConstraintTypes = [ + self::CONSTRAINT_PRIMARY, + self::CONSTRAINT_UNIQUE, + self::CONSTRAINT_FOREIGN, + ]; + + const CONSTRAINT_PRIMARY = 'primary'; + const CONSTRAINT_UNIQUE = 'unique'; + const CONSTRAINT_FOREIGN = 'foreign'; + const INDEX_INDEX = 'index'; - const INDEX_UNIQUE = 'unique'; const INDEX_FULLTEXT = 'fulltext'; - const INDEX_FOREIGN = 'foreign'; /** * Constructor. @@ -170,17 +186,16 @@ public function column($name) { } /** - * Add an index or key. + * Add an index. * - * Used to add primary keys, indexes, and foreign keys - * to a table. + * Used to add indexes, and full text indexes in platforms that support + * them. * * ### Attributes * * - `type` The type of index being added. * - `columns` The columns in the index. * - * @TODO implement foreign keys. * @param string $name The name of the index. * @param array $attrs The attributes for the index. * @return Table $this @@ -194,7 +209,7 @@ public function addIndex($name, $attrs) { $attrs = $attrs + $this->_indexKeys; if (!in_array($attrs['type'], $this->_validIndexTypes, true)) { - throw new Error\Exception(__d('cake_dev', 'Invalid index type')); + throw new Error\Exception(__d('cake_dev', 'Invalid index type "%s"', $attrs['type'])); } foreach ($attrs['columns'] as $field) { if (empty($this->_columns[$field])) { @@ -234,14 +249,72 @@ public function index($name) { * Null will be returned if a table has no primary key. */ public function primaryKey() { - foreach ($this->_indexes as $name => $data) { - if ($data['type'] === self::INDEX_PRIMARY) { + foreach ($this->_constraints as $name => $data) { + if ($data['type'] === self::CONSTRAINT_PRIMARY) { return $data['columns']; } } return null; } +/** + * Add a constraint. + * + * Used to add constraints to a table. For example primary keys, unique + * keys and foriegn keys. + * + * ### Attributes + * + * - `type` The type of constraint being added. + * - `columns` The columns in the index. + * + * @TODO implement foreign keys. + * @param string $name The name of the constraint. + * @param array $attrs The attributes for the constraint. + * @return Table $this + * @throws Cake\Error\Exception + */ + public function addConstraint($name, $attrs) { + if (is_string($attrs)) { + $attrs = ['type' => $attrs]; + } + $attrs = array_intersect_key($attrs, $this->_indexKeys); + $attrs = $attrs + $this->_indexKeys; + + if (!in_array($attrs['type'], $this->_validConstraintTypes, true)) { + throw new Error\Exception(__d('cake_dev', 'Invalid constraint type "%s"', $attrs['type'])); + } + foreach ($attrs['columns'] as $field) { + if (empty($this->_columns[$field])) { + throw new Error\Exception(__d('cake_dev', 'Columns used in constraints must already exist.')); + } + } + $this->_constraints[$name] = $attrs; + return $this; + } + +/** + * Get the names of all the constraints in the table. + * + * @return array + */ + public function constraints() { + return array_keys($this->_constraints); + } + +/** + * Read information about an constraint based on name. + * + * @param string $name The name of the constraint. + * @return array|null Array of constraint data, or null + */ + public function constraint($name) { + if (!isset($this->_constraints[$name])) { + return null; + } + return $this->_constraints[$name]; + } + /** * Generate the SQL to create the Table. * @@ -249,18 +322,22 @@ public function primaryKey() { * to generate platform specific SQL. * * @param Connection $connection The connection to generate SQL for - * @return string SQL statement to create the table. + * @return array List of SQL statements to create the table and the + * required indexes. */ public function createTableSql(Connection $connection) { $dialect = $connection->driver()->schemaDialect(); - $lines = []; + $lines = $constraints = $indexes = []; foreach (array_keys($this->_columns) as $name) { $lines[] = $dialect->columnSql($this, $name); } + foreach (array_keys($this->_constraints) as $name) { + $constraints[] = $dialect->constraintSql($this, $name); + } foreach (array_keys($this->_indexes) as $name) { - $lines[] = $dialect->indexSql($this, $name); + $indexes[] = $dialect->indexSql($this, $name); } - return $dialect->createTableSql($this->_table, $lines); + return $dialect->createTableSql($this->_table, $lines, $constraints, $indexes); } } diff --git a/lib/Cake/Test/TestCase/Database/Schema/MysqlSchemaTest.php b/lib/Cake/Test/TestCase/Database/Schema/MysqlSchemaTest.php index 6700ff7f8bc..8b5116fc5c9 100644 --- a/lib/Cake/Test/TestCase/Database/Schema/MysqlSchemaTest.php +++ b/lib/Cake/Test/TestCase/Database/Schema/MysqlSchemaTest.php @@ -398,11 +398,11 @@ public function testColumnSql($name, $data, $expected) { } /** - * Provide data for testing indexSql + * Provide data for testing constraintSql * * @return array */ - public static function indexSqlProvider() { + public static function constraintSqlProvider() { return [ [ 'primary', @@ -437,11 +437,11 @@ public static function indexSqlProvider() { } /** - * Test the indexSql method. + * Test the constraintSql method. * - * @dataProvider indexSqlProvider + * @dataProvider constraintSqlProvider */ - public function testIndexSql($name, $data, $expected) { + public function testConstraintSql($name, $data, $expected) { $driver = $this->_getMockedDriver(); $schema = new MysqlSchema($driver); @@ -452,7 +452,7 @@ public function testIndexSql($name, $data, $expected) { 'type' => 'integer', ])->addIndex($name, $data); - $this->assertEquals($expected, $schema->indexSql($table, $name)); + $this->assertEquals($expected, $schema->constraintSql($table, $name)); } /** diff --git a/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php b/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php index 0861b8fb030..aa1afdbf524 100644 --- a/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php +++ b/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php @@ -453,11 +453,11 @@ public function testColumnSqlPrimaryKey() { } /** - * Provide data for testing indexSql + * Provide data for testing constraintSql * * @return array */ - public static function indexSqlProvider() { + public static function constraintSqlProvider() { return [ [ 'primary', @@ -475,9 +475,9 @@ public static function indexSqlProvider() { /** * Test the indexSql method. * - * @dataProvider indexSqlProvider + * @dataProvider constraintSqlProvider */ - public function testIndexSql($name, $data, $expected) { + public function testConstraintSql($name, $data, $expected) { $driver = $this->_getMockedDriver(); $schema = new PostgresSchema($driver); @@ -488,7 +488,7 @@ public function testIndexSql($name, $data, $expected) { 'type' => 'integer', ])->addIndex($name, $data); - $this->assertEquals($expected, $schema->indexSql($table, $name)); + $this->assertEquals($expected, $schema->constraintSql($table, $name)); } /** diff --git a/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php b/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php index d781387638b..b61e3725319 100644 --- a/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php +++ b/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php @@ -397,16 +397,16 @@ public function testColumnSqlPrimaryKey() { $result = $schema->columnSql($table, 'id'); $this->assertEquals($result, '"id" INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT'); - $result = $schema->indexSql($table, 'primary'); + $result = $schema->constraintSql($table, 'primary'); $this->assertEquals('', $result, 'Integer primary keys are special in sqlite.'); } /** - * Provide data for testing indexSql + * Provide data for testing constraintSql * * @return array */ - public static function indexSqlProvider() { + public static function constraintSqlProvider() { return [ [ 'primary', @@ -422,11 +422,11 @@ public static function indexSqlProvider() { } /** - * Test the indexSql method. + * Test the constraintSql method. * - * @dataProvider indexSqlProvider + * @dataProvider constraintSqlProvider */ - public function testIndexSql($name, $data, $expected) { + public function testConstraintSql($name, $data, $expected) { $driver = $this->_getMockedDriver(); $schema = new SqliteSchema($driver); @@ -437,7 +437,7 @@ public function testIndexSql($name, $data, $expected) { 'type' => 'integer', ])->addIndex($name, $data); - $this->assertEquals($expected, $schema->indexSql($table, $name)); + $this->assertEquals($expected, $schema->constraintSql($table, $name)); } /** diff --git a/lib/Cake/Test/TestCase/Database/Schema/TableTest.php b/lib/Cake/Test/TestCase/Database/Schema/TableTest.php index afe9bb5d74c..3430c86d41e 100644 --- a/lib/Cake/Test/TestCase/Database/Schema/TableTest.php +++ b/lib/Cake/Test/TestCase/Database/Schema/TableTest.php @@ -88,21 +88,53 @@ public function testAddColumnFiltersAttributes() { } /** - * Test adding an index. + * Test adding an constraint. * * @return void */ - public function testAddIndex() { + public function testAddConstraint() { $table = new Table('articles'); $table->addColumn('id', [ 'type' => 'integer' ]); - $result = $table->addIndex('primary', [ + $result = $table->addConstraint('primary', [ 'type' => 'primary', 'columns' => ['id'] ]); $this->assertSame($result, $table); - $this->assertEquals(['primary'], $table->indexes()); + $this->assertEquals(['primary'], $table->constraints()); + } + +/** + * Test that an exception is raised when constraintes + * are added for fields that do not exist. + * + * @expectedException Cake\Error\Exception + * @return void + */ + public function testAddConstraintErrorWhenFieldIsMissing() { + $table = new Table('articles'); + $table->addConstraint('author_idx', [ + 'columns' => ['author_id'] + ]); + } + +/** + * Test adding an index. + * + * @return void + */ + public function testAddIndex() { + $table = new Table('articles'); + $table->addColumn('title', [ + 'type' => 'string' + ]); + $result = $table->addIndex('faster', [ + 'type' => 'index', + 'columns' => ['title'] + ]); + $this->assertSame($result, $table); + $this->assertEquals(['faster'], $table->indexes()); } /** @@ -147,15 +179,15 @@ public function testAddIndexTypes() { ->addColumn('author_id', 'integer'); $table->addIndex('author_idx', [ - 'columns' => ['author_id'], - 'type' => 'unique' - ])->addIndex('primary', [ - 'type' => 'primary', - 'columns' => ['id'] + 'columns' => ['author_id'], + 'type' => 'index' + ])->addIndex('texty', [ + 'type' => 'fulltext', + 'columns' => ['title'] ]); $this->assertEquals( - ['author_idx', 'primary'], + ['author_idx', 'texty'], $table->indexes() ); } From 74ad9002ff94c12d61bac5e9323baf8141c6e4d8 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 19 May 2013 22:13:14 -0400 Subject: [PATCH 06/15] Update MySQL schema to match how things will need to work. createTableSql will now need to get additional parameters to account for how postgres and sqlite need to create indexes + constraints differently. --- lib/Cake/Database/Schema/MysqlSchema.php | 44 +++++++++----- .../Database/Schema/MysqlSchemaTest.php | 60 ++++++++++++++----- 2 files changed, 74 insertions(+), 30 deletions(-) diff --git a/lib/Cake/Database/Schema/MysqlSchema.php b/lib/Cake/Database/Schema/MysqlSchema.php index 23d1f282166..3897539c5cb 100644 --- a/lib/Cake/Database/Schema/MysqlSchema.php +++ b/lib/Cake/Database/Schema/MysqlSchema.php @@ -172,12 +172,14 @@ public function extraSchemaColumns() { * Generate the SQL to create a table. * * @param string $table The name of the table. - * @param array $lines The lines (columns + indexes) to go inside the table. - * @return string A complete CREATE TABLE statement + * @param array $columns The columns to go inside the table. + * @param array $constraints The constraints for the table. + * @param array $indexes The indexes for the table. + * @return array A complete CREATE TABLE statement(s) */ - public function createTableSql($table, $lines) { - $content = implode(",\n", $lines); - return sprintf("CREATE TABLE `%s` (\n%s\n);", $table, $content); + public function createTableSql($table, $columns, $constraints, $indexes) { + $content = implode(",\n", array_merge($columns, $constraints, $indexes)); + return [sprintf("CREATE TABLE `%s` (\n%s\n);", $table, $content)]; } /** @@ -265,23 +267,17 @@ public function columnSql(Table $table, $name) { * @return string SQL fragment. */ public function constraintSql(Table $table, $name) { - $data = $table->index($name); - if ($data['type'] === Table::INDEX_PRIMARY) { + $data = $table->constraint($name); + if ($data['type'] === Table::CONSTRAINT_PRIMARY) { $columns = array_map( [$this->_driver, 'quoteIdentifier'], $data['columns'] ); return sprintf('PRIMARY KEY (%s)', implode(', ', $columns)); } - if ($data['type'] === Table::INDEX_UNIQUE) { + if ($data['type'] === Table::CONSTRAINT_UNIQUE) { $out = 'UNIQUE KEY '; } - if ($data['type'] === Table::INDEX_INDEX) { - $out = 'KEY '; - } - if ($data['type'] === Table::INDEX_FULLTEXT) { - $out = 'FULLTEXT KEY '; - } $out .= $this->_driver->quoteIdentifier($name); $columns = array_map( @@ -304,7 +300,25 @@ public function constraintSql(Table $table, $name) { * @return string SQL fragment. */ public function indexSql(Table $table, $name) { - return ''; + $data = $table->index($name); + if ($data['type'] === Table::INDEX_INDEX) { + $out = 'KEY '; + } + if ($data['type'] === Table::INDEX_FULLTEXT) { + $out = 'FULLTEXT KEY '; + } + $out .= $this->_driver->quoteIdentifier($name); + + $columns = array_map( + [$this->_driver, 'quoteIdentifier'], + $data['columns'] + ); + foreach ($data['columns'] as $i => $column) { + if (isset($data['length'][$column])) { + $columns[$i] .= sprintf('(%d)', $data['length'][$column]); + } + } + return $out . ' (' . implode(', ', $columns) . ')'; } } diff --git a/lib/Cake/Test/TestCase/Database/Schema/MysqlSchemaTest.php b/lib/Cake/Test/TestCase/Database/Schema/MysqlSchemaTest.php index 8b5116fc5c9..dfb0ec4c514 100644 --- a/lib/Cake/Test/TestCase/Database/Schema/MysqlSchemaTest.php +++ b/lib/Cake/Test/TestCase/Database/Schema/MysqlSchemaTest.php @@ -414,16 +414,6 @@ public static function constraintSqlProvider() { ['type' => 'unique', 'columns' => ['title', 'author_id']], 'UNIQUE KEY `unique_idx` (`title`, `author_id`)' ], - [ - 'key_key', - ['type' => 'index', 'columns' => ['author_id']], - 'KEY `key_key` (`author_id`)' - ], - [ - 'full_text', - ['type' => 'fulltext', 'columns' => ['title']], - 'FULLTEXT KEY `full_text` (`title`)' - ], [ 'length_idx', [ @@ -450,11 +440,50 @@ public function testConstraintSql($name, $data, $expected) { 'length' => 255 ])->addColumn('author_id', [ 'type' => 'integer', - ])->addIndex($name, $data); + ])->addConstraint($name, $data); $this->assertEquals($expected, $schema->constraintSql($table, $name)); } +/** + * Test provider for indexSql() + * + * @return array + */ + public static function indexSqlProvider() { + return [ + [ + 'key_key', + ['type' => 'index', 'columns' => ['author_id']], + 'KEY `key_key` (`author_id`)' + ], + [ + 'full_text', + ['type' => 'fulltext', 'columns' => ['title']], + 'FULLTEXT KEY `full_text` (`title`)' + ], + ]; + } + +/** + * Test the indexSql method. + * + * @dataProvider indexSqlProvider + */ + public function testIndexSql($name, $data, $expected) { + $driver = $this->_getMockedDriver(); + $schema = new MysqlSchema($driver); + + $table = (new Table('articles'))->addColumn('title', [ + 'type' => 'string', + 'length' => 255 + ])->addColumn('author_id', [ + 'type' => 'integer', + ])->addIndex($name, $data); + + $this->assertEquals($expected, $schema->indexSql($table, $name)); + } + /** * Test generating a column that is a primary key. * @@ -469,7 +498,7 @@ public function testColumnSqlPrimaryKey() { 'type' => 'integer', 'null' => false ]) - ->addIndex('primary', [ + ->addConstraint('primary', [ 'type' => 'primary', 'columns' => ['id'] ]); @@ -481,7 +510,7 @@ public function testColumnSqlPrimaryKey() { 'type' => 'biginteger', 'null' => false ]) - ->addIndex('primary', [ + ->addConstraint('primary', [ 'type' => 'primary', 'columns' => ['id'] ]); @@ -511,7 +540,7 @@ public function testCreateTableSql() { ]) ->addColumn('body', ['type' => 'text']) ->addColumn('created', 'datetime') - ->addIndex('primary', [ + ->addConstraint('primary', [ 'type' => 'primary', 'columns' => ['id'] ]); @@ -526,7 +555,8 @@ public function testCreateTableSql() { ); SQL; $result = $table->createTableSql($connection); - $this->assertEquals($expected, $result); + $this->assertCount(1, $result); + $this->assertEquals($expected, $result[0]); } /** From d9a6f37686123788adef50d244659c89e2cf75b6 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 19 May 2013 22:15:25 -0400 Subject: [PATCH 07/15] Remove duplicated code. --- lib/Cake/Database/Schema/MysqlSchema.php | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/Cake/Database/Schema/MysqlSchema.php b/lib/Cake/Database/Schema/MysqlSchema.php index 3897539c5cb..cbb574394c2 100644 --- a/lib/Cake/Database/Schema/MysqlSchema.php +++ b/lib/Cake/Database/Schema/MysqlSchema.php @@ -279,17 +279,7 @@ public function constraintSql(Table $table, $name) { $out = 'UNIQUE KEY '; } $out .= $this->_driver->quoteIdentifier($name); - - $columns = array_map( - [$this->_driver, 'quoteIdentifier'], - $data['columns'] - ); - foreach ($data['columns'] as $i => $column) { - if (isset($data['length'][$column])) { - $columns[$i] .= sprintf('(%d)', $data['length'][$column]); - } - } - return $out . ' (' . implode(', ', $columns) . ')'; + return $this->_keySql($out, $data); } /** @@ -308,7 +298,17 @@ public function indexSql(Table $table, $name) { $out = 'FULLTEXT KEY '; } $out .= $this->_driver->quoteIdentifier($name); + return $this->_keySql($out, $data); + } +/** + * Helper method for generating key SQL snippets. + * + * @param string $prefix The key prefix + * @param array $data Key data. + * @return string + */ + protected function _keySql($prefix, $data) { $columns = array_map( [$this->_driver, 'quoteIdentifier'], $data['columns'] @@ -318,7 +318,7 @@ public function indexSql(Table $table, $name) { $columns[$i] .= sprintf('(%d)', $data['length'][$column]); } } - return $out . ' (' . implode(', ', $columns) . ')'; + return $prefix . ' (' . implode(', ', $columns) . ')'; } } From d5ceb6303f87fb0a70af7963845d413a09540859 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 19 May 2013 22:29:42 -0400 Subject: [PATCH 08/15] Add name() to Table class. This allows index creation in sqlite and is a nice accessor to have. --- lib/Cake/Database/Schema/Table.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/Cake/Database/Schema/Table.php b/lib/Cake/Database/Schema/Table.php index d7533020307..c3303d0a36f 100644 --- a/lib/Cake/Database/Schema/Table.php +++ b/lib/Cake/Database/Schema/Table.php @@ -128,6 +128,15 @@ public function __construct($table, $columns = array()) { } } +/** + * Get the name of the table. + * + * @return string + */ + public function name() { + return $this->_table; + } + /** * Add a column to the table. * From b3743405881b9c717c1a0f4f42abbcd02b8c80da Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 19 May 2013 22:30:35 -0400 Subject: [PATCH 09/15] Fix/add index generation to Sqlite. Previously Sqlite could not create standard indexes for optimizing queries. Now it can create basic unsorted indexs like other databases. --- lib/Cake/Database/Schema/SqliteSchema.php | 37 ++++++++++---- .../Database/Schema/SqliteSchemaTest.php | 51 +++++++++++++++++-- 2 files changed, 74 insertions(+), 14 deletions(-) diff --git a/lib/Cake/Database/Schema/SqliteSchema.php b/lib/Cake/Database/Schema/SqliteSchema.php index 197c2320afd..5005639b87b 100644 --- a/lib/Cake/Database/Schema/SqliteSchema.php +++ b/lib/Cake/Database/Schema/SqliteSchema.php @@ -132,8 +132,8 @@ public function convertFieldDescription(Table $table, $row, $fieldParams = []) { } $table->addColumn($row['name'], $field); if ($row['pk'] == true) { - $table->addIndex('primary', [ - 'type' => Table::INDEX_PRIMARY, + $table->addConstraint('primary', [ + 'type' => Table::CONSTRAINT_PRIMARY, 'columns' => [$row['name']] ]); } @@ -207,7 +207,7 @@ public function columnSql(Table $table, $name) { * @return string SQL fragment. */ public function constraintSql(Table $table, $name) { - $data = $table->index($name); + $data = $table->constraint($name); if ( count($data['columns']) === 1 && $table->column($data['columns'][0])['type'] === 'integer' @@ -215,10 +215,10 @@ public function constraintSql(Table $table, $name) { return ''; } $out = 'CONSTRAINT ' . $this->_driver->quoteIdentifier($name); - if ($data['type'] === Table::INDEX_PRIMARY) { + if ($data['type'] === Table::CONSTRAINT_PRIMARY) { $out .= ' PRIMARY KEY'; } - if ($data['type'] === Table::INDEX_UNIQUE) { + if ($data['type'] === Table::CONSTRAINT_UNIQUE) { $out .= ' UNIQUE'; } $columns = array_map( @@ -236,18 +236,35 @@ public function constraintSql(Table $table, $name) { * @return string SQL fragment. */ public function indexSql(Table $table, $name) { - return ''; + $data = $table->index($name); + $columns = array_map( + [$this->_driver, 'quoteIdentifier'], + $data['columns'] + ); + return sprintf('CREATE INDEX %s ON %s (%s)', + $this->_driver->quoteIdentifier($name), + $this->_driver->quoteIdentifier($table->name()), + implode(', ', $columns) + ); } /** * Generate the SQL to create a table. * * @param string $table The name of the table. - * @param array $lines The lines (columns + indexes) to go inside the table. - * @return string A complete CREATE TABLE statement + * @param array $columns The columns to go inside the table. + * @param array $constraints The constraints for the table. + * @param array $indexes The indexes for the table. + * @return array A complete CREATE TABLE statement(s)S */ - public function createTableSql($table, $lines) { + public function createTableSql($table, $columns, $constraints, $indexes) { + $lines = array_merge($columns, $constraints); $content = implode(",\n", array_filter($lines)); - return sprintf("CREATE TABLE \"%s\" (\n%s\n);", $table, $content); + $table = sprintf("CREATE TABLE \"%s\" (\n%s\n);", $table, $content); + $out = [$table]; + foreach ($indexes as $index) { + $out[] = $index . ";"; + } + return $out; } } diff --git a/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php b/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php index b61e3725319..0184db5bafb 100644 --- a/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php +++ b/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php @@ -390,7 +390,7 @@ public function testColumnSqlPrimaryKey() { 'type' => 'integer', 'null' => false ]) - ->addIndex('primary', [ + ->addConstraint('primary', [ 'type' => 'primary', 'columns' => ['id'] ]); @@ -435,11 +435,45 @@ public function testConstraintSql($name, $data, $expected) { 'length' => 255 ])->addColumn('author_id', [ 'type' => 'integer', - ])->addIndex($name, $data); + ])->addConstraint($name, $data); $this->assertEquals($expected, $schema->constraintSql($table, $name)); } +/** + * Provide data for testing indexSql + * + * @return array + */ + public static function indexSqlProvider() { + return [ + [ + 'author_idx', + ['type' => 'index', 'columns' => ['title', 'author_id']], + 'CREATE INDEX "author_idx" ON "articles" ("title", "author_id")' + ], + ]; + } + +/** + * Test the indexSql method. + * + * @dataProvider indexSqlProvider + */ + public function testIndexSql($name, $data, $expected) { + $driver = $this->_getMockedDriver(); + $schema = new SqliteSchema($driver); + + $table = (new Table('articles'))->addColumn('title', [ + 'type' => 'string', + 'length' => 255 + ])->addColumn('author_id', [ + 'type' => 'integer', + ])->addIndex($name, $data); + + $this->assertEquals($expected, $schema->indexSql($table, $name)); + } + /** * Integration test for converting a Schema\Table into MySQL table creates. * @@ -461,9 +495,13 @@ public function testCreateTableSql() { ]) ->addColumn('body', ['type' => 'text']) ->addColumn('created', 'datetime') - ->addIndex('primary', [ + ->addConstraint('primary', [ 'type' => 'primary', 'columns' => ['id'] + ]) + ->addIndex('title_idx', [ + 'type' => 'index', + 'columns' => ['title'] ]); $expected = <<createTableSql($connection); - $this->assertEquals($expected, $result); + $this->assertCount(2, $result); + $this->assertEquals($expected, $result[0]); + $this->assertEquals( + 'CREATE INDEX "title_idx" ON "articles" ("title");', + $result[1] + ); } /** From 989903ae6cf7b799b91d5afe0183efc544998a9a Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 19 May 2013 22:33:00 -0400 Subject: [PATCH 10/15] Make constraint generation simpler to read. --- lib/Cake/Database/Schema/SqliteSchema.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/Cake/Database/Schema/SqliteSchema.php b/lib/Cake/Database/Schema/SqliteSchema.php index 5005639b87b..c42417d2a9b 100644 --- a/lib/Cake/Database/Schema/SqliteSchema.php +++ b/lib/Cake/Database/Schema/SqliteSchema.php @@ -214,18 +214,21 @@ public function constraintSql(Table $table, $name) { ) { return ''; } - $out = 'CONSTRAINT ' . $this->_driver->quoteIdentifier($name); if ($data['type'] === Table::CONSTRAINT_PRIMARY) { - $out .= ' PRIMARY KEY'; + $type = 'PRIMARY KEY'; } if ($data['type'] === Table::CONSTRAINT_UNIQUE) { - $out .= ' UNIQUE'; + $type = 'UNIQUE'; } $columns = array_map( [$this->_driver, 'quoteIdentifier'], $data['columns'] ); - return $out . ' (' . implode(', ', $columns) . ')'; + return sprintf('CONSTRAINT %s %s (%s)', + $this->_driver->quoteIdentifier($name), + $type, + implode(', ', $columns) + ); } /** From 7320ce446dd1a9c3c7e82e647442e722b1eb174e Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 19 May 2013 22:41:34 -0400 Subject: [PATCH 11/15] Add index/constraint support to PostgresSchema. --- lib/Cake/Database/Schema/PostgresSchema.php | 37 ++++++++++++++----- .../Database/Schema/PostgresSchemaTest.php | 22 ++++++++--- 2 files changed, 43 insertions(+), 16 deletions(-) diff --git a/lib/Cake/Database/Schema/PostgresSchema.php b/lib/Cake/Database/Schema/PostgresSchema.php index e20cf024a17..7f583706f2a 100644 --- a/lib/Cake/Database/Schema/PostgresSchema.php +++ b/lib/Cake/Database/Schema/PostgresSchema.php @@ -177,8 +177,8 @@ public function convertFieldDescription(Table $table, $row, $fieldParams = []) { } $table->addColumn($row['name'], $field); if (!empty($row['pk'])) { - $table->addIndex('primary', [ - 'type' => Table::INDEX_PRIMARY, + $table->addConstraint('primary', [ + 'type' => Table::CONSTRAINT_PRIMARY, 'columns' => [$row['name']] ]); } @@ -266,7 +266,16 @@ public function columnSql(Table $table, $name) { * @return string SQL fragment. */ public function indexSql(Table $table, $name) { - return ''; + $data = $table->index($name); + $columns = array_map( + [$this->_driver, 'quoteIdentifier'], + $data['columns'] + ); + return sprintf('CREATE INDEX %s ON %s (%s)', + $this->_driver->quoteIdentifier($name), + $this->_driver->quoteIdentifier($table->name()), + implode(', ', $columns) + ); } /** @@ -277,12 +286,12 @@ public function indexSql(Table $table, $name) { * @return string SQL fragment. */ public function constraintSql(Table $table, $name) { - $data = $table->index($name); + $data = $table->constraint($name); $out = 'CONSTRAINT ' . $this->_driver->quoteIdentifier($name); - if ($data['type'] === Table::INDEX_PRIMARY) { + if ($data['type'] === Table::CONSTRAINT_PRIMARY) { $out = 'PRIMARY KEY '; } - if ($data['type'] === Table::INDEX_UNIQUE) { + if ($data['type'] === Table::CONSTRAINT_UNIQUE) { $out .= ' UNIQUE '; } $columns = array_map( @@ -296,12 +305,20 @@ public function constraintSql(Table $table, $name) { * Generate the SQL to create a table. * * @param string $table The name of the table. - * @param array $lines The lines (columns + indexes) to go inside the table. + * @param array $columns The columns to go inside the table. + * @param array $constraints The constraints for the table. + * @param array $indexes The indexes for the table. * @return string A complete CREATE TABLE statement */ - public function createTableSql($table, $lines) { - $content = implode(",\n", array_filter($lines)); - return sprintf("CREATE TABLE \"%s\" (\n%s\n);", $table, $content); + public function createTableSql($table, $columns, $constraints, $indexes) { + $content = array_merge($columns, $constraints); + $content = implode(",\n", array_filter($content)); + $out = []; + $out[] = sprintf("CREATE TABLE \"%s\" (\n%s\n)", $table, $content); + foreach ($indexes as $index) { + $out[] = $index; + } + return $out; } } diff --git a/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php b/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php index aa1afdbf524..ea895a2bbb7 100644 --- a/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php +++ b/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php @@ -444,7 +444,7 @@ public function testColumnSqlPrimaryKey() { 'type' => 'integer', 'null' => false ]) - ->addIndex('primary', [ + ->addConstraint('primary', [ 'type' => 'primary', 'columns' => ['id'] ]); @@ -486,7 +486,7 @@ public function testConstraintSql($name, $data, $expected) { 'length' => 255 ])->addColumn('author_id', [ 'type' => 'integer', - ])->addIndex($name, $data); + ])->addConstraint($name, $data); $this->assertEquals($expected, $schema->constraintSql($table, $name)); } @@ -512,9 +512,13 @@ public function testCreateTableSql() { ]) ->addColumn('body', ['type' => 'text']) ->addColumn('created', 'datetime') - ->addIndex('primary', [ + ->addConstraint('primary', [ 'type' => 'primary', - 'columns' => ['id'] + 'columns' => ['id'], + ]) + ->addIndex('title_idx', [ + 'type' => 'index', + 'columns' => ['title'], ]); $expected = <<createTableSql($connection); - $this->assertEquals($expected, $result); + + $this->assertCount(2, $result); + $this->assertEquals($expected, $result[0]); + $this->assertEquals( + 'CREATE INDEX "title_idx" ON "atricles" ("title")', + $result[1] + ); } /** From 7aea6200ac05b0bf949ec094a4947086ffbfa3a1 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 19 May 2013 22:47:54 -0400 Subject: [PATCH 12/15] Pass the table instance into createTableSql This will allow postgres to preserve comments. Comments in postgres need to be separate commands. --- lib/Cake/Database/Schema/MysqlSchema.php | 4 ++-- lib/Cake/Database/Schema/SqliteSchema.php | 6 +++--- lib/Cake/Database/Schema/Table.php | 6 +++--- lib/Cake/Test/TestCase/Database/Schema/MysqlSchemaTest.php | 2 +- lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php | 4 ++-- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/Cake/Database/Schema/MysqlSchema.php b/lib/Cake/Database/Schema/MysqlSchema.php index cbb574394c2..eeeff5ce3dc 100644 --- a/lib/Cake/Database/Schema/MysqlSchema.php +++ b/lib/Cake/Database/Schema/MysqlSchema.php @@ -171,7 +171,7 @@ public function extraSchemaColumns() { /** * Generate the SQL to create a table. * - * @param string $table The name of the table. + * @param Table $table Table instance * @param array $columns The columns to go inside the table. * @param array $constraints The constraints for the table. * @param array $indexes The indexes for the table. @@ -179,7 +179,7 @@ public function extraSchemaColumns() { */ public function createTableSql($table, $columns, $constraints, $indexes) { $content = implode(",\n", array_merge($columns, $constraints, $indexes)); - return [sprintf("CREATE TABLE `%s` (\n%s\n);", $table, $content)]; + return [sprintf("CREATE TABLE `%s` (\n%s\n)", $table->name(), $content)]; } /** diff --git a/lib/Cake/Database/Schema/SqliteSchema.php b/lib/Cake/Database/Schema/SqliteSchema.php index c42417d2a9b..154cd8e2756 100644 --- a/lib/Cake/Database/Schema/SqliteSchema.php +++ b/lib/Cake/Database/Schema/SqliteSchema.php @@ -254,7 +254,7 @@ public function indexSql(Table $table, $name) { /** * Generate the SQL to create a table. * - * @param string $table The name of the table. + * @param Table $table Table instance * @param array $columns The columns to go inside the table. * @param array $constraints The constraints for the table. * @param array $indexes The indexes for the table. @@ -263,10 +263,10 @@ public function indexSql(Table $table, $name) { public function createTableSql($table, $columns, $constraints, $indexes) { $lines = array_merge($columns, $constraints); $content = implode(",\n", array_filter($lines)); - $table = sprintf("CREATE TABLE \"%s\" (\n%s\n);", $table, $content); + $table = sprintf("CREATE TABLE \"%s\" (\n%s\n)", $table->name(), $content); $out = [$table]; foreach ($indexes as $index) { - $out[] = $index . ";"; + $out[] = $index; } return $out; } diff --git a/lib/Cake/Database/Schema/Table.php b/lib/Cake/Database/Schema/Table.php index c3303d0a36f..ed0137ab1a1 100644 --- a/lib/Cake/Database/Schema/Table.php +++ b/lib/Cake/Database/Schema/Table.php @@ -336,9 +336,9 @@ public function constraint($name) { */ public function createTableSql(Connection $connection) { $dialect = $connection->driver()->schemaDialect(); - $lines = $constraints = $indexes = []; + $columns = $constraints = $indexes = []; foreach (array_keys($this->_columns) as $name) { - $lines[] = $dialect->columnSql($this, $name); + $columns[] = $dialect->columnSql($this, $name); } foreach (array_keys($this->_constraints) as $name) { $constraints[] = $dialect->constraintSql($this, $name); @@ -346,7 +346,7 @@ public function createTableSql(Connection $connection) { foreach (array_keys($this->_indexes) as $name) { $indexes[] = $dialect->indexSql($this, $name); } - return $dialect->createTableSql($this->_table, $lines, $constraints, $indexes); + return $dialect->createTableSql($this, $columns, $constraints, $indexes); } } diff --git a/lib/Cake/Test/TestCase/Database/Schema/MysqlSchemaTest.php b/lib/Cake/Test/TestCase/Database/Schema/MysqlSchemaTest.php index dfb0ec4c514..3caad4a02ef 100644 --- a/lib/Cake/Test/TestCase/Database/Schema/MysqlSchemaTest.php +++ b/lib/Cake/Test/TestCase/Database/Schema/MysqlSchemaTest.php @@ -552,7 +552,7 @@ public function testCreateTableSql() { `body` TEXT, `created` DATETIME, PRIMARY KEY (`id`) -); +) SQL; $result = $table->createTableSql($connection); $this->assertCount(1, $result); diff --git a/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php b/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php index 0184db5bafb..37d05547632 100644 --- a/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php +++ b/lib/Cake/Test/TestCase/Database/Schema/SqliteSchemaTest.php @@ -510,13 +510,13 @@ public function testCreateTableSql() { "title" VARCHAR NOT NULL, "body" TEXT, "created" DATETIME -); +) SQL; $result = $table->createTableSql($connection); $this->assertCount(2, $result); $this->assertEquals($expected, $result[0]); $this->assertEquals( - 'CREATE INDEX "title_idx" ON "articles" ("title");', + 'CREATE INDEX "title_idx" ON "articles" ("title")', $result[1] ); } From c5e116538d4b605b1dec431cef8f2261914cad5f Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 19 May 2013 22:51:36 -0400 Subject: [PATCH 13/15] Add column comment support to postgres. --- lib/Cake/Database/Schema/PostgresSchema.php | 13 ++++++++++++- .../TestCase/Database/Schema/PostgresSchemaTest.php | 9 +++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/Cake/Database/Schema/PostgresSchema.php b/lib/Cake/Database/Schema/PostgresSchema.php index 7f583706f2a..8cfc3435da9 100644 --- a/lib/Cake/Database/Schema/PostgresSchema.php +++ b/lib/Cake/Database/Schema/PostgresSchema.php @@ -313,11 +313,22 @@ public function constraintSql(Table $table, $name) { public function createTableSql($table, $columns, $constraints, $indexes) { $content = array_merge($columns, $constraints); $content = implode(",\n", array_filter($content)); + $tableName = $this->_driver->quoteIdentifier($table->name()); $out = []; - $out[] = sprintf("CREATE TABLE \"%s\" (\n%s\n)", $table, $content); + $out[] = sprintf("CREATE TABLE %s (\n%s\n)", $tableName, $content); foreach ($indexes as $index) { $out[] = $index; } + foreach ($table->columns() as $column) { + $columnData = $table->column($column); + if (isset($columnData['comment'])) { + $out[] = sprintf('COMMENT ON COLUMN %s.%s IS %s', + $tableName, + $this->_driver->quoteIdentifier($column), + $this->_driver->schemaValue($columnData['comment']) + ); + } + } return $out; } diff --git a/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php b/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php index ea895a2bbb7..13d04c4fd30 100644 --- a/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php +++ b/lib/Cake/Test/TestCase/Database/Schema/PostgresSchemaTest.php @@ -509,6 +509,7 @@ public function testCreateTableSql() { ->addColumn('title', [ 'type' => 'string', 'null' => false, + 'comment' => 'This is the title', ]) ->addColumn('body', ['type' => 'text']) ->addColumn('created', 'datetime') @@ -532,12 +533,16 @@ public function testCreateTableSql() { SQL; $result = $table->createTableSql($connection); - $this->assertCount(2, $result); + $this->assertCount(3, $result); $this->assertEquals($expected, $result[0]); $this->assertEquals( - 'CREATE INDEX "title_idx" ON "atricles" ("title")', + 'CREATE INDEX "title_idx" ON "articles" ("title")', $result[1] ); + $this->assertEquals( + 'COMMENT ON COLUMN "articles"."title" IS "This is the title"', + $result[2] + ); } /** From 9fd3a007655260dc50608011be4fe60a4c850aa6 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 19 May 2013 22:52:32 -0400 Subject: [PATCH 14/15] Fix failing test. --- lib/Cake/Test/TestCase/Database/Schema/TableTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Cake/Test/TestCase/Database/Schema/TableTest.php b/lib/Cake/Test/TestCase/Database/Schema/TableTest.php index 3430c86d41e..5c98997eaeb 100644 --- a/lib/Cake/Test/TestCase/Database/Schema/TableTest.php +++ b/lib/Cake/Test/TestCase/Database/Schema/TableTest.php @@ -202,10 +202,10 @@ public function testPrimaryKey() { $table->addColumn('id', 'integer') ->addColumn('title', 'string') ->addColumn('author_id', 'integer') - ->addIndex('author_idx', [ + ->addConstraint('author_idx', [ 'columns' => ['author_id'], 'type' => 'unique' - ])->addIndex('primary', [ + ])->addConstraint('primary', [ 'type' => 'primary', 'columns' => ['id'] ]); From b6d1d1c7158e62035813bad1515fced87ee9e53f Mon Sep 17 00:00:00 2001 From: Mark Story Date: Mon, 20 May 2013 21:00:19 -0400 Subject: [PATCH 15/15] Update doc blocks. --- lib/Cake/Database/Schema/MysqlSchema.php | 4 ++-- lib/Cake/Database/Schema/PostgresSchema.php | 6 +++--- lib/Cake/Database/Schema/SqliteSchema.php | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/Cake/Database/Schema/MysqlSchema.php b/lib/Cake/Database/Schema/MysqlSchema.php index eeeff5ce3dc..33cb5d9f7bb 100644 --- a/lib/Cake/Database/Schema/MysqlSchema.php +++ b/lib/Cake/Database/Schema/MysqlSchema.php @@ -171,11 +171,11 @@ public function extraSchemaColumns() { /** * Generate the SQL to create a table. * - * @param Table $table Table instance + * @param Cake\Database\Schema\Table $table Table instance * @param array $columns The columns to go inside the table. * @param array $constraints The constraints for the table. * @param array $indexes The indexes for the table. - * @return array A complete CREATE TABLE statement(s) + * @return array Complete CREATE TABLE statement(s) */ public function createTableSql($table, $columns, $constraints, $indexes) { $content = implode(",\n", array_merge($columns, $constraints, $indexes)); diff --git a/lib/Cake/Database/Schema/PostgresSchema.php b/lib/Cake/Database/Schema/PostgresSchema.php index 8cfc3435da9..628191e8fa1 100644 --- a/lib/Cake/Database/Schema/PostgresSchema.php +++ b/lib/Cake/Database/Schema/PostgresSchema.php @@ -304,13 +304,13 @@ public function constraintSql(Table $table, $name) { /** * Generate the SQL to create a table. * - * @param string $table The name of the table. + * @param Cake\Database\Schema\Table $table Table instance. * @param array $columns The columns to go inside the table. * @param array $constraints The constraints for the table. * @param array $indexes The indexes for the table. - * @return string A complete CREATE TABLE statement + * @return string Complete CREATE TABLE statement */ - public function createTableSql($table, $columns, $constraints, $indexes) { + public function createTableSql(Table $table, $columns, $constraints, $indexes) { $content = array_merge($columns, $constraints); $content = implode(",\n", array_filter($content)); $tableName = $this->_driver->quoteIdentifier($table->name()); diff --git a/lib/Cake/Database/Schema/SqliteSchema.php b/lib/Cake/Database/Schema/SqliteSchema.php index 154cd8e2756..b9a8920a41d 100644 --- a/lib/Cake/Database/Schema/SqliteSchema.php +++ b/lib/Cake/Database/Schema/SqliteSchema.php @@ -254,11 +254,11 @@ public function indexSql(Table $table, $name) { /** * Generate the SQL to create a table. * - * @param Table $table Table instance + * @param Table $table Cake\Database\Schema\Table instance * @param array $columns The columns to go inside the table. * @param array $constraints The constraints for the table. * @param array $indexes The indexes for the table. - * @return array A complete CREATE TABLE statement(s)S + * @return array Complete CREATE TABLE statement(s)S */ public function createTableSql($table, $columns, $constraints, $indexes) { $lines = array_merge($columns, $constraints);