Description: ------------ Hi. Sorry to bother you again, but I found a second problem when upgrading a database. The following table (just one out of many) CREATE TABLE `flyspray_admin_requests` ( `request_id` int(5) NOT NULL auto_increment, `project_id` int(5) NOT NULL default '0', `task_id` int(5) NOT NULL default '0', `submitted_by` int(5) NOT NULL default '0', `request_type` int(2) NOT NULL default '0', `reason_given` text, `time_submitted` int(11) NOT NULL default '0', `resolved_by` int(5) NOT NULL default '0', `time_resolved` int(11) NOT NULL default '0', `deny_reason` text, PRIMARY KEY (`request_id`), KEY `reminders_taskid` (`task_id`), KEY `reminders_project` (`project_id`) ) results in the following XML (which is correct so far): <table> <name><variable>db_prefix</variable>admin_requests</name> <declaration> <field> <name>request_id</name> <type>integer</type> <length>4</length> <notnull>true</notnull> <default>0</default> <autoincrement>1</autoincrement> </field> <field> <name>project_id</name> <type>integer</type> <length>4</length> <notnull>true</notnull> <default>0</default> </field> <field> <name>task_id</name> <type>integer</type> <length>4</length> <notnull>true</notnull> <default>0</default> </field> <field> <name>submitted_by</name> <type>integer</type> <length>4</length> <notnull>true</notnull> <default>0</default> </field> <field> <name>request_type</name> <type>integer</type> <length>4</length> <notnull>true</notnull> <default>0</default> </field> <field> <name>reason_given</name> <type>text</type> <notnull>false</notnull> </field> <field> <name>time_submitted</name> <type>integer</type> <length>4</length> <notnull>true</notnull> <default>0</default> </field> <field> <name>resolved_by</name> <type>integer</type> <length>4</length> <notnull>true</notnull> <default>0</default> </field> <field> <name>time_resolved</name> <type>integer</type> <length>4</length> <notnull>true</notnull> <default>0</default> </field> <field> <name>deny_reason</name> <type>text</type> <notnull>false</notnull> </field> <index> <name>reminders_taskid</name> <field> <name>task_id</name> <sorting>ascending</sorting> </field> </index> <index> <name>reminders_project</name> <field> <name>project_id</name> <sorting>ascending</sorting> </field> </index> <index> <name><variable>db_prefix</variable>admin_requests_pKey</name> <primary>true</primary> <field> <name>request_id</name> <sorting>ascending</sorting> </field> </index> </declaration> </table> However, when importing this XML and the table already exists (with $db->setOption('idxname_format', '%s') ) the following query is executed DROP INDEX primary ON flyspray_assigned which causes a syntax error and the upgrade is aborted. In fact dropping this key is not possible in MySQL anyway. If idxname_format is not set, it attempts to remove the key primary_idx, which is not found and upgrader aborts again. A workaround is to comment out line 1625 in Schema.php (0.7.2). Then it does not abort if an index could not be deleted. Any help or a fix if neccessary is appreciated. Test script: --------------- The importing script looks like this: $db->setOption('idxname_format', '%s'); $schema =& MDB2_Schema::factory($db); $schema->setOption('force_defaults', false); $previous_schema = $schema->getDefinitionFromDatabase(); $res = $schema->updateDatabase($file, $previous_schema, array('db_prefix' => $conf['database']['dbprefix'], 'db_name' => $conf['database']['dbname'])); Expected result: ---------------- MDB2 does not attempt to drop the primary index. Actual result: -------------- MDB2 attempts to remove a key called "primary" which results in a syntax error and is not possible anyway.

Have you tried with option "quote_identifier" set to TRUE ?

I tried that now, and as expected I get a similar error: DROP INDEX `primary` ON `flyspray_admin_requests`] [Native code: 1075] [Native message: Incorrect table definition; there can be only one auto column and it must be defined as a key] As I said before, the key cannot be deleted anyway.

Seems a MDB2 bug to me. Maybe the autoincrement should be dropped before the key ? Looping to Lorenzo to see what he thinks.

I still think that it shouldn't be dropped at all. Either only the whole table, for the other indices. But thanks for investigating regarding this issue.

Hello, I will take a deeper look at this in a few days. Anyway the primary key issue is also something that requires our attention. I see two bugs here. regards, iGor.

