Skip to content

Commit

Permalink
improve add_trigger consistency between producers
Browse files Browse the repository at this point in the history
Update Producer::SQLite and Producer::MySQL to only wrap the trigger
action in "BEGIN...END" when the user has not already done so, bringing
them in line with other producers and the add_trigger documentation.

Fixes dbsrgits#46.

Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com>
  • Loading branch information
andrewgregory committed Dec 14, 2014
1 parent 2491a46 commit 516e36b
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 4 deletions.
7 changes: 5 additions & 2 deletions lib/SQL/Translator/Producer/MySQL.pm
Original file line number Diff line number Diff line change
Expand Up @@ -356,11 +356,14 @@ sub create_trigger {
}

my $action = $trigger->action;
$action .= ";" unless $action =~ /;\s*\z/;
if($action !~ /^\s*BEGIN\s.*\sEND\s*$/i) {
$action .= ";" unless $action =~ /;\s*\z/;
$action = "BEGIN $action END";
}

push @statements, "DROP TRIGGER IF EXISTS " . $generator->quote($name) if $options->{add_drop_trigger};
push @statements, sprintf(
"CREATE TRIGGER %s %s %s ON %s\n FOR EACH ROW BEGIN %s END",
"CREATE TRIGGER %s %s %s ON %s\n FOR EACH ROW %s",
$generator->quote($name), $trigger->perform_action_when, $event,
$generator->quote($trigger->on_table), $action,
);
Expand Down
4 changes: 3 additions & 1 deletion lib/SQL/Translator/Producer/SQLite.pm
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,9 @@ sub create_trigger {
$DB::single = 1;
my $action = "";
if (not ref $trigger->action) {
$action .= "BEGIN " . $trigger->action . " END";
$action = $trigger->action;
$action = "BEGIN " . $action . " END"
unless $action =~ /^\s*BEGIN\s.*\sEND$/i;
}
else {
$action = $trigger->action->{for_each} . " "
Expand Down
45 changes: 44 additions & 1 deletion t/38-mysql-producer.t
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use FindBin qw/$Bin/;
#=============================================================================

BEGIN {
maybe_plan(73,
maybe_plan(75,
'YAML',
'SQL::Translator::Producer::MySQL',
'Test::Differences',
Expand Down Expand Up @@ -800,3 +800,46 @@ EOV
is(SQL::Translator::Producer::MySQL::alter_drop_constraint($constraint,$options),
'ALTER TABLE `table` DROP PRIMARY KEY','valid drop primary key');
}

{
my $schema = SQL::Translator::Schema->new();
my $table = $schema->add_table( name => 'foo', fields => ['bar'] );

{
my $trigger = $schema->add_trigger(
name => 'mytrigger',
perform_action_when => 'before',
database_events => 'update',
on_table => 'foo',
fields => ['bar'],
action => 'BEGIN baz(); END'
);
my ($def) = SQL::Translator::Producer::MySQL::create_trigger($trigger);
my $expected
= "--\n"
. "-- Trigger mytrigger\n"
. "--\n"
. "CREATE TRIGGER mytrigger before update ON foo\n"
. " FOR EACH ROW BEGIN baz(); END";
is($def, $expected, 'trigger created');
}

{
my $trigger = $schema->add_trigger(
name => 'mytrigger2',
perform_action_when => 'after',
database_events => ['insert'],
on_table => 'foo',
fields => ['bar'],
action => 'baz()'
);
my ($def) = SQL::Translator::Producer::MySQL::create_trigger($trigger);
my $expected
= "--\n"
. "-- Trigger mytrigger2\n"
. "--\n"
. "CREATE TRIGGER mytrigger2 after insert ON foo\n"
. " FOR EACH ROW BEGIN baz(); END";
is($def, $expected, 'trigger created');
}
}
32 changes: 32 additions & 0 deletions t/56-sqlite-producer.t
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use strict;
use Test::More;
use Test::SQL::Translator qw(maybe_plan);

use SQL::Translator::Schema;
use SQL::Translator::Schema::View;
use SQL::Translator::Schema::Table;
use SQL::Translator::Producer::SQLite;
Expand Down Expand Up @@ -192,4 +193,35 @@ $SQL::Translator::Producer::SQLite::NO_QUOTES = 0;
}
}

{
my $schema = SQL::Translator::Schema->new();
my $table = $schema->add_table( name => 'foo', fields => ['bar'] );

{
my $trigger = $schema->add_trigger(
name => 'mytrigger',
perform_action_when => 'before',
database_events => 'update',
on_table => 'foo',
fields => ['bar'],
action => 'BEGIN baz() END'
);
my ($def) = SQL::Translator::Producer::SQLite::create_trigger($trigger);
is($def, 'CREATE TRIGGER "mytrigger" before update on "foo" BEGIN baz() END', 'trigger created');
}

{
my $trigger = $schema->add_trigger(
name => 'mytrigger2',
perform_action_when => 'after',
database_events => ['insert'],
on_table => 'foo',
fields => ['bar'],
action => 'baz()'
);
my ($def) = SQL::Translator::Producer::SQLite::create_trigger($trigger);
is($def, 'CREATE TRIGGER "mytrigger2" after insert on "foo" BEGIN baz() END', 'trigger created');
}
}

done_testing;

1 comment on commit 516e36b

@ribasushi
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewgregory
Sorry for not getting to your pull request earlier. It looks good to me, however I wonder if the regexes should change to /^ \s* BEGIN \s .* \s END $/mix, so that .* will match multiline triggers.

Please let me know if there is a problem with that plan.
Cheers!

Please sign in to comment.