Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Implement alterSchema for SQLIte #916

Closed
wants to merge 1 commit into from

4 participants

@larryb82

This implementation can be used with CakeDC migrations plugin. It passes
most of the existing tests (both migrations and core) plus some new ones.

The code is kind of hacky but it works. I am happy to refactor /
improve this code if the core team provides some feedback.

@markstory markstory commented on the diff
lib/Cake/Model/Datasource/Database/Sqlite.php
@@ -186,13 +186,32 @@ public function describe($model) {
if ($column['pk'] == 1) {
$fields[$column['name']]['key'] = $this->index['PRI'];
$fields[$column['name']]['null'] = false;
- if (empty($fields[$column['name']]['length'])) {
- $fields[$column['name']]['length'] = 11;
+ }
+ }
+ $result->closeCursor();
+
+ // add index information
+ $indexes = $this->_execute("PRAGMA index_list($table)");
+ if ($indexes instanceof PDOStatement) {
+ foreach ($indexes as $index) {
+ $index_info = $this->_execute('PRAGMA index_info("' . $index['name'] . '")');
+ foreach($index_info as $column) {
+ if ($column['seqno'] == 0) {
+ if ($index['unique']) {
+ if (empty($fields[$column['name']]['key']) || ($fields[$column['name']]['key'] != $this->index['PRI'])) {
@markstory Owner

This is quite the conditional stack. Is there anyway you could reduce the complexity here?

@jippi Collaborator
jippi added a note

flip things around; if something isn't true, make it continue - the nesting is quite strong with this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@markstory markstory commented on the diff
lib/Cake/Model/Datasource/Database/Sqlite.php
@@ -228,7 +247,11 @@ public function update(Model $model, $fields = array(), $values = null, $conditi
* @return boolean SQL TRUNCATE TABLE statement, false if not applicable.
*/
public function truncate($table) {
- $this->_execute('DELETE FROM sqlite_sequence where name=' . $this->startQuote . $this->fullTableName($table, false, false) . $this->endQuote);
+ try {
+ $this->_execute('DELETE FROM sqlite_sequence where name=' . $this->startQuote . $this->fullTableName($table, false, false) . $this->endQuote);
+ } catch (PDOException $e) {
+ // sqlite_squence might not exist
@markstory Owner

Swallowing exceptions is a bad practice, you should at least log an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@markstory markstory commented on the diff
lib/Cake/Model/Datasource/Database/Sqlite.php
((26 lines not shown))
- $table = str_replace('"', '', $table);
- list($dbname, $table) = explode('.', $table);
- $dbname = $this->name($dbname);
+ private function _alterSchema($table, $changes) {
@markstory Owner

Private functions require __. We generally try to avoid private methods in favour of protected ones. Lastly this method is missing a doc block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@markstory markstory commented on the diff
lib/Cake/Model/Datasource/Database/Sqlite.php
((32 lines not shown))
- foreach ($indexes as $name => $value) {
+ // step 1. Recreate tables that cannot be altered
+ if ($this->_mustRecreateTable($changes)) {
@markstory Owner

Inline comments are discouraged. Write more clear code if a comment is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@markstory markstory commented on the diff
lib/Cake/Model/Datasource/Database/Sqlite.php
((68 lines not shown))
- } else {
- $value['column'] = $this->name($value['column']);
+ }
+
+ if (isset($changes['alter']['indexes'])) {
+ foreach($changes['add']['indexes'] as $index => $details) {
+
+ }
+ }
+
+ return empty($out) ? '' : implode("\n", $out) . "\n";
+ }
+
+ private function _mustRecreateTable($changes) {
+ $drop = @array_diff_key($changes['drop'], array('indexes' => 0, 'tableParameters' => 0));
+ $change = @array_diff_key($changes['change'], array('indexes => 0', 'tableParameters' => 0));
@markstory Owner

Do not suppress errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@markstory
Owner

Having an alter table method that drops the table seems exceptionally dangerous and not something I think we should support. Additionally people using SQLite should be aware of the limitations it has around modifying table structures. Trying to hack in more support is only going to lead to errors, and data loss. I don't think this pull request is a good idea.

Lastly as a new feature, it should be targeted towards the 2.3 branch not master.

@markstory markstory commented on the diff
lib/Cake/Model/Datasource/Database/Sqlite.php
((65 lines not shown))
}
- if (is_array($value['column'])) {
- $value['column'] = join(', ', array_map(array(&$this, 'name'), $value['column']));
- } else {
- $value['column'] = $this->name($value['column']);
+ }
+
+ if (isset($changes['alter']['indexes'])) {
+ foreach($changes['add']['indexes'] as $index => $details) {
+
+ }
@markstory Owner

This loop and conditional are empty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@markstory markstory commented on the diff
lib/Cake/Model/Datasource/Database/Sqlite.php
((104 lines not shown))
+ } else {
+ $schema['tables'][$table][$col] = $changes['change'][$col];
+ }
+ }
+ }
+
+ // drop columns
+ if (array_key_exists('drop', $changes)) {
+ // remove column from table
+ $schema['tables'][$table] = array_diff_key($schema['tables'][$table], $changes['drop']);
+
+ // remove any indexes that contain the dropped column
+ if (!empty($schema['tables'][$table]['indexes'])) {
+ foreach ($schema['tables'][$table]['indexes'] as $name => $details) {
+ $index_columns = (is_array($details['column'])? $details['column'] : array($details['column']));
+ $dropped_columns = array_keys($changes['drop']);
@markstory Owner

This code does not follow the coding standards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@markstory markstory commented on the diff
lib/Cake/Model/Datasource/Database/Sqlite.php
@@ -569,4 +812,11 @@ public function nestedTransactionSupported() {
return $this->useNestedTransactions && version_compare($this->getVersion(), '3.6.8', '>=');
}
+ private function _columnsAsString($cols = array()) {
@markstory Owner

This function should be protected, not private.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@xemle

@larryb82 Thank you for providing an alterSchema for sqlite! That are great news and a great feature to support sqlite with migration plugin. I would like to use this feature for my web gallery project phtagr.org

So I'm wondering if you continue on the suggestions of @markstory since some days this pull request is silent.

@larryb82

@xemle I'll post something in a couple days. This pull request may also interest you.

@xemle

@larryb82 I'm looking forward to your further changes!

Regarding the pull request: By now, I did not have migration problems. But good to be aware of :-)

@larryb82 larryb82 Sqlite datasource alter schema
This implemenation can be used with CakeDC migrations plugin.  It passes
all the exising tests plus some new ones.

The code is kind of hacky but it works.  I am happy to refactor /
improve this code if the core team provides some feedback.
e09f2ee
@larryb82

@xemle I didn’t get around to making the changes @markstory recommended, however, the latest code fixes a few bugs that were in the original pull request. I’ve been using it on a new project with a volatile schema and the latest version hasn’t given me any trouble. Sorry, I’ve been busy and it doesn’t look like I’ll have much time before the new year to make those changes.

I’m pretty sure this pull request was closed at some point because the core team doesn’t think this is a good idea. Also, I targeted this pull request against the wrong branch. For these reasons, the latest code can be found at: larryb82@34c1a20

To test the code, I’m using the unit tests from the Migrations plugin. I’ve added a few of my own that expose some bugs in earlier versions of my code. larryb82/migrations@4798548

@xemle

Thanks @larryb82 for the honest update. Probably I will have a look at your code and add some of @markstory suggestions. I will let you know!

It would be awesome for the migration plugin to support sqlite officially! You say that the core team does not like the functionality? I think that the pull request needs some polish to get into the official repro.

@markstory do you agree?

@markstory
Owner

I think the changes need a fair bit of polish, I'm also hesitant to emulate features that SQLite doesn't support, as there is always a chance of doing the emulation incorrectly and destroying data. Additionally outside of pagination, we don't emulate any other database behaviors where they are missing.

@xemle

Just a short update from my side: The priority of this feature went down and I do not have time to get the polish done. Anyway +1 for this feature.

@markstory True, this feature will emulate the alter statement for sqlite. However it seems to be a recommended best practice. So I prefer to have a support with caution instead of none.

@markstory
Owner

My main concern with emulating a these kind of database features is that any mistakes in our implementation could result in permanent data loss. That is true of all the schema migration features we have for all database vendors. This one is more risky as it is far more complex, so the risk of error is much higher.

@larryb82

@markstory, you are correct about the complexity. Since, posting this, I've fixed a few bugs with indexes and I'm sure there more issues that I haven't unearthed yet. That said, would you be more supportive of a separate datasource, say AlterableSqlite, that extends the existing Sqlite and come with a big fat warning. I already plan to do this in my project when I upgrade to the latest 2.x version so I don't have any modifications in the core.

@jippi jippi commented on the diff
lib/Cake/Model/Datasource/Database/Sqlite.php
@@ -186,13 +186,32 @@ public function describe($model) {
if ($column['pk'] == 1) {
$fields[$column['name']]['key'] = $this->index['PRI'];
$fields[$column['name']]['null'] = false;
- if (empty($fields[$column['name']]['length'])) {
- $fields[$column['name']]['length'] = 11;
+ }
+ }
+ $result->closeCursor();
+
+ // add index information
+ $indexes = $this->_execute("PRAGMA index_list($table)");
+ if ($indexes instanceof PDOStatement) {
+ foreach ($indexes as $index) {
+ $index_info = $this->_execute('PRAGMA index_info("' . $index['name'] . '")');
+ foreach($index_info as $column) {
@jippi Collaborator
jippi added a note

foreach ( (love the spaces!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jippi jippi commented on the diff
lib/Cake/Model/Datasource/Database/Sqlite.php
((6 lines not shown))
+ }
+ }
+ $result->closeCursor();
+
+ // add index information
+ $indexes = $this->_execute("PRAGMA index_list($table)");
+ if ($indexes instanceof PDOStatement) {
+ foreach ($indexes as $index) {
+ $index_info = $this->_execute('PRAGMA index_info("' . $index['name'] . '")');
+ foreach($index_info as $column) {
+ if ($column['seqno'] == 0) {
+ if ($index['unique']) {
+ if (empty($fields[$column['name']]['key']) || ($fields[$column['name']]['key'] != $this->index['PRI'])) {
+ $fields[$column['name']]['key'] = $this->index['UNI'];
+ }
+ } else {
@jippi Collaborator
jippi added a note

make it an elseif :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jippi jippi commented on the diff
lib/Cake/Model/Datasource/Database/Sqlite.php
@@ -186,13 +186,32 @@ public function describe($model) {
if ($column['pk'] == 1) {
$fields[$column['name']]['key'] = $this->index['PRI'];
$fields[$column['name']]['null'] = false;
- if (empty($fields[$column['name']]['length'])) {
- $fields[$column['name']]['length'] = 11;
+ }
+ }
+ $result->closeCursor();
+
+ // add index information
+ $indexes = $this->_execute("PRAGMA index_list($table)");
+ if ($indexes instanceof PDOStatement) {
+ foreach ($indexes as $index) {
+ $index_info = $this->_execute('PRAGMA index_info("' . $index['name'] . '")');
+ foreach($index_info as $column) {
+ if ($column['seqno'] == 0) {
@jippi Collaborator
jippi added a note

== or do you really mean === ? :)

if == might as well use empty()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@markstory
Owner

I'm going to close this. There are enough open issues/comments that need to be addressed that a new PR can be made. Also I still really don't like the idea of bolting on schema migrations into a database that intentionally doesn't support them.

@markstory markstory closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 17, 2012
  1. @larryb82

    Sqlite datasource alter schema

    larryb82 authored
    This implemenation can be used with CakeDC migrations plugin.  It passes
    all the exising tests plus some new ones.
    
    The code is kind of hacky but it works.  I am happy to refactor /
    improve this code if the core team provides some feedback.
This page is out of date. Refresh to see the latest.
View
372 lib/Cake/Model/Datasource/Database/Sqlite.php
@@ -186,13 +186,32 @@ public function describe($model) {
if ($column['pk'] == 1) {
$fields[$column['name']]['key'] = $this->index['PRI'];
$fields[$column['name']]['null'] = false;
- if (empty($fields[$column['name']]['length'])) {
- $fields[$column['name']]['length'] = 11;
+ }
+ }
+ $result->closeCursor();
+
+ // add index information
+ $indexes = $this->_execute("PRAGMA index_list($table)");
+ if ($indexes instanceof PDOStatement) {
+ foreach ($indexes as $index) {
+ $index_info = $this->_execute('PRAGMA index_info("' . $index['name'] . '")');
+ foreach($index_info as $column) {
@jippi Collaborator
jippi added a note

foreach ( (love the spaces!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ if ($column['seqno'] == 0) {
@jippi Collaborator
jippi added a note

== or do you really mean === ? :)

if == might as well use empty()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ if ($index['unique']) {
+ if (empty($fields[$column['name']]['key']) || ($fields[$column['name']]['key'] != $this->index['PRI'])) {
@markstory Owner

This is quite the conditional stack. Is there anyway you could reduce the complexity here?

@jippi Collaborator
jippi added a note

flip things around; if something isn't true, make it continue - the nesting is quite strong with this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $fields[$column['name']]['key'] = $this->index['UNI'];
+ }
+ } else {
@jippi Collaborator
jippi added a note

make it an elseif :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ if (empty($fields[$column['name']]['key'])) {
+ $fields[$column['name']]['key'] = $this->index['MUL'];
+ }
+ }
+ }
}
}
+ $indexes->closeCursor();
}
- $result->closeCursor();
$this->_cacheDescription($table, $fields);
return $fields;
}
@@ -228,7 +247,11 @@ public function update(Model $model, $fields = array(), $values = null, $conditi
* @return boolean SQL TRUNCATE TABLE statement, false if not applicable.
*/
public function truncate($table) {
- $this->_execute('DELETE FROM sqlite_sequence where name=' . $this->startQuote . $this->fullTableName($table, false, false) . $this->endQuote);
+ try {
+ $this->_execute('DELETE FROM sqlite_sequence where name=' . $this->startQuote . $this->fullTableName($table, false, false) . $this->endQuote);
+ } catch (PDOException $e) {
+ // sqlite_squence might not exist
@markstory Owner

Swallowing exceptions is a bad practice, you should at least log an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
return $this->execute('DELETE FROM ' . $this->fullTableName($table));
}
@@ -424,43 +447,249 @@ public function getEncoding() {
}
/**
- * Removes redundant primary key indexes, as they are handled in the column def of the key.
+ * Generate a alter syntax from CakeSchema::compare()
*
- * @param array $indexes
+ * See http://www.sqlite.org/faq.html#q11
+ *
+ * @param mixed $compare
* @param string $table
- * @return string
+ * @return boolean
*/
- public function buildIndex($indexes, $table = null) {
- $join = array();
+ public function alterSchema($compare, $table = null) {
+ $out = '';
+ foreach ($compare as $tableName => $changes) {
+ if (!$table || ($tableName == $table)) {
+ $out .= $this->_alterSchema($tableName, $changes) . "\n";
+ }
+ }
+ return $out;
+ }
- $table = str_replace('"', '', $table);
- list($dbname, $table) = explode('.', $table);
- $dbname = $this->name($dbname);
+ private function _alterSchema($table, $changes) {
@markstory Owner

Private functions require __. We generally try to avoid private methods in favour of protected ones. Lastly this method is missing a doc block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $out = array();
- foreach ($indexes as $name => $value) {
+ // step 1. Recreate tables that cannot be altered
+ if ($this->_mustRecreateTable($changes)) {
@markstory Owner

Inline comments are discouraged. Write more clear code if a comment is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $out = array_merge($out, $this->_recreateTable($table, $changes));
+ }
- if ($name == 'PRIMARY') {
- continue;
+ // step 2. Alter tables that can be altered
+ if (array_key_exists('add', $changes)) {
+ $out = array_merge($out, $this->_addColumns($table, $changes['add']));
+ }
+
+ // step 3. Drop indexes (including changed)
+ if (isset($changes['drop']['indexes'])) {
+ foreach($changes['drop']['indexes'] as $index => $details) {
+ $out[] = "DROP INDEX IF EXISTS {$this->fullTableName($index)};";
}
- $out = 'CREATE ';
+ }
+
+ if (isset($changes['add']['indexes'])) {
+ foreach($changes['add']['indexes'] as $index => $details) {
+ if (is_array($details['column'])) {
+ $details['column'] = join(', ', $details['column']);
+ }
- if (!empty($value['unique'])) {
- $out .= 'UNIQUE ';
+ $out[] = ((empty($details['unique']))? 'CREATE INDEX' : 'CREATE UNIQUE INDEX' ) . ' '
+ . $this->fullTableName($index)
+ . ' on "' . $table . '" ( ' . $details['column'] . ' );';
}
- if (is_array($value['column'])) {
- $value['column'] = join(', ', array_map(array(&$this, 'name'), $value['column']));
- } else {
- $value['column'] = $this->name($value['column']);
+ }
+
+ if (isset($changes['alter']['indexes'])) {
+ foreach($changes['add']['indexes'] as $index => $details) {
+
+ }
@markstory Owner

This loop and conditional are empty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
+
+ return empty($out) ? '' : implode("\n", $out) . "\n";
+ }
+
+ private function _mustRecreateTable($changes) {
+ $drop = @array_diff_key($changes['drop'], array('indexes' => 0, 'tableParameters' => 0));
+ $change = @array_diff_key($changes['change'], array('indexes => 0', 'tableParameters' => 0));
@markstory Owner

Do not suppress errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ return !(empty($drop) && empty($change));
+ }
+
+ private function _recreateTable($table, $changes) {
+ $out = array();
+
+ $reader = new CakeSchema();
+ $schema = $reader->read(array(
+ 'connection' => ConnectionManager::getSourceName($this),
+ 'models' => false
+ ));
+
+ // change columns
+ $renamed = array();
+ if (array_key_exists('change', $changes)) {
+ foreach($changes['change'] as $col => $details) {
+ if (!empty($changes['change'][$col]['name'])) {
+ $renamed[$changes['change'][$col]['name']] = $col;
+ unset($schema['tables'][$table][$col]);
+ $schema['tables'][$table][$changes['change'][$col]['name']] = $changes['change'][$col];
+ } else {
+ $schema['tables'][$table][$col] = $changes['change'][$col];
+ }
+ }
+ }
+
+ // drop columns
+ if (array_key_exists('drop', $changes)) {
+ // remove column from table
+ $schema['tables'][$table] = array_diff_key($schema['tables'][$table], $changes['drop']);
+
+ // remove any indexes that contain the dropped column
+ if (!empty($schema['tables'][$table]['indexes'])) {
+ foreach ($schema['tables'][$table]['indexes'] as $name => $details) {
+ $index_columns = (is_array($details['column'])? $details['column'] : array($details['column']));
+ $dropped_columns = array_keys($changes['drop']);
@markstory Owner

This code does not follow the coding standards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $intersection = array_intersect($index_columns, $dropped_columns);
+ if (!empty($intersection)) {
+ unset($schema['tables'][$table]['indexes'][$name]);
+ }
+ }
+ }
+ }
+
+ $reader->tables = $schema['tables'];
+ $tableSql = $this->createSchema($reader, $table);
+
+ $fullTableName = $this->fullTableName($table);
+ $fullTmpName = $this->fullTableName("{$table}_tmp");
+
+ $cols = array_keys($schema['tables'][$table]);
+ $cols = array_diff($cols, array('indexes', 'tableParameters'));
+ $cols2 = $cols;
+ if (!empty($renamed)) {
+ foreach ($cols2 as &$name) {
+ if (array_key_exists($name, $renamed)) {
+ $name = $renamed[$name];
+ }
+ }
+ }
+ $cols = $this->_columnsAsString($cols);
+ $cols2 = $this->_columnsAsString($cols2);
+
+ $out[] = preg_replace("/$fullTableName/i", $fullTmpName, $tableSql, 1);
+ $out[] = "INSERT INTO $fullTmpName ($cols) SELECT $cols2 FROM $fullTableName;";
+ $out[] = "DROP TABLE $fullTableName;";
+ $out[] = "ALTER TABLE $fullTmpName RENAME TO $table;";
+ return $out;
+ }
+
+ private function _addColumns($table, $cols) {
+ $out = array();
+
+ $cols = array_diff_key($cols, array('indexes' => 0, 'tableParameters' => 0));
+ foreach ($cols as $col => $details) {
+ $details['name'] = $col;
+
+ // alter table must have default for fields that cannot be null
+ if (isset($details['null']) && !$details['null'] && empty($details['default'])) {
+ if(isset($details['type']) && ($details['type'] == 'text') || ($details['type'] == 'string')) {
+ $details['default'] = '';
+ } else {
+ $details['default'] = 0;
+ }
+ }
+
+ $out[] = "ALTER TABLE {$this->fullTableName($table)} ADD COLUMN {$this->buildColumn($details)};";
+ }
+
+ return $out;
+ }
+
+/**
+ * Generate a database-native schema for the given Schema object
+ *
+ * @param Model $schema An instance of a subclass of CakeSchema
+ * @param string $tableName Optional. If specified only the table name given will be generated.
+ * Otherwise, all tables defined in the schema are generated.
+ * @return string
+ */
+ public function createSchema($schema, $tableName = null) {
+ if (!is_a($schema, 'CakeSchema')) {
+ trigger_error(__d('cake_dev', 'Invalid schema object'), E_USER_WARNING);
+ return null;
+ }
+ $out = '';
+
+ foreach ($schema->tables as $curTable => $columns) {
+ if (!$tableName || $tableName == $curTable) {
+ $cols = $colList = $indexes = $tableParameters = array();
+ $table = $this->fullTableName($curTable);
+
+ // prevent duplicate primary key
+ // column is higher priority because it allows autoincrement to be used
+ foreach ($columns as $name => $col) {
+ if (isset($col['key']) && $col['key'] === 'primary') {
+ unset($columns['indexes']['PRIMARY']);
+ }
+ }
+ $primaryIndex = isset($columns['indexes']['PRIMARY']) ? $columns['indexes']['PRIMARY'] : '';
+
+ foreach ($columns as $name => $col) {
+ if (is_string($col)) {
+ $col = array('type' => $col);
+ }
+ if ($name !== 'indexes' && $name !== 'tableParameters') {
+ $col['name'] = $name;
+ if (!isset($col['type'])) {
+ $col['type'] = 'string';
+ }
+ $cols[] = $this->buildColumn($col);
+ } elseif ($name === 'indexes') {
+ $indexes = array_merge($indexes, $this->buildIndex($col, $curTable));
+ } elseif ($name === 'tableParameters') {
+ $tableParameters = array_merge($tableParameters, $this->buildTableParameters($col, $table));
+ }
+ }
+
+ $columns = $cols;
+ $out .= $this->renderStatement('schema', compact('table', 'columns', 'indexes', 'tableParameters', 'primaryIndex')) . "\n\n";
+ }
+ }
+ return $out;
+ }
+
+/**
+ * Plain indexes/keys (non-unique, non-primary) cannot be added by the create table statement.
+ * The keyword UNIQUE should not be followed by KEY
+ * Ignores names because they are not used by SQLite
+ *
+ * @param array $indexes
+ * @param string $table
+ * @return string
+*/
+ public function buildIndex($indexes, $table = null) {
+ $join = array();
+ if (!empty($table)) {
+ $table = $this->startQuote . $table . $this->endQuote;
+ foreach ($indexes as $name => $value) {
+ if ($name != 'PRIMARY') {
+ $unique = empty($value['unique'])? '' : 'UNIQUE';
+ $name = $this->fullTableName($name);
+ $join[] = "CREATE $unique INDEX $name ON $table ({$this->_columnsAsString($value['column'])})";;
+ }
}
- $t = trim($table, '"');
- $indexname = $this->name($t . '_' . $name);
- $table = $this->name($table);
- $out .= "INDEX {$dbname}.{$indexname} ON {$table}({$value['column']});";
- $join[] = $out;
}
return $join;
}
+ private function _getPrimaryKey($table) {
+ $info = $this->query("PRAGMA table_info($table)");
+ $pk = array();
+ foreach ($info as $index => $column) {
+ $column = $column[0];
+ if ($column['pk']) {
+ $pk[] = $column['name'];
+ }
+ }
+ return $pk;
+ }
+
/**
* Overrides DboSource::index to handle SQLite index introspection
* Returns an array of the indexes in given table name.
@@ -469,36 +698,44 @@ public function buildIndex($indexes, $table = null) {
* @return array Fields in table. Keys are column and unique
*/
public function index($model) {
- $index = array();
+ $result = array();
$table = $this->fullTableName($model, false, false);
- if ($table) {
- $indexes = $this->query('PRAGMA index_list(' . $table . ')');
- if (is_bool($indexes)) {
- return array();
- }
- foreach ($indexes as $info) {
- $key = array_pop($info);
- $keyInfo = $this->query('PRAGMA index_info("' . $key['name'] . '")');
- foreach ($keyInfo as $keyCol) {
- if (!isset($index[$key['name']])) {
- $col = array();
- if (preg_match('/autoindex/', $key['name'])) {
- $key['name'] = 'PRIMARY';
- }
- $index[$key['name']]['column'] = $keyCol[0]['name'];
- $index[$key['name']]['unique'] = intval($key['unique'] == 1);
- } else {
- if (!is_array($index[$key['name']]['column'])) {
- $col[] = $index[$key['name']]['column'];
+ if ($table) {
+ $indexes = $this->query("PRAGMA index_list($table)");
+ $pk = $this->_getPrimaryKey($table);
+ if (is_array($indexes)) {
+ foreach ($indexes as $i => $index) {
+ $index = array_pop($index);
+ $info = $this->query('PRAGMA index_info("' . $index['name'] . '")');
+ $autoindex = preg_match('/^sqlite_autoindex/', $index['name']);
+
+ $column = array();
+ $first_column = '';
+ foreach ($info as $j => $row) {
+ $column[] = $row[0]['name'];
+ if ($row[0]['seqno'] == 0) {
+ $first_column = $row[0]['name'];
}
- $col[] = $keyCol[0]['name'];
- $index[$key['name']]['column'] = $col;
}
+
+ if ($autoindex) {
+ $index['name'] = ($column == $pk) ? 'PRIMARY' : $first_column;
+ }
+
+ $result[$index['name']] = array(
+ 'column' => (count($column) == 1) ? $column[0] : $column,
+ 'unique' => intval($index['unique'] == 1)
+ );
}
}
+
+ if (empty($result['PRIMARY']) && !empty($pk)) {
+ $result['PRIMARY'] = array('column' => (count($pk) == 1) ? $pk[0] : $pk, 'unique' => 1);
+ }
}
- return $index;
+
+ return $result;
}
/**
@@ -509,19 +746,25 @@ public function index($model) {
* @return string
*/
public function renderStatement($type, $data) {
- switch (strtolower($type)) {
- case 'schema':
- extract($data);
- if (is_array($columns)) {
- $columns = "\t" . join(",\n\t", array_filter($columns));
- }
- if (is_array($indexes)) {
- $indexes = "\t" . join("\n\t", array_filter($indexes));
+ if (strtolower($type) == 'schema') {
+ extract($data);
+
+ foreach (array('columns', 'tableParameters') as $var) {
+ if (is_array(${$var})) {
+ ${$var} = "\t" . join(",\n\t", array_filter(${$var}));
+ } else {
+ ${$var} = '';
}
- return "CREATE TABLE {$table} (\n{$columns});\n{$indexes}";
- default:
- return parent::renderStatement($type, $data);
+ }
+
+ if (!empty($primaryIndex)) {
+ $columns .= ", PRIMARY KEY ({$this->_columnsAsString($primaryIndex['column'])})";
+ }
+ $indexes = is_array($indexes) ? join(";\n", $indexes) : '';
+
+ return "CREATE TABLE {$table} (\n{$columns}) {$tableParameters}; {$indexes}";
}
+ return parent::renderStatement($type, $data);
}
/**
@@ -569,4 +812,11 @@ public function nestedTransactionSupported() {
return $this->useNestedTransactions && version_compare($this->getVersion(), '3.6.8', '>=');
}
+ private function _columnsAsString($cols = array()) {
@markstory Owner

This function should be protected, not private.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ if (is_array($cols)) {
+ return implode(', ', array_map(array(&$this, 'name'), $cols));
+ } else {
+ return $this->name($cols);
+ }
+ }
}
View
313 lib/Cake/Test/Case/Model/Datasource/Database/SqliteTest.php
@@ -77,7 +77,7 @@ class SqliteTest extends CakeTestCase {
*
* @var object
*/
- public $fixtures = array('core.user', 'core.uuid');
+ public $fixtures = array('core.user', 'core.uuid', 'core.apple');
/**
* Actual DB connection used in testing
@@ -155,13 +155,185 @@ public function testIndex() {
}
/**
+ * test Index introspection.
+ *
+ * @return void
+ */
+ public function testIndex2() {
+ $this->Dbo->cacheSources = false;
+
+ $name = 'test_index_tbl';
+ $table = $this->Dbo->fullTableName($name, false, false);
+
+ // no primary key
+ $this->Dbo->rawQuery("CREATE TABLE $table (name text)");
+ $expected = array();
+ $result = $this->Dbo->index($table);
+ $this->assertEqual($result, $expected);
+ $this->Dbo->rawQuery("DROP TABLE $table");
+
+ // alias of ROWID
+ $this->Dbo->rawQuery("CREATE TABLE $table (id integer primary key, name text)");
+ $expected = array('PRIMARY' => array('column' => 'id', 'unique' => 1));
+ $result = $this->Dbo->index($table);
+ $this->assertEqual($result, $expected);
+ $this->Dbo->rawQuery("DROP TABLE $table");
+
+ // not an alias of ROWID
+ $this->Dbo->rawQuery("CREATE TABLE $table (id int primary key, name text)");
+ $expected = array('PRIMARY' => array('column' => 'id', 'unique' => 1));
+ $result = $this->Dbo->index($table);
+ $this->assertEqual($result, $expected);
+ $this->Dbo->rawQuery("DROP TABLE $table");
+
+ // composite primary key
+ $this->Dbo->rawQuery("CREATE TABLE $table (id1 integer, id2 integer, name text, primary key(id1, id2))");
+ $expected = array('PRIMARY' => array('column' => array('id1', 'id2'), 'unique' => 1));
+ $result = $this->Dbo->index($table);
+ $this->assertEqual($result, $expected);
+ $this->Dbo->rawQuery("DROP TABLE $table");
+
+ // unique column
+ $this->Dbo->rawQuery("CREATE TABLE $table (id integer primary key, name text unique)");
+ $expected = array(
+ 'PRIMARY' => array('column' => 'id', 'unique' => 1),
+ 'name' => array('column' => 'name', 'unique' => 1) // consistent with MySql
+ );
+ $result = $this->Dbo->index($table);
+ $this->assertEqual($result, $expected);
+ $this->Dbo->rawQuery("DROP TABLE $table");
+
+ // unique column pair
+ $this->Dbo->rawQuery("CREATE TABLE $table (id integer primary key, x integer, y integer, UNIQUE(x, y))");
+ $expected = array(
+ 'PRIMARY' => array('column' => 'id', 'unique' => 1),
+ 'x' => array('column' => array('x', 'y'), 'unique' => 1) // consistent with MySql
+ );
+ $result = $this->Dbo->index($table);
+ $this->assertEqual($result, $expected);
+ $this->Dbo->rawQuery("DROP TABLE $table");
+
+ // named single column index (not unique)
+ $indexName = 'index_for_name';
+ $this->Dbo->rawQuery("CREATE TABLE $table (id integer primary key, name text)");
+ $this->Dbo->rawQuery("CREATE INDEX $indexName ON $name (name)");
+ $expected = array(
+ 'PRIMARY' => array('column' => 'id', 'unique' => 1),
+ $indexName => array('column' => 'name', 'unique' => 0));
+ $result = $this->Dbo->index($table);
+ $this->assertEqual($result, $expected);
+ $this->Dbo->rawQuery("DROP TABLE $table");
+
+ // named multi column index (not unique)
+ $indexName = 'index_for_x_and_y';
+ $this->Dbo->rawQuery("CREATE TABLE $table (id integer primary key, x integer, y integer)");
+ $this->Dbo->rawQuery("CREATE INDEX $indexName ON $name (x, y)");
+ $expected = array(
+ 'PRIMARY' => array('column' => 'id', 'unique' => 1),
+ $indexName => array('column' => array('x', 'y'), 'unique' => 0));
+ $result = $this->Dbo->index($table);
+ $this->assertEqual($result, $expected);
+ $this->Dbo->rawQuery("DROP TABLE $table");
+ }
+
+/**
+ *
+ *
+ * autoincrement can only be declared on the primary key
+ * cannot use: CREATE TABLE my_table (id int(11) AUTOINCREMENT, bool tinyint(1), small_int tinyint(2), primary key(id));
+ * must use: CREATE TABLE my_table (id integer primary key AUTOINCREMENT, bool tinyint(1), small_int tinyint(2));
+ *
+ * non-unique indexes cannot be declared in CREATE TABE statement
+ * cannot use: CREATE TABLE my_table (id integer primary key AUTOINCREMENT, bool tinyint(1), small_int tinyint(2), KEY pointless_bool (bool))
+ * must use: CREATE TABLE my_table (id integer primary key AUTOINCREMENT, bool tinyint(1), small_int tinyint(2))
+ * CREATE INDEX pointless_bool on my_table (bool);
+ *
+ * @return void
+ */
+ public function testIndexDetection() {
+ $this->Dbo->cacheSources = false;
+
+ $name = $this->Dbo->fullTableName('simple');
+ $this->Dbo->rawQuery('CREATE TABLE ' . $name . ' (id integer primary key AUTOINCREMENT, bool tinyint(1), small_int tinyint(2));');
+ $expected = array('PRIMARY' => array('column' => 'id', 'unique' => 1));
+ $result = $this->Dbo->index('simple', false);
+ $this->Dbo->rawQuery('DROP TABLE ' . $name);
+ $this->assertEquals($expected, $result);
+
+ $name = $this->Dbo->fullTableName('simple');
+ $this->Dbo->rawQuery('CREATE TABLE ' . $name . ' (id int, bool tinyint(1), small_int tinyint(2), primary key(id));');
+ $expected = array('PRIMARY' => array('column' => 'id', 'unique' => 1));
+ $result = $this->Dbo->index('simple', false);
+ $this->Dbo->rawQuery('DROP TABLE ' . $name);
+ $this->assertEquals($expected, $result);
+
+ $name = $this->Dbo->fullTableName('with_a_key');
+ $this->Dbo->rawQuery('CREATE TABLE ' . $name . ' (id integer primary key AUTOINCREMENT, bool tinyint(1), small_int tinyint(2));');
+ $this->Dbo->rawQuery('CREATE INDEX pointless_bool on with_a_key (bool);');
+ $expected = array(
+ 'PRIMARY' => array('column' => 'id', 'unique' => 1),
+ 'pointless_bool' => array('column' => 'bool', 'unique' => 0),
+ );
+ $result = $this->Dbo->index('with_a_key', false);
+ $this->Dbo->rawQuery('DROP TABLE ' . $name);
+ $this->assertEquals($expected, $result);
+
+ $name = $this->Dbo->fullTableName('with_two_keys');
+ $this->Dbo->rawQuery('CREATE TABLE ' . $name . ' (id integer primary key AUTOINCREMENT, bool tinyint(1), small_int tinyint(2));');
+ $this->Dbo->rawQuery('CREATE INDEX pointless_bool on with_two_keys (bool);');
+ $this->Dbo->rawQuery('CREATE INDEX pointless_small_int on with_two_keys (small_int);');
+ $expected = array(
+ 'PRIMARY' => array('column' => 'id', 'unique' => 1),
+ 'pointless_bool' => array('column' => 'bool', 'unique' => 0),
+ 'pointless_small_int' => array('column' => 'small_int', 'unique' => 0),
+ );
+ $result = $this->Dbo->index('with_two_keys', false);
+ $this->Dbo->rawQuery('DROP TABLE ' . $name);
+ $this->assertEquals($expected, $result);
+
+ $name = $this->Dbo->fullTableName('with_compound_keys');
+ $this->Dbo->rawQuery('CREATE TABLE ' . $name . ' (id integer primary key AUTOINCREMENT, bool tinyint(1), small_int tinyint(2));');
+ $this->Dbo->rawQuery('CREATE INDEX pointless_bool on with_compound_keys (bool);');
+ $this->Dbo->rawQuery('CREATE INDEX pointless_small_int on with_compound_keys (small_int);');
+ $this->Dbo->rawQuery('CREATE INDEX one_way on with_compound_keys (bool, small_int);');
+ $expected = array(
+ 'PRIMARY' => array('column' => 'id', 'unique' => 1),
+ 'pointless_bool' => array('column' => 'bool', 'unique' => 0),
+ 'pointless_small_int' => array('column' => 'small_int', 'unique' => 0),
+ 'one_way' => array('column' => array('bool', 'small_int'), 'unique' => 0),
+ );
+ $result = $this->Dbo->index('with_compound_keys', false);
+ $this->Dbo->rawQuery('DROP TABLE ' . $name);
+ $this->assertEquals($expected, $result);
+
+ $name = $this->Dbo->fullTableName('with_multiple_compound_keys');
+ $this->Dbo->rawQuery('CREATE TABLE ' . $name . ' (id integer primary key AUTOINCREMENT, bool tinyint(1), small_int tinyint(2));');
+ $this->Dbo->rawQuery('CREATE INDEX pointless_bool on with_multiple_compound_keys (bool);');
+ $this->Dbo->rawQuery('CREATE INDEX pointless_small_int on with_multiple_compound_keys (small_int);');
+ $this->Dbo->rawQuery('CREATE INDEX one_way on with_multiple_compound_keys (bool, small_int);');
+ $this->Dbo->rawQuery('CREATE INDEX other_way on with_multiple_compound_keys (small_int, bool);');
+ $expected = array(
+ 'PRIMARY' => array('column' => 'id', 'unique' => 1),
+ 'pointless_bool' => array('column' => 'bool', 'unique' => 0),
+ 'pointless_small_int' => array('column' => 'small_int', 'unique' => 0),
+ 'one_way' => array('column' => array('bool', 'small_int'), 'unique' => 0),
+ 'other_way' => array('column' => array('small_int', 'bool'), 'unique' => 0),
+ );
+ $result = $this->Dbo->index('with_multiple_compound_keys', false);
+ $this->Dbo->rawQuery('DROP TABLE ' . $name);
+ $this->assertEquals($expected, $result);
+ }
+
+/**
* Tests that cached table descriptions are saved under the sanitized key name
*
*/
public function testCacheKeyName() {
Configure::write('Cache.disable', false);
+ Cache::set(array('duration' => '+1 days'));
- $dbName = 'db' . rand() . '$(*%&).db';
+ $dbName = 'db' . rand();
+ $dbName .= (DS === '\\') ? '$(%&).db' : '$(*%&).db';
$this->assertFalse(file_exists(TMP . $dbName));
$config = $this->Dbo->config;
@@ -256,6 +428,82 @@ public function testBuildColumn() {
}
/**
+ * test testCreateSchemaWithIndexes()
+ *
+ * SQLite does not support named table constraints. Say you want to 'DROP INDEX x_y_is_unique;'
+ * cannot use:
+ * create table aaa (id integer primary key autoincrement, x integer, y integer, constraint x_y_is_unique unique (x, y));
+ * must use:
+ * create table aaa (id integer primary key autoincrement, x integer, y integer);
+ * create index x_y_is_unique on aaa (x, y);
+ *
+ * @return void
+ */
+ public function testCreateSchemaWithIndexes() {
+ $this->Dbo->cacheSources = false;
+ $tableName = 'test_drop_schema';
+
+ $schema = new CakeSchema();
+ $schema->tables[$tableName] = array(
+ 'id' => array('type' => 'integer', 'null' => false, 'key' => 'primary'),
+ 'x' => array('type' => 'integer', 'null' => false),
+ 'data' => array('type' => 'text', 'null' => false),
+ 'indexes' => array(
+ 'x_is_unique' => array('column' => 'x', 'unique' => 1),
+ 'data_is_fast' => array('column' => 'data', 'unique => 0')
+ )
+ );
+
+ $sql = $this->Dbo->createSchema($schema);
+ $this->Dbo->rawQuery($sql);
+ $fields = $this->Dbo->describe($tableName);
+ $this->assertEqual($fields['id'], array('type' => 'integer', 'null' => false, 'length' => null, 'default' => null, 'key' => 'primary'));
+ $this->assertEqual($fields['x'], array('type' => 'integer', 'null' => false, 'length' => null, 'default' => null, 'key' => 'unique'));
+ $this->assertEqual($fields['data'], array('type' => 'text', 'null' => false, 'length' => null, 'default' => null, 'key' => 'index'));
+
+ $this->Dbo->rawQuery('DROP INDEX x_is_unique;');
+ $fields = $this->Dbo->describe($tableName);
+ $this->assertEqual($fields['x'], array('type' => 'integer', 'null' => false, 'length' => null, 'default' => null));
+
+ $this->Dbo->rawQuery('DROP INDEX data_is_fast;');
+ $fields = $this->Dbo->describe($tableName);
+ $this->assertEqual($fields['data'], array('type' => 'text', 'null' => false, 'length' => null, 'default' => null));
+
+ $this->Dbo->rawQuery("DROP TABLE $tableName");
+
+
+
+ $schema = new CakeSchema();
+ $schema->tables[$tableName] = array(
+ 'id' => array('type' => 'integer', 'null' => false),
+ 'x' => array('type' => 'integer', 'null' => false),
+ 'data' => array('type' => 'text', 'null' => false),
+ 'indexes' => array(
+ 'PRIMARY' => array('column' => 'id', 'unique' => 1),
+ 'x_is_unique' => array('column' => 'x', 'unique' => 1),
+ 'data_is_fast' => array('column' => 'data', 'unique => 0')
+ )
+ );
+
+ $sql = $this->Dbo->createSchema($schema);
+ $this->Dbo->rawQuery($sql);
+ $fields = $this->Dbo->describe($tableName);
+ $this->assertEqual($fields['id'], array('type' => 'integer', 'null' => false, 'length' => null, 'default' => null, 'key' => 'primary'));
+ $this->assertEqual($fields['x'], array('type' => 'integer', 'null' => false, 'length' => null, 'default' => null, 'key' => 'unique'));
+ $this->assertEqual($fields['data'], array('type' => 'text', 'null' => false, 'length' => null, 'default' => null, 'key' => 'index'));
+
+ $this->Dbo->rawQuery('DROP INDEX x_is_unique;');
+ $fields = $this->Dbo->describe($tableName);
+ $this->assertEqual($fields['x'], array('type' => 'integer', 'null' => false, 'length' => null, 'default' => null));
+
+ $this->Dbo->rawQuery('DROP INDEX data_is_fast;');
+ $fields = $this->Dbo->describe($tableName);
+ $this->assertEqual($fields['data'], array('type' => 'text', 'null' => false, 'length' => null, 'default' => null));
+
+ $this->Dbo->rawQuery("DROP TABLE $tableName");
+ }
+
+/**
* test describe() and normal results.
*
* @return void
@@ -274,7 +522,7 @@ public function testDescribe() {
'key' => 'primary',
'null' => false,
'default' => null,
- 'length' => 11
+ 'length' => null
),
'user' => array(
'type' => 'string',
@@ -311,6 +559,31 @@ public function testDescribe() {
}
/**
+ * test testDescribeApple() and normal results.
+ *
+ * @return void
+ */
+ public function testDescribeApple() {
+ $this->loadFixtures('Apple');
+ $this->Dbo->cacheSources = false;
+
+ $model = new Apple();
+ $fields_by_model = $this->Dbo->describe($model);
+ $fields_by_name = $this->Dbo->describe($model->useTable);
+ $this->assertIdentical($fields_by_model, $fields_by_name);
+
+ $fields = $fields_by_model;
+ $this->assertEqual($fields['id'], array('type' => 'integer', 'null' => false, 'default' => '', 'length' => null, 'key' => 'primary'));
+ $this->assertEqual($fields['apple_id'], array('type' => 'integer', 'null' => true, 'default' => null, 'length' => null));
+ $this->assertEqual($fields['color'], array('type' => 'string', 'null' => false, 'default' => '', 'length' => 40));
+ $this->assertEqual($fields['name'], array('type' => 'string', 'null' => false, 'default' => '', 'length' => 40));
+ $this->assertEqual($fields['created'], array('type' => 'datetime', 'null' => true, 'default' => null, 'length' => null));
+ $this->assertEqual($fields['date'], array('type' => 'date', 'null' => true, 'default' => null, 'length' => null));
+ $this->assertEqual($fields['modified'], array('type' => 'datetime', 'null' => true, 'default' => null, 'length' => null));
+ $this->assertEqual($fields['mytime'], array('type' => 'time', 'null' => true, 'default' => null, 'length' => null));
+ }
+
+/**
* test that describe does not corrupt UUID primary keys
*
* @return void
@@ -346,6 +619,40 @@ public function testDescribeWithUuidPrimaryKey() {
}
/**
+ * test that describe 'key' value is consistent with MySql
+ *
+ * @return void
+ */
+ public function testDescribeWithIndexes() {
+ $this->Dbo->cacheSources = false;
+
+ $name = $this->Dbo->fullTableName('with_multiple_compound_keys');
+ $this->Dbo->rawQuery('CREATE TABLE ' . $name . ' (id int primary key, bool tinyint(1), small_int tinyint(2), unique_int integer unique);');
+ $this->Dbo->rawQuery('CREATE INDEX pointless_bool on with_multiple_compound_keys (bool);');
+ $this->Dbo->rawQuery('CREATE INDEX one_way on with_multiple_compound_keys (bool, small_int);');
+
+ $fields = $this->Dbo->describe('with_multiple_compound_keys');
+ $this->assertEqual($fields['id']['key'], 'primary');
+ $this->assertEqual($fields['unique_int']['key'], 'unique');
+ $this->assertEqual($fields['bool']['key'], 'index');
+ $this->assertTrue(empty($fields['small_int']['key']));
+
+ $this->Dbo->rawQuery('CREATE INDEX other_way on with_multiple_compound_keys (small_int, bool);');
+
+ $fields = $this->Dbo->describe('with_multiple_compound_keys');
+ $this->assertEqual($fields['bool']['key'], 'index');
+ $this->assertEqual($fields['small_int']['key'], 'index');
+
+ $this->Dbo->rawQuery('CREATE UNIQUE INDEX pointless_small_int on with_multiple_compound_keys (small_int);');
+
+ $fields = $this->Dbo->describe('with_multiple_compound_keys');
+ $this->assertEqual($fields['bool']['key'], 'index');
+ $this->assertEqual($fields['small_int']['key'], 'unique');
+
+ $this->Dbo->rawQuery("DROP TABLE $name");
+ }
+
+/**
* Test virtualFields with functions.
*
* @return void
Something went wrong with that request. Please try again.