mysqldiff fails adding an auto_increment field

[toddr]

Opening from old RT https://rt.cpan.org/Public/Bug/Display.html?id=11708

Tue Mar 01 14:54:16 2005 SREZIC [...] cpan.org - Ticket created
Subject: mysqldiff fails adding an auto_increment field
Download (untitled) / with headers
text/plain 540b
mysqldiff fails if a field with auto incrementing should be added. The
generated SQL looks like this:

ALTER TABLE visits ADD COLUMN visit_id int(10) unsigned NOT NULL auto_increment;
ALTER TABLE visits ADD PRIMARY KEY (visit_id);

MySQL fails with the error message:
Incorrect table definition; There can only be one auto column and it must be defined as a key

It would work if both ALTER statements would be put into one:

ALTER TABLE visits ADD COLUMN visit_id int(10) unsigned NOT NULL auto_increment primary key;

Regards,
Slaven

[toddr]

This patch previously applied to version 0.43, but now as of 0.50 does not. at a minimum the section of the patch that mentions "push @changes,..." is no longer relevant since the code is referencing $changes now?

From 45dc5317e802fac90e297e0154b51b606293f8eb Mon Sep 17 00:00:00 2001
From: Aneece Yazdani <aneece@cpanel.net>
Date: Mon, 6 Jul 2015 14:06:38 -0500
Subject: [PATCH 3/3] Fix AUTO_INCREMENT ALTER statements

https://rt.cpan.org/Public/Bug/Display.html?id=11708

Previously, if a new column including the AUTO_INCREMENT attribute was
introduced or if an existing field was altered to include it, the SQL produced
by the diff method would fail. MySQL complains if you try to set AUTO_INCREMENT
for a column that does not have an index in place, and the diff was creating
the indices/keys in separate statments after the ones for column creation or
modification.

A new method has been added to handle AUTO_INCREMENT ALTER statements in a
special manner to aid against failure. First, the column is added/modified
using a less complete definition. Next, a temporary index is added and the
column definition is altered to include AUTO_INCREMENT. Finally, at a later
point in the diff process after the actual indices and primary key have been
set, the temporary index is dropped.
---
 modules/MySQL-Diff/MySQL-Diff/lib/MySQL/Diff.pm | 70 ++++++++++++++++++++++---
 1 file changed, 64 insertions(+), 6 deletions(-)

