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
MIME-Version: 1.0
X-Mailer: MIME-tools 5.427 (Entity 5.427)
X-RT-Original-Encoding: utf-8
Content-Type: multipart/mixed; boundary="----------=_1328900163-1148-88"
Content-Length: 0
Content-Type: text/plain; charset="UTF-8"
Content-Disposition: inline
Content-Transfer-Encoding: binary
Content-Length: 1340
Download (untitled) / with headers
text/plain 1.3k
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
MIME-Version: 1.0
Content-Type: application/octet-stream; name="to.sql"
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline; filename="to.sql"
Content-Transfer-Encoding: base64
Content-Length: 569
Download to.sql
application/octet-stream 569b

Message body not shown because it is not plain text.

Subject: from.sql
MIME-Version: 1.0
Content-Type: application/octet-stream; name="from.sql"
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline; filename="from.sql"
Content-Transfer-Encoding: base64
Content-Length: 16
Download from.sql
application/octet-stream 16b

Message body not shown because it is not plain text.

MIME-Version: 1.0
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Type: multipart/mixed; boundary="----------=_1328918844-17364-203"
Message-ID: <rt-3.8.HEAD-17364-1328918844-1397.74859-0-0 [...] rt.cpan.org>
X-RT-Original-Encoding: utf-8
Content-Length: 0
Content-Disposition: inline
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 607
Download (untitled) / with headers
text/plain 607b
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.
MIME-Version: 1.0
Subject: MySQL.diff
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Type: application/octet-stream; name="MySQL.diff"
Content-Disposition: inline; filename="MySQL.diff"
Content-Transfer-Encoding: base64
Content-Length: 1981
Download MySQL.diff
text/x-diff 1.9k
--- 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; } }
MIME-Version: 1.0
In-Reply-To: <rt-3.8.HEAD-17364-1328918844-1397.74859-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline
References: <rt-3.8.HEAD-17364-1328918844-1397.74859-0-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="UTF-8"
Message-ID: <rt-3.8.HEAD-20175-1335579189-1191.74859-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 776
Download (untitled) / with headers
text/plain 776b
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!


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.