Sorry, I haven't looked at the actual issue yet, but maybe the problem is you're trying to drop an *INDEX* with dropIndex() while you should drop the PK *constraint* with dropConstraint()...

I have investigated a little more to get this fixed a little faster. In alterDatabaseIndexes(), Schema.php is doing a loop foreach ($changes['remove'] as $index_name => $index) ... and then checks empty($index['primary']) to decide whether to drop a constraint or an index. However, $changes['remove'] only looks like this when var_dump'ed: array(2) { ["any_usual_index"]=> bool(true) ["primary"]=> bool(true) } So there is no 'primary' key in $index at any time. This again seems to be expected, if you consider the code foreach ($previous_definition as $index_previous_name => $index_previous) { if (empty($defined_indexes[$index_previous_name])) { $changes['remove'][$index_previous_name] = true; } } in compareTableIndexesDefinitions(). So a possible fix would be to replace 'true', so that it looks like this: foreach ($previous_definition as $index_previous_name => $index_previous) { if (empty($defined_indexes[$index_previous_name])) { $changes['remove'][$index_previous_name] = $index_previous; } } I still wonder why this should me an MDB2 instead of an MDB2_Schema bug.

Thanks for the analysis, Florian. Moving bug report back to MDB2_Schema.

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.

Hm, I can confirm that the fix in CVS now generates the expected query (which is "ALTER TABLE xyz DROP PRIMARY KEY"), however, the actual problem (it attempts to delete the primary key) still remains. Now the error message is "Incorrect table definition; there can be only one auto column and it must be defined as a key". This happens because there still is an auto-increment column in the table which (as the message says) must be defined as primary key. So I'd say that unless there is a reason to delete the primary key, it shouldn't be deleted. Is a new bug required for that?

The proble is that the current / previous definitions in compareTableIndexesDefinitions are different. Only the latter one contains the "primary" key. Current: Array ( [reminders_taskid] => Array ( [fields] => Array ( [task_id] => Array ( [sorting] => ascending ) ) [was] => reminders_taskid ) [reminders_project] => Array ( [fields] => Array ( [project_id] => Array ( [sorting] => ascending ) ) [was] => reminders_project ) ) Previous: Array ( [reminders_taskid] => Array ( [fields] => Array ( [task_id] => Array ( [sorting] => ascending ) ) ) [reminders_project] => Array ( [fields] => Array ( [project_id] => Array ( [sorting] => ascending ) ) ) [primary] => Array ( [primary] => 1 [fields] => Array ( [request_id] => Array ( [sorting] => ascending ) ) ) )

Hello Florian, Yes that's why I said it is probably a MDB2 bug. Dropping a primay key shouldn't return an error, even though we are talking about MySQL. The dependencies should be handled by the MySQL driver. So you are trying to update a table, that doesn't have its primary key anymore. Right ? Or both schemas do have the PK but one of them are being dumped incorrectly ? Regards, iGor.

As you can see in my original comment, the table in the database still has the primary key. The table is dumped correctly when I export it as XML, but during database update there seems to be something wrong. I'm not quite sure where to look for the problem though. It might be that MDB2 needs to handle the dependencies for dropping a primary key, however, as I said multiple times before, there *is no need* to drop the key at all.

So in other words, the current-array from above does not contain the primary key (although it should).

Could you please provide the CREATE TABLE statement for both table schemas (previous and current) ? Or better yet, the XML of current schema and the var_dump of $previous_schema (only the section of the table in question). Thank you, iGor.