diff --git a/modules/MySQL-Diff/MySQL-Diff/lib/MySQL/Diff.pm b/modules/MySQL-Diff/MySQL-Diff/lib/MySQL/Diff.pm
index 5a97be9..8bf8458 100644
--- a/modules/MySQL-Diff/MySQL-Diff/lib/MySQL/Diff.pm
+++ b/modules/MySQL-Diff/MySQL-Diff/lib/MySQL/Diff.pm
@@ -57,6 +57,7 @@ sub new {
     bless $self, ref $class || $class;

     $self->{opts} = \%hash;
+    $self->{tmp_indices} = [];

     if($hash{debug})        { debug_level($hash{debug})     ; delete $hash{debug};      }
     if($hash{debug_file})   { debug_file($hash{debug_file}) ; delete $hash{debug_file}; }
@@ -184,7 +185,8 @@ sub _diff_tables {
         $self->_diff_fields(@_),
         $self->_diff_indices(@_),
         $self->_diff_primary_key(@_),
-        $self->_diff_options(@_)        
+        $self->_remove_tmp_indices(@_),
+        $self->_diff_options(@_)
     );

     $changes[-1] =~ s/\n*$/\n/  if (@changes);
@@ -222,10 +224,16 @@ sub _diff_fields {
                     {
                         debug(3,"field '$field' changed");

-                        my $change = "ALTER TABLE $name1 CHANGE COLUMN $field $field $f2;";
-                        $change .= " # was $f1" unless $self->{opts}{'no-old-defs'};
-                        $change .= "\n";
-                        push @changes, $change;
+                        if($table2->field($field) =~ /auto_increment/i) {
+                            push @changes, "# $field (altered below) was $f1\n" unless $self->{opts}{'no-old-defs'};
+                            push @changes, $self->_alter_auto_col($table2, $field);
+                        }
+                        else {
+                            my $change = "ALTER TABLE $name1 CHANGE COLUMN $field $field $f2;\n";
+                            $change .= " # was $f1" unless $self->{opts}{'no-old-defs'};
+                            $change .= "\n";
+                            push @changes, $change;
+                        }
                     }
                 }
             } else {
@@ -242,7 +250,12 @@ sub _diff_fields {
         for my $field (keys %$fields2) {
             unless($fields1 && $fields1->{$field}) {
                 debug(3,"field '$field' added");
-                push @changes, "ALTER TABLE $name1 ADD COLUMN $field $fields2->{$field};\n";
+                if($table2->field($field) =~ /auto_increment/i) {
+                    push @changes, $self->_alter_auto_col($table2, $field, 1);
+                }
+                else {
+                    push @changes, "ALTER TABLE $name1 ADD COLUMN $field $fields2->{$field};\n";
+                }
             }
         }
     }
@@ -347,6 +360,51 @@ sub _diff_primary_key {
     return @changes;
 }

+sub _remove_tmp_indices {
+    my ($self, $table1, $table2) = @_;
+
+    my @changes;
+    for my $tmp_index (@{$self->{tmp_indices}}) {
+        my $table_name = $table1->name;
+        push @changes, "DROP INDEX $tmp_index ON $table_name;\n";
+    }
+
+    $self->{tmp_indices} = [];
+
+    return @changes;
+}
+
+# AUTO_INCREMENT fields need to have an index. Since indices and primary keys are
+# added in separate statements in later steps, we must handle such fields a little
+# differently than normal.
+#
+# We first add/change the column without the AUTO_INCREMENT attribute, then add a
+# temporary index, and finally alter the column to include the AUTO_INCREMENT
+# attribute. Later, once the actual index and key diffs from table2 have been applied,
+# _diff_tables will safely remove the temporary index created via _remove_tmp_indices.
+sub _alter_auto_col {
+    my ($self, $table2, $field, $is_new) = @_;
+
+    my @changes;
+    my $table_name = $table2->name();
+    my $field_def = $table2->field($field);
+    (my $field_def_no_auto = $field_def) =~ s/\s*auto_increment(=\d+)?//i;
+    my $tmp_index = "tmp_${table_name}_on_$field";
+
+    push @{$self->{tmp_indices}}, $tmp_index;
+    if($is_new) {
+        push @changes, "ALTER TABLE $table_name ADD COLUMN $field $field_def_no_auto;\n";
+    }
+    else {
+        # This is not necessary in all cases, but ensures types match before getting started
+        push @changes, "ALTER TABLE $table_name CHANGE COLUMN $field $field $field_def_no_auto;\n";
+    }
+    push @changes, "CREATE INDEX $tmp_index on $table_name ($field);\n";
+    push @changes, "ALTER TABLE $table_name CHANGE COLUMN $field $field $field_def;\n";
+
+    return @changes;
+}
+
 # If we're about to drop a composite (multi-column) index, we need to
 # check whether any of the columns in the composite index are
 # auto_increment; if so, we have to add an index for that

[toddr]

This is possibly addressed by already merged pull request #32. Aneece Yazdani is the only one who can confirm this. @eserte can you comment since you originally opened the RT? Thanks!

[diegobill]

Hello, I am having a similar problem here.

source db:

CREATE TABLE `welcome_tabs_h` (
  `layout` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `pos` smallint(6) unsigned NOT NULL DEFAULT 0,
  `description` varchar(30) NOT NULL DEFAULT '',
  `fr` smallint(6) unsigned NOT NULL DEFAULT 0,
  `fg` smallint(6) unsigned NOT NULL DEFAULT 0,
  `fb` smallint(6) unsigned NOT NULL DEFAULT 0,
  `br` smallint(6) unsigned NOT NULL DEFAULT 0,
  `bg` smallint(6) unsigned NOT NULL DEFAULT 0,
  `bb` smallint(6) unsigned NOT NULL DEFAULT 0,
  `tabsv_pos` smallint(3) unsigned NOT NULL DEFAULT 0,
  PRIMARY KEY (`layout`,`pos`,`tabsv_pos`) USING BTREE
) ENGINE=MyISAM DEFAULT CHARSET=latin1;

destination db (has a primary key declared using BTREE):

CREATE TABLE `welcome_tabs_h` (
  `layout` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `pos` smallint(6) unsigned NOT NULL DEFAULT 0,
  `description` varchar(30) NOT NULL DEFAULT '',
  `fr` smallint(6) unsigned NOT NULL DEFAULT 0,
  `fg` smallint(6) unsigned NOT NULL DEFAULT 0,
  `fb` smallint(6) unsigned NOT NULL DEFAULT 0,
  `br` smallint(6) unsigned NOT NULL DEFAULT 0,
  `bg` smallint(6) unsigned NOT NULL DEFAULT 0,
  `bb` smallint(6) unsigned NOT NULL DEFAULT 0,
  `tabsv_pos` smallint(3) unsigned NOT NULL DEFAULT 0,
  PRIMARY KEY (`layout`,`pos`,`tabsv_pos`)
) ENGINE=MyISAM AUTO_INCREMENT=173 DEFAULT CHARSET=utf8mb4;

diff:

ALTER TABLE welcome_tabs_h DROP PRIMARY KEY; # was (layout,pos,tabsv_pos) USING BTREE
ALTER TABLE welcome_tabs_h ADD PRIMARY KEY (layout,pos,tabsv_pos);

executing diff on destination db:

Error Code: 1075
Incorrect table definition; there can be only one auto column and it must be defined as a key

Any suggestion?

[357r4bd]

@diegobill can you please confirm the version you're using, latest is 0.60 in CPAN - https://metacpan.org/pod/distribution/MySQL-Diff/bin/mysqldiff

[diegobill]

@357r4bd ,
yeah, I am using the version 0.60-1:

http://archive.ubuntu.com/ubuntu/pool/universe/libm/libmysql-diff-perl/libmysql-diff-perl_0.60-1_all.deb

[357r4bd]

thank you for clarifyin @diegobill, looking to see if the patch @toddr mentions above is missing

[diegobill]

@357r4bd ,
yes, these changes (https://github.com/aspiers/mysqldiff/pull/32/files) are applied to my files (/usr/share/perl5/MySQL/Diff.pm and /usr/share/perl5/MySQL/Diff/Table.pm)

[diegobill]

any update?