fix: Don't add indices on columns with UNIQUE constraint

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

lib/SQL/Translator/Parser/DBIx/Class.pm

@@ -146,15 +146,20 @@ 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(
                             type             => 'unique',
                             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(

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();

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,
     },
   ],
 

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
 };
 
 {

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" (