Skip Menu |

This queue is for tickets about the SQL-Translator CPAN distribution.

Report information
The Basics
Id: 74859
Status: open
Priority: 0/
Queue: SQL-Translator

People
Owner: Nobody in particular
Requestors: TILLY [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: 0.11010
Fixed in: (no value)



Subject: Field in UNIQUE + CONSTRAINT can give invalid SQL
If a field appears in both a UNIQUE constraint and a CONSTRAINT, SQL::Translator does not realize that the UNIQUE implies an index, inserts an INDEX, passes through the UNIQUE, and then MySQL barfs because you have 2 indexes with the same name. With the two example files I provided, try this and note that the UNIQUE and INDEX are named the same thing: --------- $ sqlt-diff from.sql=MySQL to.sql=MySQL This code is experimental, currently the new code only supports MySQL or SQLite diffing. To add support for other databases, please patch the relevant SQL::Translator::Producer:: module. If you need compatibility with the old sqlt-diff, please use sqlt-diff-old, and look into helping us make this one work for you -- Convert schema 'from.sql' to 'to.sql':; BEGIN; SET foreign_key_checks=0; CREATE TABLE `service_one_click` ( `id` integer(11) NOT NULL auto_increment, `database_name` varchar(64) NOT NULL, INDEX `database_name` (`database_name`), PRIMARY KEY (`id`), CONSTRAINT `service_one_click_fk_database_name` FOREIGN KEY (`database_name`) REFERENCES `service_database` (`name`) ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARACTER SET utf8; SET foreign_key_checks=1; COMMIT; --------- Note that if you change the UNIQUE in to.sql to an INDEX, SQL::Translator understands that the index is already there.
Subject: to.sql
Download to.sql
application/octet-stream 569b

Message body not shown because it is not plain text.

Subject: from.sql
Download from.sql
application/octet-stream 16b

Message body not shown because it is not plain text.

The appended patch fixes my bug. It does it by counting UNIQUE constraints as indexes (they are). While I was at it I improved the support for concatenated indexes (an index on (`foo`, `bar`) counts as an index on (`foo`), but not on (`bar`), and an index on (`bar`) does not count as an index on (`foo`, `bar`). Various permutations of this logic were wrong previously. I also added a deprecation note, so that people can see why the logic is there, and when they might want to deprecate it. I am using this patch locally to work around a bug, if you won't accept it upstream, please let me know.
Subject: MySQL.diff
--- MySQL.pm.old 2012-02-10 15:22:04.000000000 -0800 +++ MySQL.pm 2012-02-10 16:01:04.000000000 -0800 @@ -456,21 +456,46 @@ my %indexed_fields; for my $index ( $table->get_indices ) { push @index_defs, create_index($index, $options); - $indexed_fields{ $_ } = 1 for $index->fields; + my @fields; + for my $field ( $index->fields ) { + push @fields, $field; + $indexed_fields{ join ",", @fields } = 1; + } } # - # Constraints -- need to handle more than just FK. -ky + # Constraints -- may need to handle more than just FK and UNIQUE. # my @constraint_defs; my @constraints = $table->get_constraints; + my @fk_constraints; for my $c ( @constraints ) { my $constr = create_constraint($c, $options); push @constraint_defs, $constr if($constr); - - unless ( $indexed_fields{ ($c->fields())[0] } || $c->type ne FOREIGN_KEY ) { - push @index_defs, "INDEX ($qf" . ($c->fields())[0] . "$qf)"; - $indexed_fields{ ($c->fields())[0] } = 1; + + if ( $c->type eq UNIQUE ) { + my @fields; + for my $field ( $c->fields() ) { + push @fields, $field; + $indexed_fields{ join ",", @fields } = 1; + } + } + elsif ( $c->type eq FOREIGN_KEY ) { + push @fk_constraints, $c; + } + } + + # + # And add indexes for fk constraints. + # + # DEPRECATION NOTE, manual index creation logic has not been needed since + # MySQL 4.1.2, released May 30, 2004. Should we deprecate support for + # archaic versions of MySQL, this can go. + for my $c ( @fk_constraints ) { + if ( not $indexed_fields{ join ",", $c->fields() } ) { + push @index_defs, "INDEX ($qf" . + (join "$qf, $qf", $c->fields()) . "$qf)"; + $indexed_fields{ join ",", $c->fields() } = 1; } }
On Fri Feb 10 19:07:24 2012, TILLY wrote: Show quoted text
> The appended patch fixes my bug. It does it by counting UNIQUE > constraints as indexes (they > are). > > While I was at it I improved the support for concatenated indexes (an > index on (`foo`, `bar`) > counts as an index on (`foo`), but not on (`bar`), and an index on > (`bar`) does not count as an > index on (`foo`, `bar`). Various permutations of this logic were > wrong previously. > > I also added a deprecation note, so that people can see why the logic > is there, and when they > might want to deprecate it. > > I am using this patch locally to work around a bug, if you won't > accept it upstream, please let me > know.
Can you please add an extra test for this, so we know we won't break it after fixing. Thanks!