Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PostgreSQL: add support for TRIGGERs and FUNCTIONs #82

Open
3 tasks
SPodjasek opened this issue Jun 2, 2016 · 9 comments
Open
3 tasks

PostgreSQL: add support for TRIGGERs and FUNCTIONs #82

SPodjasek opened this issue Jun 2, 2016 · 9 comments

Comments

@SPodjasek
Copy link
Contributor

SPodjasek commented Jun 2, 2016

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
Copy link
Contributor

KES777 commented Jan 19, 2017

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

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

@KES777
Copy link
Contributor

KES777 commented Jan 21, 2017

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
Copy link
Contributor

KES777 commented Jan 26, 2017

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
Copy link
Contributor

KES777 commented Jan 26, 2017

--- 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
Copy link
Contributor

KES777 commented Jan 26, 2017

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 added a commit to SPodjasek/sql-translator that referenced this issue Jan 30, 2017
@SPodjasek
Copy link
Contributor Author

@KES777 applied all above patches in 64d744a

@KES777
Copy link
Contributor

KES777 commented Jan 31, 2017

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
Copy link
Contributor

KES777 commented Jul 11, 2018

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
Copy link

yewtc commented Mar 5, 2019

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'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants