Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 0.8.6

Request #10296 Existence of a single index results in no further index creation
Submitted: 2007-03-07 15:56 UTC
From: fornax Assigned: ifeghali
Status: Closed Package: MDB2_Schema (version 0.7.1)
PHP Version: 4.3.11 OS: Centos 4.3, Apache 1.3
Roadmaps: (Not assigned)    
Subscription  


 [2007-03-07 15:56 UTC] fornax (Andrew Hill)
Description: ------------ When creating table indexes with MDB2_Schema::createTableIndexes(), if the $overwrite parameter is set to false, the existence of any index will prevent any further indexes from being created. As MDB2 often creates the PRIMARY KEY index for a table at table creation time (as it may be required for AUTO_INCREMENT, etc), this means that the MDB2_Schema::createTableIndexes() cannot be used to add additional indexes without re-creating the original index(es). This may be a little slow; and the ability to simply create any new indexes would be preferred. (After all, MDB2_OK is returned if the index already exists now, so not overwriting the index and creating the new index[es] is just as valid.) Patch below. Test script: --------------- --- MDB2/Schema.php (revision 4892) +++ MDB2/Schema.php (working copy) @@ -531,12 +531,12 @@ // {{{ createTableIndexes() /** - * Create indexes on a table + * A method to create indexes for an existing table * - * @param string name of the table - * @param array indexes to be created - * @param bool if the table/index should be overwritten if it already exists - * @return bool|MDB2_Error MDB2_OK or error object + * @param string Name of the table + * @param array An array of indexes to be created + * @param boolean If the table/index should be overwritten if it already exists + * @return mixed MDB2_Error if there is an error creating an index, MDB2_OK otherwise * @access public */ function createTableIndexes($table_name, $indexes, $overwrite = false) @@ -546,9 +546,10 @@ return MDB2_OK; } - $supports_primary_key = $this->db->supports('primary_key'); + $errorcodes = array(MDB2_ERROR_UNSUPPORTED, MDB2_ERROR_NOT_CAPABLE); foreach ($indexes as $index_name => $index) { - $errorcodes = array(MDB2_ERROR_UNSUPPORTED, MDB2_ERROR_NOT_CAPABLE); + // Does the index already exist, and if so, should it be overwritten? + $create_index = true; $this->db->expectError($errorcodes); if (!empty($index['primary']) || !empty($index['unique'])) { $current_indexes = $this->db->manager->listTableConstraints($table_name); @@ -563,36 +564,30 @@ } elseif (is_array($current_indexes) && in_array($index_name, $current_indexes)) { if (!$overwrite) { $this->db->debug('Index already exists: '.$index_name, __FUNCTION__); - return MDB2_OK; - } - if (!empty($index['primary']) || !empty($index['unique'])) { - $result = $this->db->manager->dropConstraint($table_name, $index_name); + $create_index = false; } else { - $result = $this->db->manager->dropIndex($table_name, $index_name); + $this->db->debug('Preparing to overwrite index: '.$index_name, __FUNCTION__); + if (!empty($index['primary']) || !empty($index['unique'])) { + $result = $this->db->manager->dropConstraint($table_name, $index_name); + } else { + $result = $this->db->manager->dropIndex($table_name, $index_name); + } + if (PEAR::isError($result)) { + return $result; + } } - if (PEAR::isError($result)) { - return $result; - } - $this->db->debug('Overwritting index: '.$index_name, __FUNCTION__); } - - // check if primary is being used and if it's supported - if (!empty($index['primary']) && !$supports_primary_key) { - /** - * Primary not supported so we fallback to UNIQUE - * and making the field NOT NULL - */ + // Check if primary is being used and if it's supported + if (!empty($index['primary']) && !$this->db->supports('primary_key')) { + // Primary not supported so we fallback to UNIQUE and making the field NOT NULL unset($index['primary']); $index['unique'] = true; - $changes = array(); - foreach ($index['fields'] as $field => $empty) { $field_info = $this->db->reverse->getTableFieldDefinition($table_name, $field); if (PEAR::isError($field_info)) { return $field_info; } - if (!$field_info[0]['notnull']) { $changes['change'][$field] = $field_info[0]; $changes['change'][$field]['notnull'] = true; @@ -602,15 +597,17 @@ $this->db->manager->alterTable($table_name, $changes, false); } } - - if (!empty($index['primary']) || !empty($index['unique'])) { - $result = $this->db->manager->createConstraint($table_name, $index_name, $index); - } else { - $result = $this->db->manager->createIndex($table_name, $index_name, $index); + // Should the index be created? + if ($create_index) { + if (!empty($index['primary']) || !empty($index['unique'])) { + $result = $this->db->manager->createConstraint($table_name, $index_name, $index); + } else { + $result = $this->db->manager->createIndex($table_name, $index_name, $index); + } + if (PEAR::isError($result)) { + return $result; + } } - if (PEAR::isError($result)) { - return $result; - } } return MDB2_OK; }

Comments

 [2007-03-10 15:35 UTC] ifeghali (Igor Feghali)
This bug has been fixed in CVS. If this was a documentation problem, the fix will appear on pear.php.net by the end of next Sunday (CET). If this was a problem with the pear.php.net website, the change should be live shortly. Otherwise, the fix will appear in the package's next release. Thank you for the report and for helping us make PEAR better.