PostgreSQL: add support for TRIGGERs and FUNCTIONs

[SPodjasek]

I was in need to support functions & triggers in schemas generated from DBIx::Class, so I started to add this features in this branch.

• [ ] fix t/46xml-to-pg.t with some clarification for procedure parameter sql
• [ ] fix t/60roundtrip.t
• [ ] fix CREATE FUNCTION statement from Schema object in diff - after clarifying sql meaning (IMHO it should contain complete SQL statement for creating function, every database has its own specs)

[KES777]

good work, man. That is the thing I was looking last few days.

PS. But I hardcode already triggers into upgrade/downgrade scripts (

[KES777]

I have found problem. DROP FUNCTION does not work

PostgreSQL requires parameters should be specified. This is copy/paste from create_procedure

git diff -b -w --ignore-blank-lines lib/SQL/Translator/Producer/PostgreSQL.pm
diff --git a/lib/SQL/Translator/Producer/PostgreSQL.pm b/lib/SQL/Translator/Producer/PostgreSQL.pm
index 9f8f727..d1de659 100644
--- a/lib/SQL/Translator/Producer/PostgreSQL.pm
+++ b/lib/SQL/Translator/Producer/PostgreSQL.pm
@@ -763,6 +763,16 @@ sub drop_procedure {
     my $generator = _generator($options);

     my $out = "DROP FUNCTION " . $generator->quote($procedure->name);
+    $out .= ' (';
+    my @args = ();
+    foreach my $arg (@{$procedure->parameters}) {
+    $arg = {name => $arg} if ref($arg) ne 'HASH';
+    push @args, join(' ', map $arg->{$_},
+                          grep defined($arg->{$_}),
+                          qw/argmode name type/);
+    }
+    $out .= join(', ', @args);
+    $out .= ')';

     return $out;
 }

[KES777]

The problem with function body. When it is created you use ->extra field but when calculate diff ->sql So functions with changed body are not updated in DB

I think here $procedure->sql should be updated by the generated $sql

[KES777]

--- a/lib/SQL/Translator/Diff.pm
+++ b/lib/SQL/Translator/Diff.pm
@@ -272,6 +272,8 @@ sub compute_differences {
       $src_procedure_name = lc $src_procedure_name if $self->case_insensitive;
       $src_procedures_checked{$src_procedure_name} = 1;

+     $producer_class->can('create_procedure')->( $src_procedure )   unless $src_procedure->sql;
+     $producer_class->can('create_procedure')->( $tgt_procedure )   unless $tgt_procedure->sql;
       # Compare SQL in procedure declaration
       next unless $src_procedure->sql ne $tgt_procedure->sql;
       push @{$self->procedures_to_modify}, $tgt_procedure;
--- a/lib/SQL/Translator/Producer/PostgreSQL.pm
+++ b/lib/SQL/Translator/Producer/PostgreSQL.pm
@@ -754,6 +754,8 @@ sub create_procedure {
   }

   push @statements, $sql;
+  $procedure->sql( $sql );

   return @statements;
 }

[KES777]

Just harcoding DROP FUNCTION does not help. We should reuse code

diff --git a/lib/SQL/Translator/Producer/PostgreSQL.pm b/lib/SQL/Translator/Producer/PostgreSQL.pm
index ebe39e4..298e0cb 100644
--- a/lib/SQL/Translator/Producer/PostgreSQL.pm
+++ b/lib/SQL/Translator/Producer/PostgreSQL.pm
@@ -713,7 +713,7 @@ sub create_procedure {

   my @statements;

-  push @statements, sprintf( 'DROP FUNCTION IF EXISTS %s', $generator->quote($procedure->name) )
+  push @statements, drop_procedure( $procedure )
     if $options->{add_drop_procedure};

   my $sql = 'CREATE FUNCTION ';

otherwise we get: ERROR: syntax error at end of input

LINE 1: DROP FUNCTION IF EXISTS "ch_saldo"

[SPodjasek]

@KES777 applied all above patches in 64d744a

[KES777]

Also I have changed this:

--- a/lib/SQL/Translator/Producer/PostgreSQL.pm
+++ b/lib/SQL/Translator/Producer/PostgreSQL.pm
@@ -595,12 +595,10 @@ sub create_index
     if ( $type eq PRIMARY_KEY ) {
         push @constraint_defs, "${def_start}PRIMARY KEY ".$field_names;
     }
-    elsif ( $type eq UNIQUE ) {
-        push @constraint_defs, "${def_start}UNIQUE " .$field_names;
-    }
-    elsif ( $type eq NORMAL ) {
+    elsif ( $type eq NORMAL  ||  $type eq UNIQUE ) {
+        my $unique =  $type eq UNIQUE ? 'UNIQUE' : '';
         $index_def =
-            'CREATE INDEX ' . $generator->quote($name) . ' on ' . $generator->quote($table_name) . ' ' .
+            "CREATE $unique INDEX " . $generator->quote($name) . ' on ' . $generator->quote($table_name) . ' ' .
             join ' ', grep { defined } $index_using, $field_names, $index_where;
     }
     else {

This allows me create UNIQUE INDEX instead of constraint. I think this is better.

[KES777]

I was wrong about UNIQUE INDEX (prof):

The preferred way to add a unique constraint to a table is ALTER TABLE … ADD CONSTRAINT. The use of indexes to enforce unique constraints could be considered an implementation detail that should not be accessed directly

Also:

The uniqueness property is a constraint and so a "unique index" without a corresponding constraint is an improper model

[yewtc]

wrt #82. The procedures need to be added first, before the tables. I have a situation where a procedure is used for the default of a column and the deploy fails because it is not defined. Oddly you can define procedures that refer to tables and columns that don't yet exist. So doing it first 'just works'.