diff --git a/Makefile.PL b/Makefile.PL index 044b0694b..17b94090c 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -94,6 +94,9 @@ my $test_requires = { # this is already a dep of n::c, but just in case - used by t/55namespaces_cleaned.t # remove and do a manual glob-collection if n::c is no longer a dep 'Package::Stash' => '0.28', + + # Why isn't that neccessary in t/86sqlt.t? + 'SQL::Translator' => 0, }; # if the user has this env var set and no SQLT installed, tests will fail diff --git a/lib/SQL/Translator/Parser/DBIx/Class.pm b/lib/SQL/Translator/Parser/DBIx/Class.pm index e197491aa..ad23b6e2d 100644 --- a/lib/SQL/Translator/Parser/DBIx/Class.pm +++ b/lib/SQL/Translator/Parser/DBIx/Class.pm @@ -146,6 +146,7 @@ sub parse { $table->primary_key(@primary) if @primary; my %unique_constraints = $source->unique_constraints; + my @unique; foreach my $uniq (sort keys %unique_constraints) { if (!$source->_compare_relationship_keys($unique_constraints{$uniq}, \@primary)) { $table->add_constraint( @@ -153,8 +154,12 @@ sub parse { name => $uniq, fields => $unique_constraints{$uniq} ); + # store first column of unique constraint to avoid useless + # automatic index creation over this column later + push @unique, $unique_constraints{$uniq}->[0]; } } + #{ local $" = q(, ); warn qq(Unique columns [$table_name]: @unique); } my @rels = $source->relationships(); @@ -284,6 +289,11 @@ sub parse { # needed next if join("\x00", @keys) eq join("\x00", @primary); + # Check that we do not create an index on a column being the + # first column of a unique constraint + #{ local $" = q(, ); warn qq(Checking for unique columns [$table_name]: [@keys] vs. [@unique]); } + next if join("\x00", @keys) eq join("\x00", @unique); + if ($add_fk_index_rel) { (my $idx_name = $table_name) =~ s/ ^ [^\.]+ \. //x; my $index = $table->add_index( diff --git a/t/86sqlt-no-indices-on-unique-cols.t b/t/86sqlt-no-indices-on-unique-cols.t new file mode 100644 index 000000000..58d275e1a --- /dev/null +++ b/t/86sqlt-no-indices-on-unique-cols.t @@ -0,0 +1,95 @@ +use strict; +use warnings; +use Data::Dumper; +use SQL::Translator; # Why isn't that neccessary in t/86sqlt.t? + +use Test::More; +use lib qw(t/lib); +use DBICTest; + +BEGIN { + require DBIx::Class; + plan skip_all => 'Test needs ' + . DBIx::Class::Optional::Dependencies->req_missing_for('deploy') + unless DBIx::Class::Optional::Dependencies->req_ok_for('deploy'); +} ## end BEGIN + +note( + q(Checking there are no additional indices on first columns of unique constraints) +); + +note q(Change schema initialization to deploy automatically); +my $schema = DBICTest->init_schema(no_deploy => 1, no_populate => 1); +note q(Remove the custom deployment callback); +for my $t ($schema->sources) { + $schema->source($t)->sqlt_deploy_callback( + sub { + my ($self, $table) = @_; + note qq(Table resource $table was just deployed); + $self->default_sqlt_deploy_hook($table); + } + ); +} ## end for my $t ($schema->sources) +$schema->deploy; + +my $translator = SQL::Translator->new( + parser_args => { dbic_schema => $schema }, + parser => q(SQL::Translator::Parser::DBIx::Class), + producer_args => {}, +) or die SQL::Translator->error; +my $tschema = $translator->schema; + +TABLE: for ($schema->sources()) { + my $source = $schema->source($_); + my $table_name = $source->name; + note( + Data::Dumper->Dump( + [$_, $table_name], + [qw( schema_source source_name_before )] + ) + ); + my $tablename_type = ref $table_name; + if ($tablename_type) { + if ($tablename_type eq 'SCALAR') { + $tablename_type = $$table_name; + } + else { + note qq($_ type is skipped for unexpected type: $tablename_type); + next TABLE; + } + } ## end if ($tablename_type) + note( + Data::Dumper->Dump( + [$_, $table_name], + [qw( schema_source source_name_after )] + ) + ); + + my %ucs = $source->unique_constraints; + my @uc_first_cols = map { $ucs{$_}->[0] } keys %ucs; + + note qq(Searching indices of table $table_name); + my $_t = $tschema->get_table($table_name); # why are tables not populated?? + unless ($_t) { + note qq(Table not found: $table_name); + next; + } + + my @index_first_cols = $_t->get_indices; + + # table "cd" is my first demonstration + note( + explain( + { + $table_name => { + unique_constraints => [@uc_first_cols], + relations => [@index_first_cols], + } + } + ) + ) if $_ eq q(CD); +} ## end TABLE: for ($schema->sources) + +fail(q(!!! REMOVE ME WHEN FINISHED !!!)); + +done_testing(); diff --git a/t/86sqlt.t b/t/86sqlt.t index eccc5e245..3c7795a53 100644 --- a/t/86sqlt.t +++ b/t/86sqlt.t @@ -225,11 +225,18 @@ my %fk_constraints = ( # CD cd => [ { - 'display' => 'cd->artist', - 'name' => 'cd_fk_artist', 'index_name' => 'cd_idx_artist', - 'selftable' => 'cd', 'foreigntable' => 'artist', - 'selfcols' => ['artist'], 'foreigncols' => ['artistid'], - on_delete => 'CASCADE', on_update => 'CASCADE', deferrable => 1, + 'display' => 'cd->single_track', + 'name' => 'cd_fk_single_track', 'index_name' => 'cd_idx_single_track', + 'selftable' => 'cd', 'foreigntable' => 'track', + 'selfcols' => ['single_track'], 'foreigncols' => ['trackid'], + on_delete => 'CASCADE', on_update => '', deferrable => 1, + }, + { + 'display' => 'cd->genreid', + 'name' => 'cd_fk_genreid', 'index_name' => 'cd_idx_genreid', + 'selftable' => 'cd', 'foreigntable' => 'genre', + 'selfcols' => ['genreid'], 'foreigncols' => ['genreid'], + on_delete => 'SET NULL', on_update => 'CASCADE', deferrable => 1, }, ], diff --git a/t/99dbic_sqlt_parser.t b/t/99dbic_sqlt_parser.t index 0b368508b..55e8a91c3 100644 --- a/t/99dbic_sqlt_parser.t +++ b/t/99dbic_sqlt_parser.t @@ -108,6 +108,8 @@ my $idx_exceptions = { 'ForceForeign' => -1, 'LinerNotes' => -1, 'TwoKeys' => -1, # TwoKeys has the index turned off on the rel def + 'CD' => -1, # "track_cd_title" is UNIQUE constraint + 'LyricVersion' => -1, # "lyric_versions" is UNIQUE constraint }; { diff --git a/t/lib/sqlite.sql b/t/lib/sqlite.sql index 64ddc33c3..28d6dcd96 100644 --- a/t/lib/sqlite.sql +++ b/t/lib/sqlite.sql @@ -239,8 +239,6 @@ CREATE TABLE "cd" ( FOREIGN KEY ("genreid") REFERENCES "genre"("genreid") ON DELETE SET NULL ON UPDATE CASCADE ); -CREATE INDEX "cd_idx_artist" ON "cd" ("artist"); - CREATE INDEX "cd_idx_single_track" ON "cd" ("single_track"); CREATE INDEX "cd_idx_genreid" ON "cd" ("genreid"); @@ -285,8 +283,6 @@ CREATE TABLE "lyric_versions" ( FOREIGN KEY ("lyric_id") REFERENCES "lyrics"("lyric_id") ON DELETE CASCADE ON UPDATE CASCADE ); -CREATE INDEX "lyric_versions_idx_lyric_id" ON "lyric_versions" ("lyric_id"); - CREATE UNIQUE INDEX "lyric_versions_lyric_id_text" ON "lyric_versions" ("lyric_id", "text"); CREATE TABLE "tags" (