Current === Previous I'm actually updating the database without expecting anything to be changed. So the create table statement in my very first comment is both the current and previous table. Here is the content of $previous_schema in my example script: array(5) { ["name"]=> string(8) "clean099" ["create"]=> bool(true) ["overwrite"]=> bool(false) ["tables"]=> array(28) { ["flyspray_admin_requests"]=> &array(2) { ["fields"]=> array(10) { ["request_id"]=> array(8) { ["notnull"]=> bool(true) ["nativetype"]=> string(3) "int" ["length"]=> int(4) ["unsigned"]=> int(0) ["default"]=> string(1) "0" ["autoincrement"]=> bool(true) ["type"]=> string(7) "integer" ["mdb2type"]=> string(7) "integer" } ["project_id"]=> array(7) { ["notnull"]=> bool(true) ["nativetype"]=> string(3) "int" ["length"]=> int(4) ["unsigned"]=> int(0) ["default"]=> string(1) "0" ["type"]=> string(7) "integer" ["mdb2type"]=> string(7) "integer" } ["task_id"]=> array(7) { ["notnull"]=> bool(true) ["nativetype"]=> string(3) "int" ["length"]=> int(4) ["unsigned"]=> int(0) ["default"]=> string(1) "0" ["type"]=> string(7) "integer" ["mdb2type"]=> string(7) "integer" } ["submitted_by"]=> array(7) { ["notnull"]=> bool(true) ["nativetype"]=> string(3) "int" ["length"]=> int(4) ["unsigned"]=> int(0) ["default"]=> string(1) "0" ["type"]=> string(7) "integer" ["mdb2type"]=> string(7) "integer" } ["request_type"]=> array(7) { ["notnull"]=> bool(true) ["nativetype"]=> string(3) "int" ["length"]=> int(4) ["unsigned"]=> int(0) ["default"]=> string(1) "0" ["type"]=> string(7) "integer" ["mdb2type"]=> string(7) "integer" } ["reason_given"]=> array(7) { ["notnull"]=> bool(false) ["nativetype"]=> string(4) "text" ["fixed"]=> bool(false) ["default"]=> NULL ["type"]=> string(4) "text" ["mdb2type"]=> string(4) "text" ["choices"]=> array(2) { [0]=> array(6) { ["notnull"]=> bool(false) ["nativetype"]=> string(4) "text" ["fixed"]=> bool(false) ["default"]=> NULL ["type"]=> string(4) "text" ["mdb2type"]=> string(4) "text" } [1]=> array(5) { ["notnull"]=> bool(false) ["nativetype"]=> string(4) "text" ["fixed"]=> bool(false) ["type"]=> string(4) "clob" ["mdb2type"]=> string(4) "clob" } } } ["time_submitted"]=> array(7) { ["notnull"]=> bool(true) ["nativetype"]=> string(3) "int" ["length"]=> int(4) ["unsigned"]=> int(0) ["default"]=> string(1) "0" ["type"]=> string(7) "integer" ["mdb2type"]=> string(7) "integer" } ["resolved_by"]=> array(7) { ["notnull"]=> bool(true) ["nativetype"]=> string(3) "int" ["length"]=> int(4) ["unsigned"]=> int(0) ["default"]=> string(1) "0" ["type"]=> string(7) "integer" ["mdb2type"]=> string(7) "integer" } ["time_resolved"]=> array(7) { ["notnull"]=> bool(true) ["nativetype"]=> string(3) "int" ["length"]=> int(4) ["unsigned"]=> int(0) ["default"]=> string(1) "0" ["type"]=> string(7) "integer" ["mdb2type"]=> string(7) "integer" } ["deny_reason"]=> array(7) { ["notnull"]=> bool(false) ["nativetype"]=> string(4) "text" ["fixed"]=> bool(false) ["default"]=> NULL ["type"]=> string(4) "text" ["mdb2type"]=> string(4) "text" ["choices"]=> array(2) { [0]=> array(6) { ["notnull"]=> bool(false) ["nativetype"]=> string(4) "text" ["fixed"]=> bool(false) ["default"]=> NULL ["type"]=> string(4) "text" ["mdb2type"]=> string(4) "text" } [1]=> array(5) { ["notnull"]=> bool(false) ["nativetype"]=> string(4) "text" ["fixed"]=> bool(false) ["type"]=> string(4) "clob" ["mdb2type"]=> string(4) "clob" } } } } ["indexes"]=> array(3) { ["reminders_taskid"]=> array(1) { ["fields"]=> array(1) { ["task_id"]=> array(1) { ["sorting"]=> string(9) "ascending" } } } ["reminders_project"]=> array(1) { ["fields"]=> array(1) { ["project_id"]=> array(1) { ["sorting"]=> string(9) "ascending" } } } ["primary"]=> array(2) { ["primary"]=> bool(true) ["fields"]=> array(1) { ["request_id"]=> array(1) { ["sorting"]=> string(9) "ascending" } } } } }

This is a collateral effect of validateTable(). Since we have an autoincrement field, validateTable() skip the primary key when creating the definition array. This is a desirable behaviour when creating a table but now it is clear that it can be a problem when updating tables. Working on the fix.

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. please let me know if you still have any issue.

It works fine now, thanks.