added a working mechanism for naming foreign keys

diff --git a/Changes b/Changes
index ddb3489..ea7fa94 100644 (file)
--- a/Changes
+++ b/Changes
@@ -3,6 +3,7 @@
 * Add column and table comments in SQLT::Parser::DBI::PostgreSQL(patch from Andrew Pam)
 * Fixed alter_drop_constraint for foreign keys and applying multiple changes
   via alter_field to a column in Postgres Producer
+* added a working mechanism for naming foreign keys in the PostgreSQL producer
 
 # ----------------------------------------------------------
 # 0.11010 2011-10-05

diff --git a/lib/SQL/Translator/Producer/PostgreSQL.pm b/lib/SQL/Translator/Producer/PostgreSQL.pm
index 66b295b..5f3d745 100644 (file)
--- a/lib/SQL/Translator/Producer/PostgreSQL.pm
+++ b/lib/SQL/Translator/Producer/PostgreSQL.pm
@@ -39,7 +39,7 @@ Does not yet support PostGIS Views.
 
 use strict;
 use warnings;
-use vars qw[ $DEBUG $WARN $VERSION %used_names ];
+use vars qw[ $DEBUG $WARN $VERSION ];
 $VERSION = '1.59';
 $DEBUG = 0 unless defined $DEBUG;
 
@@ -278,24 +278,6 @@ sub mk_name {
     return $name;
 }
 
-# -------------------------------------------------------------------
-sub next_unused_name {
-    my $orig_name = shift or return;
-    my $name      = $orig_name;
-
-    my $suffix_gen = sub {
-        my $suffix = 0;
-        return ++$suffix ? '' : $suffix;
-    };
-
-    for (;;) {
-        $name = $orig_name . $suffix_gen->();
-        last if $used_names{ $name }++;
-    }
-
-    return $name;
-}
-
 sub is_geometry
 {
        my $field = shift;
@@ -584,10 +566,9 @@ sub create_index
 
     my ($index_def, @constraint_defs);
 
-    my $name = next_unused_name(
-        $index->name 
-        || join('_', $table_name, 'idx', ++$index_name{ $table_name })
-    );
+    my $name
+        = $index->name
+        || join('_', $table_name, 'idx', ++$index_name{ $table_name });
 
     my $type = $index->type || NORMAL;
     my @fields     =  $index->fields;
@@ -624,9 +605,6 @@ sub create_constraint
     my (@constraint_defs, @fks);
 
     my $name = $c->name || '';
-    if ( $name ) {
-        $name = next_unused_name($name);
-    }
 
     my @fields = grep { defined } $c->fields;
 
@@ -646,8 +624,8 @@ sub create_constraint
         push @constraint_defs, "${def_start}CHECK ($expression)";
     }
     elsif ( $c->type eq FOREIGN_KEY ) {
-        my $def .= "ALTER TABLE ${qt}${table_name}${qt} ADD FOREIGN KEY " . $field_names .
-            "\n  REFERENCES " . $qt . $c->reference_table . $qt;
+        my $def .= "ALTER TABLE $qt$table_name$qt ADD ${def_start}FOREIGN KEY $field_names"
+            . "\n  REFERENCES " . $qt . $c->reference_table . $qt;
 
         if ( @rfields ) {
             $def .= ' ('.$qf . join( $qf.', '.$qf, @rfields ) . $qf.')';
@@ -974,7 +952,9 @@ sub alter_drop_constraint {
         # for naming foreign keys, it names them uses the name of
         # the table as prefix and fkey as suffix, concatenated by a underscore
         $c->type eq FOREIGN_KEY
-            ? $qc . $c->table->name . '_' . ($c->fields)[0] . '_fkey' . $qc
+            ? $c->name
+                ? $qc . $c->name . $qc
+                : $qc . $c->table->name . '_' . ($c->fields)[0] . '_fkey' . $qc
             : $qc . $c->name . $qc
     );
 }

diff --git a/t/30sqlt-new-diff-pgsql.t b/t/30sqlt-new-diff-pgsql.t
index eb39f01..6e635cf 100644 (file)
--- a/t/30sqlt-new-diff-pgsql.t
+++ b/t/30sqlt-new-diff-pgsql.t
@@ -57,7 +57,7 @@ CREATE TABLE added (
 
 ALTER TABLE old_name RENAME TO new_name;
 
-ALTER TABLE employee DROP CONSTRAINT employee_employee_id_fkey;
+ALTER TABLE employee DROP CONSTRAINT FK5302D47D93FE702E;
 
 ALTER TABLE person DROP CONSTRAINT UC_age_name;
 
@@ -81,7 +81,7 @@ ALTER TABLE person RENAME COLUMN description TO physical_description;
 
 ALTER TABLE person ADD CONSTRAINT unique_name UNIQUE (name);
 
-ALTER TABLE employee ADD FOREIGN KEY (employee_id)
+ALTER TABLE employee ADD CONSTRAINT FK5302D47D93FE702E_diff FOREIGN KEY (employee_id)
   REFERENCES person (person_id) DEFERRABLE;
 
 ALTER TABLE person ADD CONSTRAINT UC_person_id UNIQUE (person_id);

diff --git a/t/47postgres-producer.t b/t/47postgres-producer.t
index ebb0888..447930f 100644 (file)
--- a/t/47postgres-producer.t
+++ b/t/47postgres-producer.t
@@ -14,7 +14,7 @@ use FindBin qw/$Bin/;
 #=============================================================================
 
 BEGIN {
-    maybe_plan(48,
+    maybe_plan(51,
         'SQL::Translator::Producer::PostgreSQL',
         'Test::Differences',
     )
@@ -90,23 +90,47 @@ my $field1_2 = SQL::Translator::Schema::Field->new( name => 'myfield_2',
                                                   is_foreign_key => 0,
                                                   is_unique => 0 );
 
-my $fk_constraint = SQL::Translator::Schema::Constraint->new(
-    table => $table,
-    name   => 'foo',
-    fields => [qw(myfield)],
-    type   => 'FOREIGN_KEY',
-    reference_table => $table2,
-    reference_fields => [qw(myfield_2)],
-);
+# check named, and unnamed foreign keys
+for my $name ( 'foo', undef ) {
+    my $fk_constraint = SQL::Translator::Schema::Constraint->new(
+        table => $table,
+        name   => $name,
+        fields => [qw(myfield)],
+        type   => 'FOREIGN_KEY',
+        reference_table => $table2,
+        reference_fields => [qw(myfield_2)],
+    );
+    my $fk_constraint_2 = SQL::Translator::Schema::Constraint->new(
+        table => $table,
+        name   => $name,
+        fields => [qw(myfield)],
+        type   => 'FOREIGN_KEY',
+        reference_table => $table2,
+        reference_fields => [qw(myfield_2)],
+    );
+
+    my  ($fk_constraint_def_ref, $fk_constraint_fk_ref ) = SQL::Translator::Producer::PostgreSQL::create_constraint($fk_constraint);
 
-my  ($fk_constraint_def_ref, $fk_constraint_fk_ref ) = SQL::Translator::Producer::PostgreSQL::create_constraint($fk_constraint);
+    ok(@{$fk_constraint_def_ref} == 0 && @{$fk_constraint_fk_ref} == 1,  'precheck of create_Foreign Key constraint');
 
-ok(@{$fk_constraint_def_ref} == 0 && @{$fk_constraint_fk_ref} == 1,  'precheck of create_Foreign Key constraint');
-is($fk_constraint_fk_ref->[0], 'ALTER TABLE mytable ADD FOREIGN KEY (myfield)
-  REFERENCES mytable2 (myfield_2) DEFERRABLE', 'Create Foreign Key Constraint works');
+    if ( $name ) {
+        is($fk_constraint_fk_ref->[0], "ALTER TABLE mytable ADD CONSTRAINT $name FOREIGN KEY (myfield)
+  REFERENCES mytable2 (myfield_2) DEFERRABLE", 'Create Foreign Key Constraint works');
+
+        # ToDo: may we should check if the constraint name was valid, or if next
+        #       unused_name created has choosen a different one
+        my $alter_fk_constraint = SQL::Translator::Producer::PostgreSQL::alter_drop_constraint($fk_constraint);
+        is($alter_fk_constraint, "ALTER TABLE mytable DROP CONSTRAINT $name", 'Alter drop Foreign Key constraint works');
+    }
+    else {
+        is($fk_constraint_fk_ref->[0], 'ALTER TABLE mytable ADD FOREIGN KEY (myfield)
+  REFERENCES mytable2 (myfield_2) DEFERRABLE', 'Create named Foreign Key Constraint works');
+
+        my $alter_fk_constraint = SQL::Translator::Producer::PostgreSQL::alter_drop_constraint($fk_constraint);
+        is($alter_fk_constraint, 'ALTER TABLE mytable DROP CONSTRAINT mytable_myfield_fkey', 'Alter drop named Foreign Key constraint works');
+    }
+}
 
-my $alter_fk_constraint = SQL::Translator::Producer::PostgreSQL::alter_drop_constraint($fk_constraint);
-is($alter_fk_constraint, 'ALTER TABLE mytable DROP CONSTRAINT mytable_myfield_fkey', 'Alter drop Foreign Key constraint works');
 
 my $alter_field = SQL::Translator::Producer::PostgreSQL::alter_field($field1,
                                                                 $field2);