From ca601dbbeb7150ac99a0eb9a9ca20d13e26a08da Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Wed, 10 Aug 2022 11:10:48 -0400 Subject: [PATCH 01/19] Added support for CURRENT_TIMESTAMP as a default value when parsing Oracle SQL. --- lib/SQL/Translator/Parser/Oracle.pm | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/SQL/Translator/Parser/Oracle.pm b/lib/SQL/Translator/Parser/Oracle.pm index 66a885c7..388452b7 100644 --- a/lib/SQL/Translator/Parser/Oracle.pm +++ b/lib/SQL/Translator/Parser/Oracle.pm @@ -489,7 +489,17 @@ parens_name_list : '(' NAME(s /,/) ')' field_meta : default_val | column_constraint -default_val : /default/i VALUE +default_val : + /default/i CURRENT_TIMESTAMP + { + my $val = $item[2]; + $return = { + supertype => 'constraint', + type => 'default', + value => $val, + } + } + | /default/i VALUE { my $val = $item[2]; $return = { @@ -616,6 +626,11 @@ VALUE : /[-+]?\d*\.?\d+(?:[eE]\d+)?/ | /null/i { 'NULL' } +# always a scalar-ref, so that it is treated as a function and not quoted by consumers +CURRENT_TIMESTAMP : + /current_timestamp(\(\))?/i { \'CURRENT_TIMESTAMP' } + | /now\(\)/i { \'CURRENT_TIMESTAMP' } + END_OF_GRAMMAR sub parse { From 0d8ce8f2703e11cb0201a6cbf0e1485950c305c1 Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Wed, 10 Aug 2022 12:54:43 -0400 Subject: [PATCH 02/19] Added tests to verify changes to oracle parser. --- t/15oracle-parser.t | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/t/15oracle-parser.t b/t/15oracle-parser.t index 8cae44a7..ee610fb3 100644 --- a/t/15oracle-parser.t +++ b/t/15oracle-parser.t @@ -7,7 +7,7 @@ use SQL::Translator; use SQL::Translator::Schema::Constants; use Test::SQL::Translator qw(maybe_plan); -maybe_plan(99, 'SQL::Translator::Parser::Oracle'); +maybe_plan(103, 'SQL::Translator::Parser::Oracle'); SQL::Translator::Parser::Oracle->import('parse'); my $t = SQL::Translator->new( trace => 0 ); @@ -34,6 +34,7 @@ my $sql = q[ trait_symbol VARCHAR2(100 BYTE) NOT NULL, trait_name VARCHAR2(200 CHAR) NOT NULL, qtl_trait_category_id NUMBER(11) NOT NULL, + trait_date DATE DEFAULT CURRENT_TIMESTAMP NOT NULL, UNIQUE ( trait_symbol ), UNIQUE ( trait_name ), FOREIGN KEY ( qtl_trait_category_id ) REFERENCES qtl_trait_category @@ -179,7 +180,7 @@ is( $t2->name, 'qtl_trait', 'Table "qtl_trait" exists' ); is( $t2->comments, 'foo bar comment', 'Comment "foo bar" exists' ); my @t2_fields = $t2->get_fields; -is( scalar @t2_fields, 4, '4 fields in table' ); +is( scalar @t2_fields, 5, '5 fields in table' ); my $t2_f1 = shift @t2_fields; is( $t2_f1->name, 'qtl_trait_id', 'First field is "qtl_trait_id"' ); @@ -217,6 +218,16 @@ is( $f4_fk->reference_table, 'qtl_trait_category', is( join(',', $f4_fk->reference_fields), 'qtl_trait_category_id', 'FK references field "qtl_trait_category_id"' ); +my $t2_f5 = shift @t2_fields; +is( $t2_f5->name, 'trait_date', 'Fifth field is "trait_date"'); +is( $t2_f5->data_type, 'DATE', 'Field is a timestamp' ); +is( $t2_f5->is_nullable, 0, 'Field cannot be null' ); +is_deeply( + $t2_f5->default_value, + \'CURRENT_TIMESTAMP', + 'Field has correct default value' +); + my @t2_constraints = $t2->get_constraints; is( scalar @t2_constraints, 4, '4 constraints on table' ); From dbd1da135552f60464019a2bef10ee0d52e1d51a Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Thu, 18 Aug 2022 13:15:03 -0400 Subject: [PATCH 03/19] Split the creation of contraints out from the creation of the table and added alter_create_constraint and alter_drop_constraint functions. --- lib/SQL/Translator/Producer/Oracle.pm | 150 +++++++++++++++++--------- 1 file changed, 97 insertions(+), 53 deletions(-) diff --git a/lib/SQL/Translator/Producer/Oracle.pm b/lib/SQL/Translator/Producer/Oracle.pm index 017f41a2..ef61ff38 100644 --- a/lib/SQL/Translator/Producer/Oracle.pm +++ b/lib/SQL/Translator/Producer/Oracle.pm @@ -307,59 +307,10 @@ sub create_table { # Table constraints # for my $c ( $table->get_constraints ) { - my $name = $c->name || ''; - my @fields = map { quote($_,$qf) } $c->fields; - my @rfields = map { quote($_,$qf) } $c->reference_fields; - - next if !@fields && $c->type ne CHECK_C; - - if ( $c->type eq PRIMARY_KEY ) { - # create a name if delay_constraints - $name ||= mk_name( $table_name, 'pk' ) - if $options->{delay_constraints}; - $name = quote($name,$qf); - push @constraint_defs, ($name ? "CONSTRAINT $name " : '') . - 'PRIMARY KEY (' . join( ', ', @fields ) . ')'; - } - elsif ( $c->type eq UNIQUE ) { - # Don't create UNIQUE constraints identical to the primary key - if ( my $pk = $table->primary_key ) { - my $u_fields = join(":", @fields); - my $pk_fields = join(":", $pk->fields); - next if $u_fields eq $pk_fields; - } - - if ($name) { - # Force prepend of table_name as ORACLE doesn't allow duplicate - # CONSTRAINT names even for different tables (ORA-02264) - $name = mk_name( "${table_name}_$name", 'u' ) unless $name =~ /^$table_name/; - } - else { - $name = mk_name( $table_name, 'u' ); - } - - $name = quote($name, $qf); - - for my $f ( $c->fields ) { - my $field_def = $table->get_field( $f ) or next; - my $dtype = $translate{ ref $field_def->data_type eq "ARRAY" ? $field_def->data_type->[0] : $field_def->data_type} or next; - if ( $WARN && $dtype =~ /clob/i ) { - warn "Oracle will not allow UNIQUE constraints on " . - "CLOB field '" . $field_def->table->name . '.' . - $field_def->name . ".'\n" - } - } - - push @constraint_defs, "CONSTRAINT $name UNIQUE " . - '(' . join( ', ', @fields ) . ')'; - } - elsif ( $c->type eq CHECK_C ) { - $name ||= mk_name( $name || $table_name, 'ck' ); - $name = quote($name, $qf); - my $expression = $c->expression || ''; - push @constraint_defs, "CONSTRAINT $name CHECK ($expression)"; - } - elsif ( $c->type eq FOREIGN_KEY ) { + my $constr = create_constraint($c, $options); + push @constraint_defs, $constr if ($constr); + + if ( $c->type eq FOREIGN_KEY ) { $name = mk_name( join('_', $table_name, $c->fields). '_fk' ); $name = quote($name, $qf); my $on_delete = uc ($c->on_delete || ''); @@ -727,6 +678,99 @@ sub create_field { } +sub alter_drop_constraint { + my ($c, $options) = @_; + + my $generator = _generator($options); + my $table_name = $generator->quote($c->table->name); + + my @out = ('ALTER','TABLE',$table_name,'DROP'); + if($c->type eq PRIMARY_KEY) { + push @out, $c->type; + } + else { + push @out, ($c->type eq FOREIGN_KEY ? $c->type : "CONSTRAINT"), + $generator->quote($c->name); + } + return join(' ',@out); +} + +sub alter_create_constraint { + my ($index, $options) = @_; + + my $table_name = _generator($options)->quote($index->table->name); + return join( ' ', + 'ALTER TABLE', + $table_name, + 'ADD', + create_constraint(@_) ); +} + +sub create_constraint { + my ($c, $options) = @_; + + my $qt = $options->{quote_table_names}; + my $qf = $options->{quote_field_names}; + my $table = $c->table; + my $table_name = $table->name; + my $table_name_q = quote($table_name,$qt); + my $name = $c->name || ''; + my @fields = map { quote($_,$qf) } $c->fields; + my @rfields = map { quote($_,$qf) } $c->reference_fields; + + return undef if !@fields && $c->type ne 'CHECK'; + + my $definition; + + if ( $c->type eq PRIMARY_KEY ) { + # create a name if delay_constraints + $name ||= mk_name( $table_name, 'pk' ) + if $options->{delay_constraints}; + $name = quote($name,$qf); + $definition = ($name ? "CONSTRAINT $name " : '') . + 'PRIMARY KEY (' . join( ', ', @fields ) . ')'; + } + elsif ( $c->type eq UNIQUE ) { + # Don't create UNIQUE constraints identical to the primary key + if ( my $pk = $table->primary_key ) { + my $u_fields = join(":", @fields); + my $pk_fields = join(":", $pk->fields); + next if $u_fields eq $pk_fields; + } + + if ($name) { + # Force prepend of table_name as ORACLE doesn't allow duplicate + # CONSTRAINT names even for different tables (ORA-02264) + $name = mk_name( "${table_name}_$name", 'u' ) unless $name =~ /^$table_name/; + } + else { + $name = mk_name( $table_name, 'u' ); + } + + $name = quote($name, $qf); + + for my $f ( $c->fields ) { + my $field_def = $table->get_field( $f ) or next; + my $dtype = $translate{ ref $field_def->data_type eq "ARRAY" ? $field_def->data_type->[0] : $field_def->data_type} or next; + if ( $WARN && $dtype =~ /clob/i ) { + warn "Oracle will not allow UNIQUE constraints on " . + "CLOB field '" . $field_def->table->name . '.' . + $field_def->name . ".'\n" + } + } + + $definition = "CONSTRAINT $name UNIQUE " . + '(' . join( ', ', @fields ) . ')'; + } + elsif ( $c->type eq CHECK_C ) { + $name ||= mk_name( $name || $table_name, 'ck' ); + $name = quote($name, $qf); + my $expression = $c->expression || ''; + $definition = "CONSTRAINT $name CHECK ($expression)"; + } + + return $definition ? $definition : undef; +} sub create_view { my ($view, $options) = @_; From 929ae338b01d4a6857c17428c03a671cdd4d6d6c Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Thu, 18 Aug 2022 13:20:49 -0400 Subject: [PATCH 04/19] Missing a few small things --- lib/SQL/Translator/Producer/Oracle.pm | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/SQL/Translator/Producer/Oracle.pm b/lib/SQL/Translator/Producer/Oracle.pm index ef61ff38..0ceacc82 100644 --- a/lib/SQL/Translator/Producer/Oracle.pm +++ b/lib/SQL/Translator/Producer/Oracle.pm @@ -310,6 +310,10 @@ sub create_table { my $constr = create_constraint($c, $options); push @constraint_defs, $constr if ($constr); + my $name = $c->name || ''; + my @fields = map { quote($_,$qf) } $c->fields; + my @rfields = map { quote($_,$qf) } $c->reference_fields; + if ( $c->type eq FOREIGN_KEY ) { $name = mk_name( join('_', $table_name, $c->fields). '_fk' ); $name = quote($name, $qf); From ed59a4dd418e95b504f476819b75b73166e78d45 Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Fri, 19 Aug 2022 12:27:55 -0400 Subject: [PATCH 05/19] Added drop_field to Oracle.pm and added alter constraint testing. --- lib/SQL/Translator/Producer/Oracle.pm | 20 ++++- t/54-oracle-alter-constraint.t | 55 ++++++++++++++ ...add-field.t => 55-oracle-add-drop-field.t} | 7 +- t/data/oracle/schema_diff_b.yaml | 11 +++ t/data/oracle/schema_diff_d.yaml | 73 +++++++++++++++++++ t/data/oracle/schema_diff_e.yaml | 70 ++++++++++++++++++ 6 files changed, 229 insertions(+), 7 deletions(-) create mode 100644 t/54-oracle-alter-constraint.t rename t/{55-oracle-add-field.t => 55-oracle-add-drop-field.t} (84%) create mode 100644 t/data/oracle/schema_diff_d.yaml create mode 100644 t/data/oracle/schema_diff_e.yaml diff --git a/lib/SQL/Translator/Producer/Oracle.pm b/lib/SQL/Translator/Producer/Oracle.pm index 0ceacc82..15dc0ac0 100644 --- a/lib/SQL/Translator/Producer/Oracle.pm +++ b/lib/SQL/Translator/Producer/Oracle.pm @@ -462,6 +462,19 @@ sub alter_field { return 'ALTER TABLE '.$table_name.' MODIFY ( '.join('', @$field_defs).' )'; } +sub drop_field +{ + my ($old_field, $options) = @_; + + my $table_name = quote($old_field->table->name); + + my $out = sprintf('ALTER TABLE %s DROP COLUMN %s', + $table_name, + quote($old_field->name)); + + return $out; +} + sub add_field { my ($new_field, $options) = @_; @@ -685,8 +698,7 @@ sub create_field { sub alter_drop_constraint { my ($c, $options) = @_; - my $generator = _generator($options); - my $table_name = $generator->quote($c->table->name); + my $table_name = quote($c->table->name); my @out = ('ALTER','TABLE',$table_name,'DROP'); if($c->type eq PRIMARY_KEY) { @@ -694,7 +706,7 @@ sub alter_drop_constraint { } else { push @out, ($c->type eq FOREIGN_KEY ? $c->type : "CONSTRAINT"), - $generator->quote($c->name); + quote($c->name); } return join(' ',@out); } @@ -702,7 +714,7 @@ sub alter_drop_constraint { sub alter_create_constraint { my ($index, $options) = @_; - my $table_name = _generator($options)->quote($index->table->name); + my $table_name = quote($index->table->name); return join( ' ', 'ALTER TABLE', $table_name, diff --git a/t/54-oracle-alter-constraint.t b/t/54-oracle-alter-constraint.t new file mode 100644 index 00000000..4fad7abd --- /dev/null +++ b/t/54-oracle-alter-constraint.t @@ -0,0 +1,55 @@ +#!/usr/bin/perl + +use FindBin qw/$Bin/; +use Test::More; +use Test::SQL::Translator; +use Test::Exception; +use Data::Dumper; +use SQL::Translator; +use SQL::Translator::Diff; + +BEGIN { + maybe_plan(4, 'SQL::Translator::Parser::YAML', + 'SQL::Translator::Producer::Oracle'); +} + +my $schema1 = $Bin.'/data/oracle/schema_diff_d.yaml'; +my $schema2 = $Bin.'/data/oracle/schema_diff_e.yaml'; + +open my $io1, '<', $schema1 or die $!; +open my $io2, '<', $schema2 or die $!; + +my ($yaml1, $yaml2); +{ + local $/ = undef; + $yaml1 = <$io1>; + $yaml2 = <$io2>; +}; + +close $io1; +close $io2; + +my $s = SQL::Translator->new(from => 'YAML'); +$s->parser->($s,$yaml1); + +my $t = SQL::Translator->new(from => 'YAML'); +$t->parser->($t,$yaml2); + +my $d = SQL::Translator::Diff->new + ({ + output_db => 'Oracle', + target_db => 'Oracle', + source_schema => $s->schema, + target_schema => $t->schema, + }); + + +my $diff = $d->compute_differences->produce_diff_sql || die $d->error; + +ok($diff, 'Diff generated.'); + +like($diff, '/ALTER TABLE d_operator DROP CONSTRAINT foo_unique/', 'DROP constraint foo_unique generated'); + +like($diff, '/ALTER TABLE d_operator DROP CONSTRAINT other_check/', 'DROP constraint other_check generated'); + +like($diff, '/ADD CONSTRAINT other_check CHECK \(other BETWEEN 100 and 99999\)/', 'ADD check constraint generated'); diff --git a/t/55-oracle-add-field.t b/t/55-oracle-add-drop-field.t similarity index 84% rename from t/55-oracle-add-field.t rename to t/55-oracle-add-drop-field.t index e4e2535e..b7f74f2b 100644 --- a/t/55-oracle-add-field.t +++ b/t/55-oracle-add-drop-field.t @@ -9,7 +9,7 @@ use SQL::Translator; use SQL::Translator::Diff; BEGIN { - maybe_plan(2, 'SQL::Translator::Parser::YAML', + maybe_plan(3, 'SQL::Translator::Parser::YAML', 'SQL::Translator::Producer::Oracle'); } @@ -46,5 +46,6 @@ my $d = SQL::Translator::Diff->new my $diff = $d->compute_differences->produce_diff_sql || die $d->error; ok($diff, 'Diff generated.'); -like($diff, '/ALTER TABLE d_operator ADD \( foo nvarchar2\(10\) NOT NULL \)/', - 'Alter table generated.'); + +like($diff, '/ALTER TABLE d_operator DROP COLUMN bar/', 'DROP column generated.'); +like($diff, '/ALTER TABLE d_operator ADD \( foo nvarchar2\(10\) NOT NULL \)/', 'ADD column generated.'); diff --git a/t/data/oracle/schema_diff_b.yaml b/t/data/oracle/schema_diff_b.yaml index 42f71ba1..3c6b7a2f 100644 --- a/t/data/oracle/schema_diff_b.yaml +++ b/t/data/oracle/schema_diff_b.yaml @@ -29,6 +29,17 @@ schema: order: 58 size: - 0 + bar: + data_type: varchar2 + default_value: ~ + extra: {} + is_nullable: 1 + is_primary_key: 0 + is_unique: 0 + name: bar + order: 61 + size: + - 3 name: data_type: nvarchar2 default_value: ~ diff --git a/t/data/oracle/schema_diff_d.yaml b/t/data/oracle/schema_diff_d.yaml new file mode 100644 index 00000000..400ddd5d --- /dev/null +++ b/t/data/oracle/schema_diff_d.yaml @@ -0,0 +1,73 @@ +--- +schema: + procedures: {} + tables: + d_operator: + constraints: + - deferrable: 1 + expression: '' + fields: + - id_operator + match_type: '' + name: '' + on_delete: '' + on_update: '' + options: [] + reference_fields: [] + reference_table: '' + type: PRIMARY KEY + - fields: foo + name: 'foo_unique' + type: UNIQUE + - fields: other + name: 'other_check' + type: CHECK + expression: other BETWEEN 1 and 99999 + fields: + id_operator: + data_type: integer + default_value: ~ + extra: {} + is_auto_increment: 1 + is_nullable: 0 + is_primary_key: 1 + is_unique: 0 + name: id_operator + order: 58 + size: + - 0 + name: + data_type: nvarchar2 + default_value: ~ + extra: {} + is_nullable: 0 + is_primary_key: 0 + is_unique: 0 + name: name + order: 59 + size: + - 10 + foo: + data_type: nvarchar2 + default_value: ~ + extra: {} + is_nullable: 0 + is_primary_key: 0 + is_unique: 1 + name: foo + order: 60 + size: + - 10 + other: + data_type: integer + default_value: ~ + extra: {} + is_nullable: 0 + is_primary_key: 0 + is_unique: 1 + name: other + order: 61 + size: + - 5 + name: d_operator + order: 11 diff --git a/t/data/oracle/schema_diff_e.yaml b/t/data/oracle/schema_diff_e.yaml new file mode 100644 index 00000000..9cc31901 --- /dev/null +++ b/t/data/oracle/schema_diff_e.yaml @@ -0,0 +1,70 @@ +--- +schema: + procedures: {} + tables: + d_operator: + constraints: + - deferrable: 1 + expression: '' + fields: + - id_operator + match_type: '' + name: '' + on_delete: '' + on_update: '' + options: [] + reference_fields: [] + reference_table: '' + type: PRIMARY KEY + - fields: other + name: 'other_check' + type: CHECK + expression: other BETWEEN 100 and 99999 + fields: + id_operator: + data_type: integer + default_value: ~ + extra: {} + is_auto_increment: 1 + is_nullable: 0 + is_primary_key: 1 + is_unique: 0 + name: id_operator + order: 58 + size: + - 0 + name: + data_type: nvarchar2 + default_value: ~ + extra: {} + is_nullable: 0 + is_primary_key: 0 + is_unique: 0 + name: name + order: 59 + size: + - 10 + foo: + data_type: nvarchar2 + default_value: ~ + extra: {} + is_nullable: 0 + is_primary_key: 0 + is_unique: 0 + name: foo + order: 60 + size: + - 10 + other: + data_type: integer + default_value: ~ + extra: {} + is_nullable: 0 + is_primary_key: 0 + is_unique: 1 + name: other + order: 61 + size: + - 5 + name: d_operator + order: 11 From cdb544f4acf394a23991e0e4f7ccfcd93013e2d2 Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Mon, 22 Aug 2022 13:35:23 -0400 Subject: [PATCH 06/19] Added drop_table functionality along with related tests. --- lib/SQL/Translator/Producer/Oracle.pm | 69 ++++++++++++++++++++++++++- t/54-oracle-alter-constraint.t | 8 +++- t/data/oracle/schema_diff_d.yaml | 49 +++++++++++++++++++ t/data/oracle/schema_diff_e.yaml | 24 ++++++++++ 4 files changed, 147 insertions(+), 3 deletions(-) diff --git a/lib/SQL/Translator/Producer/Oracle.pm b/lib/SQL/Translator/Producer/Oracle.pm index 15dc0ac0..97f65de7 100644 --- a/lib/SQL/Translator/Producer/Oracle.pm +++ b/lib/SQL/Translator/Producer/Oracle.pm @@ -95,7 +95,7 @@ $DEBUG = 0 unless defined $DEBUG; use base 'SQL::Translator::Producer'; use SQL::Translator::Schema::Constants; -use SQL::Translator::Utils qw(header_comment); +use SQL::Translator::Utils qw(header_comment batch_alter_table_statements); my %translate = ( # @@ -695,6 +695,73 @@ sub create_field { } +sub batch_alter_table { + my ($table, $diff_hash, $options) = @_; + + my %fks_to_alter; + my %fks_to_drop = map { + $_->type eq FOREIGN_KEY + ? ( $_->name => $_ ) + : ( ) + } @{$diff_hash->{alter_drop_constraint} }; + + my %fks_to_create = map { + if ( $_->type eq FOREIGN_KEY) { + $fks_to_alter{$_->name} = $fks_to_drop{$_->name} if $fks_to_drop{$_->name}; + ( $_->name => $_ ); + } else { ( ) } + } @{$diff_hash->{alter_create_constraint} }; + + my @drop_stmt; + if (scalar keys %fks_to_alter) { + $diff_hash->{alter_drop_constraint} = [ + grep { !$fks_to_alter{$_->name} } @{ $diff_hash->{alter_drop_constraint} } + ]; + + @drop_stmt = batch_alter_table($table, { alter_drop_constraint => [ values %fks_to_alter ] }, $options); + + } + + my @stmts = batch_alter_table_statements($diff_hash, $options); + + # rename_table makes things a bit more complex + my $renamed_from = ""; + $renamed_from = quote($diff_hash->{rename_table}[0][0]->name) + if $diff_hash->{rename_table} && @{$diff_hash->{rename_table}}; + + return unless @stmts; + # Just zero or one stmts. return now + return (@drop_stmt,@stmts) unless @stmts > 1; + + # Now strip off the 'ALTER TABLE xyz' of all but the first one + + my $table_name = quote($table->name); + + my $re = $renamed_from + ? qr/^ALTER TABLE (?:\Q$table_name\E|\Q$renamed_from\E) / + : qr/^ALTER TABLE \Q$table_name\E /; + + my $first = shift @stmts; + my ($alter_table) = $first =~ /($re)/; + + my $padd = " " x length($alter_table); + + return @drop_stmt, join( ",\n", $first, map { s/$re//; $padd . $_ } @stmts); + +} + +sub drop_table { + my ($table, $options) = @_; + + return ( + # Drop (foreign key) constraints so table drops cleanly + batch_alter_table( + $table, { alter_drop_constraint => [ grep { $_->type eq 'FOREIGN KEY' } $table->get_constraints ] }, $options + ), + 'DROP TABLE ' . quote($table), + ); +} + sub alter_drop_constraint { my ($c, $options) = @_; diff --git a/t/54-oracle-alter-constraint.t b/t/54-oracle-alter-constraint.t index 4fad7abd..eeac1bc4 100644 --- a/t/54-oracle-alter-constraint.t +++ b/t/54-oracle-alter-constraint.t @@ -9,7 +9,7 @@ use SQL::Translator; use SQL::Translator::Diff; BEGIN { - maybe_plan(4, 'SQL::Translator::Parser::YAML', + maybe_plan(6, 'SQL::Translator::Parser::YAML', 'SQL::Translator::Producer::Oracle'); } @@ -50,6 +50,10 @@ ok($diff, 'Diff generated.'); like($diff, '/ALTER TABLE d_operator DROP CONSTRAINT foo_unique/', 'DROP constraint foo_unique generated'); -like($diff, '/ALTER TABLE d_operator DROP CONSTRAINT other_check/', 'DROP constraint other_check generated'); +like($diff, '/DROP CONSTRAINT other_check/', 'DROP constraint other_check generated'); like($diff, '/ADD CONSTRAINT other_check CHECK \(other BETWEEN 100 and 99999\)/', 'ADD check constraint generated'); + +like($diff, '/ALTER TABLE supplier DROP FOREIGN KEY fk_customer/', 'DROP Foreign key constraint generated'); + +like($diff, '/DROP TABLE customer/', 'DROP TABLE customer generated'); \ No newline at end of file diff --git a/t/data/oracle/schema_diff_d.yaml b/t/data/oracle/schema_diff_d.yaml index 400ddd5d..e73e578d 100644 --- a/t/data/oracle/schema_diff_d.yaml +++ b/t/data/oracle/schema_diff_d.yaml @@ -71,3 +71,52 @@ schema: - 5 name: d_operator order: 11 + supplier: + constraints: + - fields: cust_id + name: fk_customer + reference_table: customer + reference_fields: + - customer_id + type: FOREIGN KEY + fields: + id: + data_type: integer + is_nullable: 0 + is_primary_key: 1 + size: 11 + name: id + order: 62 + cust_id: + data_type: integer + is_nullable: 1 + is_primary_key: 0 + size: 11 + name: cust_id + order: 63 + supplier_name: + data_type: nvarchar2 + is_nullable: 0 + is_primary_key: 0 + size: 256 + name: supplier_name + order: 64 + name: supplier + customer: + fields: + customer_id: + data_type: integer + is_nullable: 0 + is_primary_key: 1 + size: 11 + name: customer_id + order: 65 + customer_name: + data_type: nvarchar2 + is_nullable: 0 + is_primary_key: 0 + size: 256 + name: customer_name + order: 66 + name: customer + order: 12 \ No newline at end of file diff --git a/t/data/oracle/schema_diff_e.yaml b/t/data/oracle/schema_diff_e.yaml index 9cc31901..a262c33f 100644 --- a/t/data/oracle/schema_diff_e.yaml +++ b/t/data/oracle/schema_diff_e.yaml @@ -68,3 +68,27 @@ schema: - 5 name: d_operator order: 11 + supplier: + fields: + id: + data_type: integer + is_nullable: 0 + is_primary_key: 1 + size: 11 + name: id + order: 62 + cust_id: + data_type: integer + is_nullable: 1 + is_primary_key: 0 + size: 11 + name: cust_id + order: 63 + supplier_name: + data_type: nvarchar2 + is_nullable: 0 + is_primary_key: 0 + size: 256 + name: supplier_name + order: 65 + name: supplier From 22a15de6fc368a505623cf5eb08e9ea3f8c99eba Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Wed, 24 Aug 2022 10:35:18 -0400 Subject: [PATCH 07/19] Make sure the sqlt_args actually get passed in to the SQL::Translator::Diff object. Can be an issue if SQL::Translator::Diff::schema_diff is called directly. --- lib/SQL/Translator/Diff.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/SQL/Translator/Diff.pm b/lib/SQL/Translator/Diff.pm index 50045a22..399a77dc 100644 --- a/lib/SQL/Translator/Diff.pm +++ b/lib/SQL/Translator/Diff.pm @@ -92,7 +92,7 @@ sub schema_diff { $options ||= {}; my $obj = SQL::Translator::Diff->new( { - %$options, + sqlt_args => $options, source_schema => $source_schema, target_schema => $target_schema, output_db => $output_db From ab42ef849111e4091ccae0981bb4241d60c15146 Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Wed, 24 Aug 2022 16:29:18 -0400 Subject: [PATCH 08/19] Removed batch alter stuff from Oracle.pm. It was just complicating things. Also fixed 90% of quoting issues that I think I introduced. Added additional testing. --- lib/SQL/Translator/Producer/Oracle.pm | 155 +++++++------------ t/54-oracle-sql-diff.t | 66 ++++++++ t/data/oracle/schema-1.5.sql | 211 ++++++++++++++++++++++++++ t/data/oracle/schema-1.6.sql | 207 +++++++++++++++++++++++++ 4 files changed, 541 insertions(+), 98 deletions(-) create mode 100644 t/54-oracle-sql-diff.t create mode 100644 t/data/oracle/schema-1.5.sql create mode 100644 t/data/oracle/schema-1.6.sql diff --git a/lib/SQL/Translator/Producer/Oracle.pm b/lib/SQL/Translator/Producer/Oracle.pm index 97f65de7..7eb64728 100644 --- a/lib/SQL/Translator/Producer/Oracle.pm +++ b/lib/SQL/Translator/Producer/Oracle.pm @@ -95,7 +95,8 @@ $DEBUG = 0 unless defined $DEBUG; use base 'SQL::Translator::Producer'; use SQL::Translator::Schema::Constants; -use SQL::Translator::Utils qw(header_comment batch_alter_table_statements); +use SQL::Translator::Utils qw(header_comment); +use Data::Dumper; my %translate = ( # @@ -390,23 +391,9 @@ sub create_table { push @field_defs, 'CONSTRAINT '.$index_name.' PRIMARY KEY '. '(' . join( ', ', @fields ) . ')'; } - elsif ( $index_type eq NORMAL ) { - $index_name = $index_name ? mk_name( $index_name ) - : mk_name( $table_name, $index_name || 'i' ); - $index_name = quote($index_name, $qf); - push @index_defs, - "CREATE INDEX $index_name on $table_name_q (". - join( ', ', @fields ). - ")$index_options"; - } - elsif ( $index_type eq UNIQUE ) { - $index_name = $index_name ? mk_name( $index_name ) - : mk_name( $table_name, $index_name || 'i' ); - $index_name = quote($index_name, $qf); - push @index_defs, - "CREATE UNIQUE INDEX $index_name on $table_name_q (". - join( ', ', @fields ). - ")$index_options"; + elsif ($index_type eq NORMAL or $index_type eq UNIQUE) { + warn "CAlling create index with options: " . Dumper($options) . "\n"; + push @index_defs, create_index($index, $options, $index_options); } else { warn "Unknown index type ($index_type) on table $table_name.\n" @@ -465,12 +452,12 @@ sub alter_field { sub drop_field { my ($old_field, $options) = @_; - - my $table_name = quote($old_field->table->name); + my $qi = $options->{quote_identifiers}; + my $table_name = quote($old_field->table->name, $qi); my $out = sprintf('ALTER TABLE %s DROP COLUMN %s', $table_name, - quote($old_field->name)); + quote($old_field->name, $qi)); return $out; } @@ -668,19 +655,20 @@ sub create_field { push @trigger_defs, $trigger; } - if ( lc $field->data_type eq 'timestamp' ) { - my $base_name = $table_name . "_". $field_name; - my $trig_name = quote(mk_name( $base_name, 'ts' ), $qt); - my $trigger = - "CREATE OR REPLACE TRIGGER $trig_name\n". - "BEFORE INSERT OR UPDATE ON $table_name_q\n". - "FOR EACH ROW WHEN (new.$field_name_q IS NULL)\n". - "BEGIN\n". - " SELECT sysdate INTO :new.$field_name_q FROM dual;\n". - "END;\n"; - - push @trigger_defs, $trigger; - } + # Do not create a trigger to insert sysdate to all timestamp fields + # if ( lc $field->data_type eq 'timestamp' ) { + # my $base_name = $table_name . "_". $field_name; + # my $trig_name = quote(mk_name( $base_name, 'ts' ), $qt); + # my $trigger = + # "CREATE OR REPLACE TRIGGER $trig_name\n". + # "BEFORE INSERT OR UPDATE ON $table_name_q\n". + # "FOR EACH ROW WHEN (new.$field_name_q IS NULL)\n". + # "BEGIN\n". + # " SELECT sysdate INTO :new.$field_name_q FROM dual;\n". + # "END;\n"; + + # push @trigger_defs, $trigger; + # } push @field_defs, $field_def; @@ -695,77 +683,48 @@ sub create_field { } -sub batch_alter_table { - my ($table, $diff_hash, $options) = @_; - - my %fks_to_alter; - my %fks_to_drop = map { - $_->type eq FOREIGN_KEY - ? ( $_->name => $_ ) - : ( ) - } @{$diff_hash->{alter_drop_constraint} }; - - my %fks_to_create = map { - if ( $_->type eq FOREIGN_KEY) { - $fks_to_alter{$_->name} = $fks_to_drop{$_->name} if $fks_to_drop{$_->name}; - ( $_->name => $_ ); - } else { ( ) } - } @{$diff_hash->{alter_create_constraint} }; - - my @drop_stmt; - if (scalar keys %fks_to_alter) { - $diff_hash->{alter_drop_constraint} = [ - grep { !$fks_to_alter{$_->name} } @{ $diff_hash->{alter_drop_constraint} } - ]; - - @drop_stmt = batch_alter_table($table, { alter_drop_constraint => [ values %fks_to_alter ] }, $options); - - } - - my @stmts = batch_alter_table_statements($diff_hash, $options); - - # rename_table makes things a bit more complex - my $renamed_from = ""; - $renamed_from = quote($diff_hash->{rename_table}[0][0]->name) - if $diff_hash->{rename_table} && @{$diff_hash->{rename_table}}; - - return unless @stmts; - # Just zero or one stmts. return now - return (@drop_stmt,@stmts) unless @stmts > 1; - - # Now strip off the 'ALTER TABLE xyz' of all but the first one - - my $table_name = quote($table->name); - - my $re = $renamed_from - ? qr/^ALTER TABLE (?:\Q$table_name\E|\Q$renamed_from\E) / - : qr/^ALTER TABLE \Q$table_name\E /; +sub drop_table { + my ($table, $options) = @_; - my $first = shift @stmts; - my ($alter_table) = $first =~ /($re)/; + my @foreign_key_constraints = grep { $_->type eq 'FOREIGN KEY' } $table->get_constraints; + my @statements; + for my $constraint(@foreign_key_constraints) { + push @statements, alter_drop_constraint($constraint); + } - my $padd = " " x length($alter_table); + return @statements, 'DROP TABLE ' . quote($table); +} - return @drop_stmt, join( ",\n", $first, map { s/$re//; $padd . $_ } @stmts); +sub alter_create_index { + my ($index, $options) = @_; + return create_index($index, $options); +} +sub create_index { + my ( $index, $options, $index_options) = @_; + my $qf = $options->{quote_field_names} || $options->{quote_identifiers}; + my $qt = $options->{quote_table_names} || $options->{quote_identifiers}; + return join( + ' ', + map { $_ || () } + 'CREATE', + lc $index->type eq 'normal' ? 'INDEX' : $index->type . ' INDEX', + $index->name ? quote($index->name, $qf): '', + 'ON', + quote($index->table, $qt), + '(' . join( ', ', map { quote($_) } $index->fields ) . ")$index_options" + ); } -sub drop_table { - my ($table, $options) = @_; - - return ( - # Drop (foreign key) constraints so table drops cleanly - batch_alter_table( - $table, { alter_drop_constraint => [ grep { $_->type eq 'FOREIGN KEY' } $table->get_constraints ] }, $options - ), - 'DROP TABLE ' . quote($table), - ); +sub alter_drop_index { + my ($index, $options) = @_; + return 'DROP INDEX ' . $index->name; } sub alter_drop_constraint { my ($c, $options) = @_; - - my $table_name = quote($c->table->name); + my $qi = $options->{quote_identifiers}; + my $table_name = quote($c->table->name, $qi); my @out = ('ALTER','TABLE',$table_name,'DROP'); if($c->type eq PRIMARY_KEY) { @@ -773,15 +732,15 @@ sub alter_drop_constraint { } else { push @out, ($c->type eq FOREIGN_KEY ? $c->type : "CONSTRAINT"), - quote($c->name); + quote($c->name, $qi); } return join(' ',@out); } sub alter_create_constraint { my ($index, $options) = @_; - - my $table_name = quote($index->table->name); + my $qi = $options->{quote_identifiers}; + my $table_name = quote($index->table->name, $qi); return join( ' ', 'ALTER TABLE', $table_name, diff --git a/t/54-oracle-sql-diff.t b/t/54-oracle-sql-diff.t new file mode 100644 index 00000000..66e53f0c --- /dev/null +++ b/t/54-oracle-sql-diff.t @@ -0,0 +1,66 @@ +#!/usr/bin/perl + +use FindBin qw/$Bin/; +use Test::More; +use Test::SQL::Translator; +use Test::Exception; +use Data::Dumper; +use SQL::Translator; +use SQL::Translator::Diff; + +BEGIN { + maybe_plan(6, 'SQL::Translator::Producer::Oracle'); +} + +my $schema1 = $Bin.'/data/oracle/schema-1.5.sql'; +my $schema2 = $Bin.'/data/oracle/schema-1.6.sql'; + +open my $io1, '<', $schema1 or die $!; +open my $io2, '<', $schema2 or die $!; + +my ($yaml1, $yaml2); +{ + local $/ = undef; + $sql1 = <$io1>; + $sql2 = <$io2>; +}; + +close $io1; +close $io2; + +my $s = SQL::Translator->new(from => 'Oracle'); +$s->parser->($s,$sql1); + +my $t = SQL::Translator->new(from => 'Oracle', debug => 1); +$t->parser->($t,$sql2); + +my $d = SQL::Translator::Diff->new + ({ + output_db => 'Oracle', + target_db => 'Oracle', + source_schema => $s->schema, + target_schema => $t->schema, + sqlt_args => {quote_identifiers => 1} + }); + + +my $diff = $d->compute_differences->produce_diff_sql || die $d->error; + +ok($diff, 'Diff generated.'); + +warn "The diff is: " . Dumper($diff); + +like($diff, '/CREATE TABLE t_group/', 'CREATE TABLE t_group generated'); + +like($diff, '/ALTER TABLE t_category DROP CONSTRAINT t_category_display_name/', 'DROP constraint display_name generated'); + +like($diff, '/ALTER TABLE t_message MODIFY \( alert_id number\(11\) \)/', 'MODIFY alert_id generated'); + +like($diff, '/CREATE INDEX t_user_groups_idx_user_id ON t_user_groups \(user_id\)/', 'CREATE INDEX generated'); +# like($diff, '/ALTER TABLE t_category_defaults DROP FOREIGN KEY t_category_defaults_user_id/', 'DROP FOREIGN KEY constraint generated'); + +# like($diff, '/ALTER TABLE t_user_groups ADD CONSTRAINT t_user_groups_group_id_fk FOREIGN KEY \(group_id\) REFERENCES t_group \(group_id\) ON DELETE CASCADE/', 'ADD FOREIGN KEY constraint generated'); + +# like($diff, '/ADD CONSTRAINT other_check CHECK \(other BETWEEN 100 and 99999\)/', 'ADD check constraint generated'); + +# like($diff, '/DROP TABLE customer/', 'DROP TABLE customer generated'); \ No newline at end of file diff --git a/t/data/oracle/schema-1.5.sql b/t/data/oracle/schema-1.5.sql new file mode 100644 index 00000000..1fcd7e44 --- /dev/null +++ b/t/data/oracle/schema-1.5.sql @@ -0,0 +1,211 @@ +CREATE TABLE t_category ( + category_id number(11) NOT NULL, + display_name varchar2(256) NOT NULL, + description varchar2(4000) NOT NULL, + added date DEFAULT CURRENT_TIMESTAMP NOT NULL, + added_by varchar2(32) NOT NULL, + modified date, + modified_by varchar2(32), + PRIMARY KEY (category_id), + CONSTRAINT t_category_display_name UNIQUE (display_name) +); + + +CREATE SEQUENCE sq_t_message_message_id; + +CREATE TABLE t_message ( + message_id number(11) NOT NULL, + alert_id number(45) NOT NULL, + from_address varchar2(256) NOT NULL, + recipient nvarchar2(64) NOT NULL, + subject_line varchar2(512) NOT NULL, + body_text clob NOT NULL, + body_html clob NOT NULL, + short_body varchar2(160) NOT NULL, + template_id number(11) NOT NULL, + added date DEFAULT CURRENT_TIMESTAMP NOT NULL, + added_by varchar2(32) NOT NULL, + modified date, + modified_by varchar2(32), + PRIMARY KEY (message_id) +); + + +CREATE TABLE t_user ( + user_id varchar2(32) NOT NULL, + name varchar2(256), + last4_pid varchar2(4) NOT NULL, + pidm number(11) NOT NULL, + added date DEFAULT CURRENT_TIMESTAMP NOT NULL, + added_by varchar2(32) NOT NULL, + modified date, + modified_by varchar2(32), + mobile_phone varchar2(11), + mobile_phone_source varchar2(64), + reason_for_change varchar2(128), + im_id varchar2(512), + opt_in date, + opt_in_confirm date, + mobile_phone_2 varchar2(11), + PRIMARY KEY (user_id), + CONSTRAINT t_user_pidm UNIQUE (pidm) +); + + +CREATE SEQUENCE sq_t_population_group_group_; + +CREATE TABLE t_population_group ( + group_id number(11) NOT NULL, + group_name varchar2(256) NOT NULL, + group_description varchar2(256) NOT NULL, + group_role number(11), + added date DEFAULT CURRENT_TIMESTAMP NOT NULL, + added_by varchar2(32) NOT NULL, + modified date, + modified_by varchar2(32), + group_type varchar2(256), + group_sql clob NOT NULL, + active number(1) NOT NULL, + source varchar2(256) NOT NULL, + private number(1) NOT NULL, + fpm_bldg_no varchar2(11) NOT NULL, + PRIMARY KEY (group_id) +); + + +CREATE SEQUENCE sq_t_role_role_id; + +CREATE TABLE t_role ( + role_id number(11) NOT NULL, + role_name varchar2(64) NOT NULL, + role_desc varchar2(128) NOT NULL, + PRIMARY KEY (role_id) +); + + +CREATE SEQUENCE sq_t_alert_alert_id; + +CREATE TABLE t_alert ( + alert_id number(11) NOT NULL, + category number(11) NOT NULL, + title varchar2(64) NOT NULL, + allow_email_opt_out number(1) NOT NULL, + enabled number(1) NOT NULL, + added date DEFAULT CURRENT_TIMESTAMP NOT NULL, + added_by varchar2(32) NOT NULL, + modified date, + modified_by varchar2(32), + PRIMARY KEY (alert_id) +); + + +CREATE TABLE t_user_groups ( + user_id varchar2(32) NOT NULL, + group_id number(11) NOT NULL, + PRIMARY KEY (user_id, group_id) +); + + +CREATE TABLE t_user_roles ( + user_id varchar2(32) NOT NULL, + role_id number(11) NOT NULL, + PRIMARY KEY (user_id, role_id) +); + + +CREATE TABLE t_category_defaults ( + category_id number(11) NOT NULL, + user_id varchar2(32) NOT NULL, + default_email number(1) NOT NULL, + default_sms number(1) NOT NULL, + default_push number(1) NOT NULL, + default_im number(1) NOT NULL, + modified date, + modified_by varchar2(32), + PRIMARY KEY (category_id, user_id) +); + + +CREATE TABLE t_alert_roles ( + alert_id number(11) NOT NULL, + role_id number(11) NOT NULL, + PRIMARY KEY (alert_id, role_id) +); + +ALTER TABLE t_alert ADD CONSTRAINT t_alert_category_fk FOREIGN KEY (category) REFERENCES t_category (category_id); + +ALTER TABLE t_user_groups ADD CONSTRAINT t_user_groups_group_id_fk FOREIGN KEY (group_id) REFERENCES t_population_group (group_id) ON DELETE CASCADE; + +ALTER TABLE t_user_roles ADD CONSTRAINT t_user_roles_role_id_fk FOREIGN KEY (role_id) REFERENCES t_role (role_id) ON DELETE CASCADE; + +ALTER TABLE t_category_defaults ADD CONSTRAINT t_category_defaults_category FOREIGN KEY (category_id) REFERENCES t_category (category_id); + +ALTER TABLE t_category_defaults ADD CONSTRAINT t_category_defaults_user_id FOREIGN KEY (user_id) REFERENCES t_user (user_id); + +ALTER TABLE t_alert_roles ADD CONSTRAINT t_alert_roles_alert_id_fk FOREIGN KEY (alert_id) REFERENCES t_alert (alert_id) ON DELETE CASCADE; + +ALTER TABLE t_alert_roles ADD CONSTRAINT t_alert_roles_role_id_fk FOREIGN KEY (role_id) REFERENCES t_role (role_id); + +ALTER TABLE t_population_group ADD CONSTRAINT t_population_group_group_role_fk FOREIGN KEY (group_role) REFERENCES t_role (role_id); + +CREATE INDEX t_alert_idx_category on t_alert (category); + +CREATE INDEX t_user_groups_idx_group_id on t_user_groups (group_id); + +CREATE INDEX t_user_roles_idx_role_id on t_user_roles (role_id); + +CREATE INDEX t_category_defaults_idx_cate on t_category_defaults (category_id); + +CREATE INDEX t_category_defaults_idx_acce on t_category_defaults (user_id); + +CREATE INDEX t_alert_roles_idx_alert_id on t_alert_roles (alert_id); + +CREATE INDEX t_alert_roles_idx_role_id on t_alert_roles (role_id); + +CREATE OR REPLACE TRIGGER ai_t_message_message_id +BEFORE INSERT ON t_message +FOR EACH ROW WHEN ( + new.message_id IS NULL OR new.message_id = 0 +) +BEGIN + SELECT sq_t_message_message_id.nextval + INTO :new.message_id + FROM dual; +END; +/ + +CREATE OR REPLACE TRIGGER ai_t_population_group_group_ +BEFORE INSERT ON t_population_group +FOR EACH ROW WHEN ( + new.group_id IS NULL OR new.group_id = 0 +) +BEGIN + SELECT sq_t_population_group_group_.nextval + INTO :new.group_id + FROM dual; +END; +/ + +CREATE OR REPLACE TRIGGER ai_t_role_role_id +BEFORE INSERT ON t_role +FOR EACH ROW WHEN ( + new.role_id IS NULL OR new.role_id = 0 +) +BEGIN + SELECT sq_t_role_role_id.nextval + INTO :new.role_id + FROM dual; +END; +/ + +CREATE OR REPLACE TRIGGER ai_t_alert_alert_id +BEFORE INSERT ON t_alert +FOR EACH ROW WHEN ( + new.alert_id IS NULL OR new.alert_id = 0 +) +BEGIN + SELECT sq_t_alert_alert_id.nextval + INTO :new.alert_id + FROM dual; +END; +/ \ No newline at end of file diff --git a/t/data/oracle/schema-1.6.sql b/t/data/oracle/schema-1.6.sql new file mode 100644 index 00000000..7bcd3257 --- /dev/null +++ b/t/data/oracle/schema-1.6.sql @@ -0,0 +1,207 @@ +CREATE TABLE t_category ( + category_id number(11) NOT NULL, + display_name varchar2(256) NOT NULL, + description varchar2(4000) NOT NULL, + added date DEFAULT CURRENT_TIMESTAMP NOT NULL, + added_by varchar2(32) NOT NULL, + modified date, + modified_by varchar2(32), + PRIMARY KEY (category_id) +); + +CREATE SEQUENCE sq_t_group_group_id; + +CREATE TABLE t_group ( + group_id number(11) NOT NULL, + group_name varchar2(256) NOT NULL, + group_description varchar2(256) NOT NULL, + added date DEFAULT CURRENT_TIMESTAMP NOT NULL, + added_by varchar2(32) NOT NULL, + modified date, + modified_by varchar2(32), + group_type varchar2(256), + group_sql clob NOT NULL, + active number(1) NOT NULL, + source varchar2(256) NOT NULL, + private number(1) NOT NULL, + fpm_bldg_no varchar2(11) NOT NULL, + PRIMARY KEY (group_id) +); + +CREATE SEQUENCE sq_t_message_message_id; + +CREATE TABLE t_message ( + message_id number(11) NOT NULL, + alert_id number(11) NOT NULL, + from_address varchar2(256) NOT NULL, + recipient varchar2(64) NOT NULL, + subject_line varchar2(512) NOT NULL, + body_text clob NOT NULL, + body_html clob NOT NULL, + short_body varchar2(160) NOT NULL, + template_id number(11) NOT NULL, + added date DEFAULT CURRENT_TIMESTAMP NOT NULL, + added_by varchar2(32) NOT NULL, + modified date, + modified_by varchar2(32), + PRIMARY KEY (message_id) +); + +CREATE SEQUENCE sq_t_role_role_id; + +CREATE TABLE t_role ( + role_id number(11) NOT NULL, + role_name varchar2(64) NOT NULL, + role_desc varchar2(128) NOT NULL, + PRIMARY KEY (role_id) +); + +CREATE TABLE t_user ( + user_id varchar2(32) NOT NULL, + name varchar2(256), + last4_pid varchar2(4) NOT NULL, + pidm number(11) NOT NULL, + added date DEFAULT CURRENT_TIMESTAMP NOT NULL, + added_by varchar2(32) NOT NULL, + modified date, + modified_by varchar2(32), + mobile_phone varchar2(11), + mobile_phone_source varchar2(64), + reason_for_change varchar2(128), + im_id varchar2(512), + opt_in date, + opt_in_confirm date, + mobile_phone_2 varchar2(11), + PRIMARY KEY (user_id), + CONSTRAINT t_user_pidm UNIQUE (pidm) +); + +CREATE SEQUENCE sq_t_alert_alert_id; + +CREATE TABLE t_alert ( + alert_id number(11) NOT NULL, + category number(11) NOT NULL, + title varchar2(64) NOT NULL, + allow_email_opt_out number(1) NOT NULL, + enabled number(1) NOT NULL, + added date DEFAULT CURRENT_TIMESTAMP NOT NULL, + added_by varchar2(32) NOT NULL, + modified date, + modified_by varchar2(32), + PRIMARY KEY (alert_id) +); + +CREATE TABLE t_category_defaults ( + category_id number(11) NOT NULL, + user_id varchar2(32) NOT NULL, + default_email number(1) NOT NULL, + default_sms number(1) NOT NULL, + default_push number(1) NOT NULL, + default_im number(1) NOT NULL, + modified date, + modified_by varchar2(32), + PRIMARY KEY (category_id, user_id) +); + +CREATE TABLE t_user_groups ( + user_id varchar2(32) NOT NULL, + group_id number(11) NOT NULL, + PRIMARY KEY (user_id, group_id) +); + +CREATE TABLE t_user_roles ( + user_id varchar2(32) NOT NULL, + role_id number(11) NOT NULL, + PRIMARY KEY (user_id, role_id) +); + +CREATE TABLE t_alert_roles ( + alert_id number(11) NOT NULL, + role_id number(11) NOT NULL, + PRIMARY KEY (alert_id, role_id) +); + +ALTER TABLE t_alert ADD CONSTRAINT t_alert_category_fk FOREIGN KEY (category) REFERENCES t_category (category_id); + +ALTER TABLE t_category_defaults ADD CONSTRAINT t_category_defaults_category FOREIGN KEY (category_id) REFERENCES t_category (category_id); + +ALTER TABLE t_category_defaults ADD CONSTRAINT t_category_defaults_user_id FOREIGN KEY (user_id) REFERENCES t_user (user_id); + +ALTER TABLE t_user_groups ADD CONSTRAINT t_user_groups_group_id_fk FOREIGN KEY (group_id) REFERENCES t_group (group_id) ON DELETE CASCADE; + +ALTER TABLE t_user_groups ADD CONSTRAINT t_user_groups_user_id_fk FOREIGN KEY (user_id) REFERENCES t_user (user_id) ON DELETE CASCADE; + +ALTER TABLE t_user_roles ADD CONSTRAINT t_user_roles_role_id_fk FOREIGN KEY (role_id) REFERENCES t_role (role_id) ON DELETE CASCADE; + +ALTER TABLE t_user_roles ADD CONSTRAINT t_user_roles_user_id_fk FOREIGN KEY (user_id) REFERENCES t_user (user_id) ON DELETE CASCADE; + +ALTER TABLE t_alert_roles ADD CONSTRAINT t_alert_roles_alert_id_fk FOREIGN KEY (alert_id) REFERENCES t_alert (alert_id) ON DELETE CASCADE; + +ALTER TABLE t_alert_roles ADD CONSTRAINT t_alert_roles_role_id_fk FOREIGN KEY (role_id) REFERENCES t_role (role_id); + +CREATE INDEX t_alert_idx_category on t_alert (category); + +CREATE INDEX t_category_defaults_idx_cate on t_category_defaults (category_id); + +CREATE INDEX t_category_defaults_idx_acce on t_category_defaults (user_id); + +CREATE INDEX t_user_groups_idx_group_id on t_user_groups (group_id); + +CREATE INDEX t_user_groups_idx_user_id on t_user_groups (user_id); + +CREATE INDEX t_user_roles_idx_role_id on t_user_roles (role_id); + +CREATE INDEX t_user_roles_idx_user_id on t_user_roles (user_id); + +CREATE INDEX t_alert_roles_idx_alert_id on t_alert_roles (alert_id); + +CREATE INDEX t_alert_roles_idx_role_id on t_alert_roles (role_id); + +CREATE OR REPLACE TRIGGER ai_t_group_group_id +BEFORE INSERT ON t_group +FOR EACH ROW WHEN ( + new.group_id IS NULL OR new.group_id = 0 +) +BEGIN + SELECT sq_t_group_group_id.nextval + INTO :new.group_id + FROM dual; +END; +/ + +CREATE OR REPLACE TRIGGER ai_t_message_message_id +BEFORE INSERT ON t_message +FOR EACH ROW WHEN ( + new.message_id IS NULL OR new.message_id = 0 +) +BEGIN + SELECT sq_t_message_message_id.nextval + INTO :new.message_id + FROM dual; +END; +/ + +CREATE OR REPLACE TRIGGER ai_t_role_role_id +BEFORE INSERT ON t_role +FOR EACH ROW WHEN ( + new.role_id IS NULL OR new.role_id = 0 +) +BEGIN + SELECT sq_t_role_role_id.nextval + INTO :new.role_id + FROM dual; +END; +/ + +CREATE OR REPLACE TRIGGER ai_t_alert_alert_id +BEFORE INSERT ON t_alert +FOR EACH ROW WHEN ( + new.alert_id IS NULL OR new.alert_id = 0 +) +BEGIN + SELECT sq_t_alert_alert_id.nextval + INTO :new.alert_id + FROM dual; +END; +/ + From df7e62f16b7eb56e12c8c059a33906541f81da5f Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Thu, 25 Aug 2022 12:46:44 -0400 Subject: [PATCH 09/19] All quoting is working consistent to how it works for MySQL and SQLServer producers. FK constraint generation works properly and additional testing has been cleaned up. --- lib/SQL/Translator/Producer/Oracle.pm | 94 ++++++++++++--------------- t/54-oracle-sql-diff.t | 21 +++--- t/55-oracle-producer.t | 9 +-- t/data/oracle/schema-1.5.sql | 2 + t/data/oracle/schema-1.6.sql | 6 -- 5 files changed, 55 insertions(+), 77 deletions(-) diff --git a/lib/SQL/Translator/Producer/Oracle.pm b/lib/SQL/Translator/Producer/Oracle.pm index 7eb64728..1373d5d9 100644 --- a/lib/SQL/Translator/Producer/Oracle.pm +++ b/lib/SQL/Translator/Producer/Oracle.pm @@ -195,7 +195,7 @@ sub produce { my $schema = $translator->schema; my $oracle_version = $translator->producer_args->{oracle_version} || 0; my $delay_constraints = $translator->producer_args->{delay_constraints}; - my ($output, $create, @table_defs, @fk_defs, @trigger_defs, @index_defs, @constraint_defs); + my ($output, $create, @table_defs, @trigger_defs, @index_defs, @constraint_defs); $create .= header_comment unless ($no_comments); my $qt = 1 if $translator->quote_table_names; @@ -212,7 +212,7 @@ sub produce { } for my $table ( $schema->get_tables ) { - my ( $table_def, $fk_def, $trigger_def, $index_def, $constraint_def ) = create_table( + my ( $table_def, $trigger_def, $index_def, $constraint_def ) = create_table( $table, { add_drop_table => $add_drop_table, @@ -224,7 +224,6 @@ sub produce { } ); push @table_defs, @$table_def; - push @fk_defs, @$fk_def; push @trigger_defs, @$trigger_def; push @index_defs, @$index_def; push @constraint_defs, @$constraint_def; @@ -243,10 +242,10 @@ sub produce { } if (wantarray) { - return defined $create ? $create : (), @table_defs, @view_defs, @fk_defs, @trigger_defs, @index_defs, @constraint_defs; + return defined $create ? $create : (), @table_defs, @view_defs, @trigger_defs, @index_defs, @constraint_defs; } else { - $create .= join (";\n\n", @table_defs, @view_defs, @fk_defs, @index_defs, @constraint_defs); + $create .= join (";\n\n", @table_defs, @view_defs, @index_defs, @constraint_defs); $create .= ";\n\n"; # If wantarray is not set we have to add "/" in this statement # DBI->do() needs them omitted @@ -266,7 +265,7 @@ sub create_table { my $item = ''; my $drop; - my (@create, @field_defs, @constraint_defs, @fk_defs, @trigger_defs); + my (@create, @field_defs, @constraint_defs, @trigger_defs); push @create, "--\n-- Table: $table_name\n--" unless $options->{no_comments}; push @create, qq[DROP TABLE $table_name_q CASCADE CONSTRAINTS] if $options->{add_drop_table}; @@ -310,46 +309,6 @@ sub create_table { for my $c ( $table->get_constraints ) { my $constr = create_constraint($c, $options); push @constraint_defs, $constr if ($constr); - - my $name = $c->name || ''; - my @fields = map { quote($_,$qf) } $c->fields; - my @rfields = map { quote($_,$qf) } $c->reference_fields; - - if ( $c->type eq FOREIGN_KEY ) { - $name = mk_name( join('_', $table_name, $c->fields). '_fk' ); - $name = quote($name, $qf); - my $on_delete = uc ($c->on_delete || ''); - - my $def = "CONSTRAINT $name FOREIGN KEY "; - - if ( @fields ) { - $def .= '(' . join( ', ', @fields ) . ')'; - } - - my $ref_table = quote($c->reference_table,$qt); - - $def .= " REFERENCES $ref_table"; - - if ( @rfields ) { - $def .= ' (' . join( ', ', @rfields ) . ')'; - } - - if ( $c->match_type ) { - $def .= ' MATCH ' . - ( $c->match_type =~ /full/i ) ? 'FULL' : 'PARTIAL'; - } - - if ( $on_delete && $on_delete ne "RESTRICT") { - $def .= ' ON DELETE '.$c->on_delete; - } - - # disabled by plu 2007-12-29 - doesn't exist for oracle - #if ( $c->on_update ) { - # $def .= ' ON UPDATE '. $c->on_update; - #} - - push @fk_defs, sprintf("ALTER TABLE %s ADD %s", $table_name_q, $def); - } } # @@ -392,7 +351,6 @@ sub create_table { '(' . join( ', ', @fields ) . ')'; } elsif ($index_type eq NORMAL or $index_type eq UNIQUE) { - warn "CAlling create index with options: " . Dumper($options) . "\n"; push @index_defs, create_index($index, $options, $index_options); } else { @@ -427,7 +385,7 @@ sub create_table { } } - return \@create, \@fk_defs, \@trigger_defs, \@index_defs, ($options->{delay_constraints} ? \@constraint_defs : []); + return \@create, \@trigger_defs, \@index_defs, ($options->{delay_constraints} ? \@constraint_defs : []); } sub alter_field { @@ -686,13 +644,14 @@ sub create_field { sub drop_table { my ($table, $options) = @_; + my $qi = $options->{quote_identifiers}; my @foreign_key_constraints = grep { $_->type eq 'FOREIGN KEY' } $table->get_constraints; my @statements; for my $constraint(@foreign_key_constraints) { - push @statements, alter_drop_constraint($constraint); + push @statements, alter_drop_constraint($constraint, $options); } - return @statements, 'DROP TABLE ' . quote($table); + return @statements, 'DROP TABLE ' . quote($table, $qi); } sub alter_create_index { @@ -702,6 +661,7 @@ sub alter_create_index { sub create_index { my ( $index, $options, $index_options) = @_; + $index_options = $index_options || ''; my $qf = $options->{quote_field_names} || $options->{quote_identifiers}; my $qt = $options->{quote_table_names} || $options->{quote_identifiers}; return join( @@ -738,14 +698,14 @@ sub alter_drop_constraint { } sub alter_create_constraint { - my ($index, $options) = @_; + my ($c, $options) = @_; my $qi = $options->{quote_identifiers}; - my $table_name = quote($index->table->name, $qi); + my $table_name = quote($c->table->name, $qi); return join( ' ', 'ALTER TABLE', $table_name, 'ADD', - create_constraint(@_) ); + create_constraint($c, $options) ); } sub create_constraint { @@ -810,6 +770,34 @@ sub create_constraint { my $expression = $c->expression || ''; $definition = "CONSTRAINT $name CHECK ($expression)"; } + elsif ( $c->type eq FOREIGN_KEY ) { + $name = mk_name( join('_', $table_name, $c->fields). '_fk' ); + $name = quote($name, $qf); + my $on_delete = uc ($c->on_delete || ''); + + $definition = "CONSTRAINT $name FOREIGN KEY "; + + if ( @fields ) { + $definition .= '(' . join( ', ', @fields ) . ')'; + } + + my $ref_table = quote($c->reference_table,$qt); + + $definition .= " REFERENCES $ref_table"; + + if ( @rfields ) { + $definition .= ' (' . join( ', ', @rfields ) . ')'; + } + + if ( $c->match_type ) { + $definition .= ' MATCH ' . + ( $c->match_type =~ /full/i ) ? 'FULL' : 'PARTIAL'; + } + + if ( $on_delete && $on_delete ne "RESTRICT") { + $definition .= ' ON DELETE '.$c->on_delete; + } + } return $definition ? $definition : undef; } diff --git a/t/54-oracle-sql-diff.t b/t/54-oracle-sql-diff.t index 66e53f0c..abc16210 100644 --- a/t/54-oracle-sql-diff.t +++ b/t/54-oracle-sql-diff.t @@ -9,7 +9,7 @@ use SQL::Translator; use SQL::Translator::Diff; BEGIN { - maybe_plan(6, 'SQL::Translator::Producer::Oracle'); + maybe_plan(10, 'SQL::Translator::Producer::Oracle'); } my $schema1 = $Bin.'/data/oracle/schema-1.5.sql'; @@ -37,30 +37,29 @@ $t->parser->($t,$sql2); my $d = SQL::Translator::Diff->new ({ output_db => 'Oracle', - target_db => 'Oracle', source_schema => $s->schema, target_schema => $t->schema, - sqlt_args => {quote_identifiers => 1} + sqlt_args => {quote_identifiers => 0} }); - my $diff = $d->compute_differences->produce_diff_sql || die $d->error; ok($diff, 'Diff generated.'); -warn "The diff is: " . Dumper($diff); - like($diff, '/CREATE TABLE t_group/', 'CREATE TABLE t_group generated'); -like($diff, '/ALTER TABLE t_category DROP CONSTRAINT t_category_display_name/', 'DROP constraint display_name generated'); +like($diff, '/ALTER TABLE t_category DROP CONSTRAINT t_category_display_name/', 'DROP constraint t_category_display_name generated'); + +like($diff, '/ALTER TABLE t_user_groups DROP FOREIGN KEY t_user_groups_group_id_fk/', 'DROP FOREIGN KEY constraint generated'); + +like($diff, '/DROP INDEX t_alert_roles_idx_alert_id/', 'DROP INDEX generated'); like($diff, '/ALTER TABLE t_message MODIFY \( alert_id number\(11\) \)/', 'MODIFY alert_id generated'); like($diff, '/CREATE INDEX t_user_groups_idx_user_id ON t_user_groups \(user_id\)/', 'CREATE INDEX generated'); -# like($diff, '/ALTER TABLE t_category_defaults DROP FOREIGN KEY t_category_defaults_user_id/', 'DROP FOREIGN KEY constraint generated'); -# like($diff, '/ALTER TABLE t_user_groups ADD CONSTRAINT t_user_groups_group_id_fk FOREIGN KEY \(group_id\) REFERENCES t_group \(group_id\) ON DELETE CASCADE/', 'ADD FOREIGN KEY constraint generated'); +like($diff, '/ALTER TABLE t_user_groups ADD CONSTRAINT t_user_groups_group_id_fk FOREIGN KEY \(group_id\) REFERENCES t_group \(group_id\) ON DELETE CASCADE/', 'ADD FOREIGN KEY constraint generated'); -# like($diff, '/ADD CONSTRAINT other_check CHECK \(other BETWEEN 100 and 99999\)/', 'ADD check constraint generated'); +like($diff, '/ALTER TABLE t_population_group DROP FOREIGN KEY t_population_group_group_role_fk/', 'DROP FOREIGN KEY before drop table generated'); -# like($diff, '/DROP TABLE customer/', 'DROP TABLE customer generated'); \ No newline at end of file +like($diff, '/DROP TABLE t_population_group/', 'DROP TABLE generated'); \ No newline at end of file diff --git a/t/55-oracle-producer.t b/t/55-oracle-producer.t index 57e25ea5..beeb294a 100644 --- a/t/55-oracle-producer.t +++ b/t/55-oracle-producer.t @@ -69,16 +69,11 @@ use SQL::Translator::Producer::Oracle; type => FOREIGN_KEY, ); - my ($table1_def, $fk1_def, $trigger1_def, + my ($table1_def, $trigger1_def, $index1_def, $constraint1_def ) = SQL::Translator::Producer::Oracle::create_table($table1); - is_deeply( - $fk1_def, - [ 'ALTER TABLE table1 ADD CONSTRAINT table1_fk_col1_fk_col2_fk FOREIGN KEY (fk_col1, fk_col2) REFERENCES table2 (fk_col1, fk_col2)' - ], - 'correct "CREATE CONSTRAINT" SQL' - ); + like($table1_def->[1], '/CONSTRAINT table1_fk_col1_fk_col2_fk FOREIGN KEY \(fk_col1, fk_col2\) REFERENCES table2 \(fk_col1, fk_col2\)/', 'correct "CONSTRAINT" SQL'); my $materialized_view = SQL::Translator::Schema::View->new( name => 'matview', diff --git a/t/data/oracle/schema-1.5.sql b/t/data/oracle/schema-1.5.sql index 1fcd7e44..17380c5f 100644 --- a/t/data/oracle/schema-1.5.sql +++ b/t/data/oracle/schema-1.5.sql @@ -152,6 +152,8 @@ CREATE INDEX t_alert_idx_category on t_alert (category); CREATE INDEX t_user_groups_idx_group_id on t_user_groups (group_id); +CREATE INDEX t_user_roles_idx_user_id on t_user_roles (user_id); + CREATE INDEX t_user_roles_idx_role_id on t_user_roles (role_id); CREATE INDEX t_category_defaults_idx_cate on t_category_defaults (category_id); diff --git a/t/data/oracle/schema-1.6.sql b/t/data/oracle/schema-1.6.sql index 7bcd3257..152fd6f1 100644 --- a/t/data/oracle/schema-1.6.sql +++ b/t/data/oracle/schema-1.6.sql @@ -129,12 +129,8 @@ ALTER TABLE t_category_defaults ADD CONSTRAINT t_category_defaults_user_id FOREI ALTER TABLE t_user_groups ADD CONSTRAINT t_user_groups_group_id_fk FOREIGN KEY (group_id) REFERENCES t_group (group_id) ON DELETE CASCADE; -ALTER TABLE t_user_groups ADD CONSTRAINT t_user_groups_user_id_fk FOREIGN KEY (user_id) REFERENCES t_user (user_id) ON DELETE CASCADE; - ALTER TABLE t_user_roles ADD CONSTRAINT t_user_roles_role_id_fk FOREIGN KEY (role_id) REFERENCES t_role (role_id) ON DELETE CASCADE; -ALTER TABLE t_user_roles ADD CONSTRAINT t_user_roles_user_id_fk FOREIGN KEY (user_id) REFERENCES t_user (user_id) ON DELETE CASCADE; - ALTER TABLE t_alert_roles ADD CONSTRAINT t_alert_roles_alert_id_fk FOREIGN KEY (alert_id) REFERENCES t_alert (alert_id) ON DELETE CASCADE; ALTER TABLE t_alert_roles ADD CONSTRAINT t_alert_roles_role_id_fk FOREIGN KEY (role_id) REFERENCES t_role (role_id); @@ -153,8 +149,6 @@ CREATE INDEX t_user_roles_idx_role_id on t_user_roles (role_id); CREATE INDEX t_user_roles_idx_user_id on t_user_roles (user_id); -CREATE INDEX t_alert_roles_idx_alert_id on t_alert_roles (alert_id); - CREATE INDEX t_alert_roles_idx_role_id on t_alert_roles (role_id); CREATE OR REPLACE TRIGGER ai_t_group_group_id From c90a59957b52a27563d5f20d7714577cdbeb13e3 Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Wed, 7 Sep 2022 12:21:26 -0400 Subject: [PATCH 10/19] Fixed issue with drop FK constraint syntax --- lib/SQL/Translator/Producer/Oracle.pm | 9 +-------- t/54-oracle-alter-constraint.t | 4 ++-- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/lib/SQL/Translator/Producer/Oracle.pm b/lib/SQL/Translator/Producer/Oracle.pm index 1373d5d9..d19bab11 100644 --- a/lib/SQL/Translator/Producer/Oracle.pm +++ b/lib/SQL/Translator/Producer/Oracle.pm @@ -686,14 +686,7 @@ sub alter_drop_constraint { my $qi = $options->{quote_identifiers}; my $table_name = quote($c->table->name, $qi); - my @out = ('ALTER','TABLE',$table_name,'DROP'); - if($c->type eq PRIMARY_KEY) { - push @out, $c->type; - } - else { - push @out, ($c->type eq FOREIGN_KEY ? $c->type : "CONSTRAINT"), - quote($c->name, $qi); - } + my @out = ('ALTER','TABLE',$table_name,'DROP','CONSTRAINT',quote($c->name, $qi)); return join(' ',@out); } diff --git a/t/54-oracle-alter-constraint.t b/t/54-oracle-alter-constraint.t index eeac1bc4..35099300 100644 --- a/t/54-oracle-alter-constraint.t +++ b/t/54-oracle-alter-constraint.t @@ -54,6 +54,6 @@ like($diff, '/DROP CONSTRAINT other_check/', 'DROP constraint other_check genera like($diff, '/ADD CONSTRAINT other_check CHECK \(other BETWEEN 100 and 99999\)/', 'ADD check constraint generated'); -like($diff, '/ALTER TABLE supplier DROP FOREIGN KEY fk_customer/', 'DROP Foreign key constraint generated'); +like($diff, '/ALTER TABLE supplier CONSTRAINT fk_customer/', 'DROP Foreign key constraint generated'); -like($diff, '/DROP TABLE customer/', 'DROP TABLE customer generated'); \ No newline at end of file +like($diff, '/DROP TABLE customer/', 'DROP TABLE customer generated'); From b371f163984549daf03976df0f1448cc9ac08b6e Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Thu, 8 Sep 2022 11:34:09 -0400 Subject: [PATCH 11/19] Fixed restriction stopping change of NOT NULL to NULL for non CLOB fields. Updated tests for fix of FK drop and Not Null to Null. --- lib/SQL/Translator/Producer/Oracle.pm | 9 +++++++-- t/54-oracle-alter-constraint.t | 2 +- t/54-oracle-alter-field.t | 2 +- t/54-oracle-sql-diff.t | 4 ++-- t/data/oracle/schema_diff_b.yaml | 2 +- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/SQL/Translator/Producer/Oracle.pm b/lib/SQL/Translator/Producer/Oracle.pm index d19bab11..31e5effb 100644 --- a/lib/SQL/Translator/Producer/Oracle.pm +++ b/lib/SQL/Translator/Producer/Oracle.pm @@ -396,8 +396,13 @@ sub alter_field { create_field($to_field, $options, {}); # Fix ORA-01442 - if ($to_field->is_nullable && !$from_field->is_nullable) { - die 'Cannot remove NOT NULL from table field'; + if (!$from_field->is_nullable && $to_field->is_nullable) { + if ($from_field->data_type =~ /text/) { + die 'Cannot alter CLOB field in this way'; + } + else { + @$field_defs = map { $_ .= ' NULL' } @$field_defs; + } } elsif (!$from_field->is_nullable && !$to_field->is_nullable) { @$field_defs = map { s/ NOT NULL//; $_} @$field_defs; } diff --git a/t/54-oracle-alter-constraint.t b/t/54-oracle-alter-constraint.t index 35099300..1dd6cb5d 100644 --- a/t/54-oracle-alter-constraint.t +++ b/t/54-oracle-alter-constraint.t @@ -54,6 +54,6 @@ like($diff, '/DROP CONSTRAINT other_check/', 'DROP constraint other_check genera like($diff, '/ADD CONSTRAINT other_check CHECK \(other BETWEEN 100 and 99999\)/', 'ADD check constraint generated'); -like($diff, '/ALTER TABLE supplier CONSTRAINT fk_customer/', 'DROP Foreign key constraint generated'); +like($diff, '/ALTER TABLE supplier DROP CONSTRAINT fk_customer/', 'DROP Foreign key constraint generated'); like($diff, '/DROP TABLE customer/', 'DROP TABLE customer generated'); diff --git a/t/54-oracle-alter-field.t b/t/54-oracle-alter-field.t index 5f736ccf..38b37b96 100644 --- a/t/54-oracle-alter-field.t +++ b/t/54-oracle-alter-field.t @@ -47,7 +47,7 @@ my $d = SQL::Translator::Diff->new my $diff = $d->compute_differences->produce_diff_sql || die $d->error; ok($diff, 'Diff generated.'); -like($diff, '/ALTER TABLE d_operator MODIFY \( name nvarchar2\(10\) \)/', +like($diff, '/ALTER TABLE d_operator MODIFY \( name nvarchar2\(10\) NULL \)/', 'Alter table generated.'); like($diff, '/ALTER TABLE d_operator MODIFY \( other nvarchar2\(10\) NOT NULL \)/', 'Alter table generated.'); diff --git a/t/54-oracle-sql-diff.t b/t/54-oracle-sql-diff.t index abc16210..7b5042a1 100644 --- a/t/54-oracle-sql-diff.t +++ b/t/54-oracle-sql-diff.t @@ -50,7 +50,7 @@ like($diff, '/CREATE TABLE t_group/', 'CREATE TABLE t_group generated'); like($diff, '/ALTER TABLE t_category DROP CONSTRAINT t_category_display_name/', 'DROP constraint t_category_display_name generated'); -like($diff, '/ALTER TABLE t_user_groups DROP FOREIGN KEY t_user_groups_group_id_fk/', 'DROP FOREIGN KEY constraint generated'); +like($diff, '/ALTER TABLE t_user_groups DROP CONSTRAINT t_user_groups_group_id_fk/', 'DROP FOREIGN KEY constraint generated'); like($diff, '/DROP INDEX t_alert_roles_idx_alert_id/', 'DROP INDEX generated'); @@ -60,6 +60,6 @@ like($diff, '/CREATE INDEX t_user_groups_idx_user_id ON t_user_groups \(user_id\ like($diff, '/ALTER TABLE t_user_groups ADD CONSTRAINT t_user_groups_group_id_fk FOREIGN KEY \(group_id\) REFERENCES t_group \(group_id\) ON DELETE CASCADE/', 'ADD FOREIGN KEY constraint generated'); -like($diff, '/ALTER TABLE t_population_group DROP FOREIGN KEY t_population_group_group_role_fk/', 'DROP FOREIGN KEY before drop table generated'); +like($diff, '/ALTER TABLE t_population_group DROP CONSTRAINT t_population_group_group_role_fk/', 'DROP FOREIGN KEY before drop table generated'); like($diff, '/DROP TABLE t_population_group/', 'DROP TABLE generated'); \ No newline at end of file diff --git a/t/data/oracle/schema_diff_b.yaml b/t/data/oracle/schema_diff_b.yaml index 3c6b7a2f..cb9699e8 100644 --- a/t/data/oracle/schema_diff_b.yaml +++ b/t/data/oracle/schema_diff_b.yaml @@ -44,7 +44,7 @@ schema: data_type: nvarchar2 default_value: ~ extra: {} - is_nullable: 0 + is_nullable: 1 is_primary_key: 0 is_unique: 0 name: name From b14da74a5bb7e2e342e2651c6abfdf656e153128 Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Fri, 9 Sep 2022 13:44:32 -0400 Subject: [PATCH 12/19] Removed commented out section of code. --- lib/SQL/Translator/Producer/Oracle.pm | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/lib/SQL/Translator/Producer/Oracle.pm b/lib/SQL/Translator/Producer/Oracle.pm index 31e5effb..897ebf2c 100644 --- a/lib/SQL/Translator/Producer/Oracle.pm +++ b/lib/SQL/Translator/Producer/Oracle.pm @@ -618,21 +618,6 @@ sub create_field { push @trigger_defs, $trigger; } - # Do not create a trigger to insert sysdate to all timestamp fields - # if ( lc $field->data_type eq 'timestamp' ) { - # my $base_name = $table_name . "_". $field_name; - # my $trig_name = quote(mk_name( $base_name, 'ts' ), $qt); - # my $trigger = - # "CREATE OR REPLACE TRIGGER $trig_name\n". - # "BEFORE INSERT OR UPDATE ON $table_name_q\n". - # "FOR EACH ROW WHEN (new.$field_name_q IS NULL)\n". - # "BEGIN\n". - # " SELECT sysdate INTO :new.$field_name_q FROM dual;\n". - # "END;\n"; - - # push @trigger_defs, $trigger; - # } - push @field_defs, $field_def; if ( my $comment = $field->comments ) { From d2d080cb4ce28ef4e22dc33072f61e9ef73c571a Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Mon, 26 Sep 2022 11:06:40 -0400 Subject: [PATCH 13/19] fixed issue with alter_drop_constraint when dropping unnamed primary key. --- lib/SQL/Translator/Producer/Oracle.pm | 9 +++++++-- t/54-oracle-sql-diff.t | 2 ++ t/data/oracle/schema-1.6.sql | 8 ++++---- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/SQL/Translator/Producer/Oracle.pm b/lib/SQL/Translator/Producer/Oracle.pm index 897ebf2c..2a0a4dd1 100644 --- a/lib/SQL/Translator/Producer/Oracle.pm +++ b/lib/SQL/Translator/Producer/Oracle.pm @@ -675,8 +675,13 @@ sub alter_drop_constraint { my ($c, $options) = @_; my $qi = $options->{quote_identifiers}; my $table_name = quote($c->table->name, $qi); - - my @out = ('ALTER','TABLE',$table_name,'DROP','CONSTRAINT',quote($c->name, $qi)); + my @out = ('ALTER','TABLE',$table_name,'DROP',); + if ($c->name) { + push @out, ('CONSTRAINT',quote($c->name, $qi)); + } + elsif ($c->type eq PRIMARY_KEY) { + push @out, 'PRIMARY KEY'; + } return join(' ',@out); } diff --git a/t/54-oracle-sql-diff.t b/t/54-oracle-sql-diff.t index 7b5042a1..f3c4d60c 100644 --- a/t/54-oracle-sql-diff.t +++ b/t/54-oracle-sql-diff.t @@ -48,6 +48,8 @@ ok($diff, 'Diff generated.'); like($diff, '/CREATE TABLE t_group/', 'CREATE TABLE t_group generated'); +like($diff, '/ALTER TABLE t_category DROP PRIMARY KEY/', 'Drop PRIMARY KEY generated'); + like($diff, '/ALTER TABLE t_category DROP CONSTRAINT t_category_display_name/', 'DROP constraint t_category_display_name generated'); like($diff, '/ALTER TABLE t_user_groups DROP CONSTRAINT t_user_groups_group_id_fk/', 'DROP FOREIGN KEY constraint generated'); diff --git a/t/data/oracle/schema-1.6.sql b/t/data/oracle/schema-1.6.sql index 152fd6f1..3b7be9cd 100644 --- a/t/data/oracle/schema-1.6.sql +++ b/t/data/oracle/schema-1.6.sql @@ -1,12 +1,12 @@ CREATE TABLE t_category ( - category_id number(11) NOT NULL, + id number(11) NOT NULL, display_name varchar2(256) NOT NULL, description varchar2(4000) NOT NULL, added date DEFAULT CURRENT_TIMESTAMP NOT NULL, added_by varchar2(32) NOT NULL, modified date, modified_by varchar2(32), - PRIMARY KEY (category_id) + PRIMARY KEY (id) ); CREATE SEQUENCE sq_t_group_group_id; @@ -121,9 +121,9 @@ CREATE TABLE t_alert_roles ( PRIMARY KEY (alert_id, role_id) ); -ALTER TABLE t_alert ADD CONSTRAINT t_alert_category_fk FOREIGN KEY (category) REFERENCES t_category (category_id); +ALTER TABLE t_alert ADD CONSTRAINT t_alert_category_fk FOREIGN KEY (category) REFERENCES t_category (id); -ALTER TABLE t_category_defaults ADD CONSTRAINT t_category_defaults_category FOREIGN KEY (category_id) REFERENCES t_category (category_id); +ALTER TABLE t_category_defaults ADD CONSTRAINT t_category_defaults_category FOREIGN KEY (category_id) REFERENCES t_category (id); ALTER TABLE t_category_defaults ADD CONSTRAINT t_category_defaults_user_id FOREIGN KEY (user_id) REFERENCES t_user (user_id); From 2d47b5c2e3566485752d2ec1b2a7986147b5cdff Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Thu, 29 Sep 2022 10:24:11 -0400 Subject: [PATCH 14/19] Reverted change to passing of $options and fixed test plan. --- lib/SQL/Translator/Diff.pm | 2 +- t/54-oracle-sql-diff.t | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/SQL/Translator/Diff.pm b/lib/SQL/Translator/Diff.pm index 399a77dc..50045a22 100644 --- a/lib/SQL/Translator/Diff.pm +++ b/lib/SQL/Translator/Diff.pm @@ -92,7 +92,7 @@ sub schema_diff { $options ||= {}; my $obj = SQL::Translator::Diff->new( { - sqlt_args => $options, + %$options, source_schema => $source_schema, target_schema => $target_schema, output_db => $output_db diff --git a/t/54-oracle-sql-diff.t b/t/54-oracle-sql-diff.t index f3c4d60c..20aba524 100644 --- a/t/54-oracle-sql-diff.t +++ b/t/54-oracle-sql-diff.t @@ -9,7 +9,7 @@ use SQL::Translator; use SQL::Translator::Diff; BEGIN { - maybe_plan(10, 'SQL::Translator::Producer::Oracle'); + maybe_plan(11, 'SQL::Translator::Producer::Oracle'); } my $schema1 = $Bin.'/data/oracle/schema-1.5.sql'; From b4a8ca76a6693129399ddda96c33594504186640 Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Wed, 5 Oct 2022 11:07:20 -0400 Subject: [PATCH 15/19] Added debug statements to figure out constraint order issue --- lib/SQL/Translator/Producer/Oracle.pm | 177 ++++++++++++++------------ 1 file changed, 95 insertions(+), 82 deletions(-) diff --git a/lib/SQL/Translator/Producer/Oracle.pm b/lib/SQL/Translator/Producer/Oracle.pm index 2a0a4dd1..433f82d4 100644 --- a/lib/SQL/Translator/Producer/Oracle.pm +++ b/lib/SQL/Translator/Producer/Oracle.pm @@ -95,7 +95,7 @@ $DEBUG = 0 unless defined $DEBUG; use base 'SQL::Translator::Producer'; use SQL::Translator::Schema::Constants; -use SQL::Translator::Utils qw(header_comment); +use SQL::Translator::Utils qw(debug header_comment); use Data::Dumper; my %translate = ( @@ -197,6 +197,7 @@ sub produce { my $delay_constraints = $translator->producer_args->{delay_constraints}; my ($output, $create, @table_defs, @trigger_defs, @index_defs, @constraint_defs); + debug("ORA: Beginning production"); $create .= header_comment unless ($no_comments); my $qt = 1 if $translator->quote_table_names; my $qf = 1 if $translator->quote_field_names; @@ -212,6 +213,7 @@ sub produce { } for my $table ( $schema->get_tables ) { + debug("ORA: Producing for table " . $table->name); my ( $table_def, $trigger_def, $index_def, $constraint_def ) = create_table( $table, { @@ -270,111 +272,116 @@ sub create_table { push @create, "--\n-- Table: $table_name\n--" unless $options->{no_comments}; push @create, qq[DROP TABLE $table_name_q CASCADE CONSTRAINTS] if $options->{add_drop_table}; - my ( %field_name_scope, @field_comments ); - for my $field ( $table->get_fields ) { - my ($field_create, $field_defs, $trigger_defs, $field_comments) = - create_field($field, $options, \%field_name_scope); - push @create, @$field_create if ref $field_create; - push @field_defs, @$field_defs if ref $field_defs; - push @trigger_defs, @$trigger_defs if ref $trigger_defs; - push @field_comments, @$field_comments if ref $field_comments; + my ( %field_name_scope, @field_comments ); + for my $field ( $table->get_fields ) { + debug("ORA: Creating field " . $field->name . "(" . $field->data_type . ")"); + my ($field_create, $field_defs, $trigger_defs, $field_comments) = + create_field($field, $options, \%field_name_scope); + push @create, @$field_create if ref $field_create; + push @field_defs, @$field_defs if ref $field_defs; + push @trigger_defs, @$trigger_defs if ref $trigger_defs; + push @field_comments, @$field_comments if ref $field_comments; + } + + # + # Table options + # + my @table_options; + for my $opt ( $table->options ) { + if ( ref $opt eq 'HASH' ) { + my ( $key, $value ) = each %$opt; + if ( ref $value eq 'ARRAY' ) { + push @table_options, "$key\n(\n". join ("\n", + map { " $_->[0]\t$_->[1]" } + map { [ each %$_ ] } + @$value + )."\n)"; + } + elsif ( !defined $value ) { + push @table_options, $key; + } + else { + push @table_options, "$key $value"; + } } + } - # - # Table options - # - my @table_options; - for my $opt ( $table->options ) { + # + # Table constraints + # + for my $c ( $table->get_constraints ) { + my $constr = create_constraint($c, $options); + push @constraint_defs, $constr if ($constr); + } + + # + # Index Declarations + # + my @index_defs = (); + for my $index ( $table->get_indices ) { + my $index_name = $index->name || ''; + my $index_type = $index->type || NORMAL; + my @fields = map { quote($_, $qf) } $index->fields; + next unless @fields; + debug("ORA: Creating $index_type index on fields (" . join(', ', @fields) . ") named $index_name"); + my @index_options; + for my $opt ( $index->options ) { if ( ref $opt eq 'HASH' ) { my ( $key, $value ) = each %$opt; if ( ref $value eq 'ARRAY' ) { push @table_options, "$key\n(\n". join ("\n", map { " $_->[0]\t$_->[1]" } map { [ each %$_ ] } - @$value + @$value )."\n)"; } elsif ( !defined $value ) { - push @table_options, $key; + push @index_options, $key; } else { - push @table_options, "$key $value"; + push @index_options, "$key $value"; } } } - - # - # Table constraints - # - for my $c ( $table->get_constraints ) { - my $constr = create_constraint($c, $options); - push @constraint_defs, $constr if ($constr); + my $index_options = @index_options + ? "\n".join("\n", @index_options) : ''; + + if ( $index_type eq PRIMARY_KEY ) { + $index_name = $index_name ? mk_name( $index_name ) + : mk_name( $table_name, 'pk' ); + $index_name = quote($index_name, $qf); + push @field_defs, 'CONSTRAINT '.$index_name.' PRIMARY KEY '. + '(' . join( ', ', @fields ) . ')'; } - - # - # Index Declarations - # - my @index_defs = (); - for my $index ( $table->get_indices ) { - my $index_name = $index->name || ''; - my $index_type = $index->type || NORMAL; - my @fields = map { quote($_, $qf) } $index->fields; - next unless @fields; - - my @index_options; - for my $opt ( $index->options ) { - if ( ref $opt eq 'HASH' ) { - my ( $key, $value ) = each %$opt; - if ( ref $value eq 'ARRAY' ) { - push @table_options, "$key\n(\n". join ("\n", - map { " $_->[0]\t$_->[1]" } - map { [ each %$_ ] } - @$value - )."\n)"; - } - elsif ( !defined $value ) { - push @index_options, $key; - } - else { - push @index_options, "$key $value"; - } - } - } - my $index_options = @index_options - ? "\n".join("\n", @index_options) : ''; - - if ( $index_type eq PRIMARY_KEY ) { - $index_name = $index_name ? mk_name( $index_name ) - : mk_name( $table_name, 'pk' ); - $index_name = quote($index_name, $qf); - push @field_defs, 'CONSTRAINT '.$index_name.' PRIMARY KEY '. - '(' . join( ', ', @fields ) . ')'; - } - elsif ($index_type eq NORMAL or $index_type eq UNIQUE) { - push @index_defs, create_index($index, $options, $index_options); - } - else { - warn "Unknown index type ($index_type) on table $table_name.\n" - if $WARN; - } + elsif ($index_type eq NORMAL or $index_type eq UNIQUE) { + push @index_defs, create_index($index, $options, $index_options); } + else { + warn "Unknown index type ($index_type) on table $table_name.\n" + if $WARN; + } + } - if ( my @table_comments = $table->comments ) { - for my $comment ( @table_comments ) { - next unless $comment; - $comment = __PACKAGE__->_quote_string($comment); - push @field_comments, "COMMENT ON TABLE $table_name_q is\n $comment" - unless $options->{no_comments}; - } + if ( my @table_comments = $table->comments ) { + for my $comment ( @table_comments ) { + next unless $comment; + $comment = __PACKAGE__->_quote_string($comment); + push @field_comments, "COMMENT ON TABLE $table_name_q is\n $comment" + unless $options->{no_comments}; } + } - my $table_options = @table_options - ? "\n".join("\n", @table_options) : ''; + my $table_options = @table_options ? "\n".join("\n", @table_options) : ''; + debug("Create is @create"); + debug("Constraints are @constraint_defs"); + debug("Delay_constraints is " . $options->{delay_constraints}); + debug("Field DEFS: " . join(', ', @field_defs)); push @create, "CREATE TABLE $table_name_q (\n" . join( ",\n", map { " $_" } @field_defs, ($options->{delay_constraints} ? () : @constraint_defs) ) . "\n)$table_options"; + debug("NOW Create is @create"); @constraint_defs = map { "ALTER TABLE $table_name_q ADD $_" } @constraint_defs; @@ -546,6 +553,7 @@ sub create_field { # my $default = $field->default_value; if ( defined $default ) { + debug("ORA: Handling default value: $default"); # # Wherein we try to catch a string being used as # a default value for a numerical field. If "true/false," @@ -587,6 +595,7 @@ sub create_field { # Not null constraint # unless ( $field->is_nullable ) { + debug("ORA: Field is NOT NULL"); $field_def .= ' NOT NULL'; } @@ -596,6 +605,7 @@ sub create_field { # Auto_increment # if ( $field->is_auto_increment ) { + debug("ORA: Handling auto increment"); my $base_name = $table_name . "_". $field_name; my $seq_name = quote(mk_name( $base_name, 'sq' ),$qt); my $trigger_name = quote(mk_name( $base_name, 'ai' ),$qt); @@ -621,6 +631,7 @@ sub create_field { push @field_defs, $field_def; if ( my $comment = $field->comments ) { + debug("ORA: Handling comment"); $comment =~ __PACKAGE__->_quote_string($comment); push @field_comments, "COMMENT ON COLUMN $table_name_q.$field_name_q is\n $comment;" @@ -713,6 +724,7 @@ sub create_constraint { my $definition; if ( $c->type eq PRIMARY_KEY ) { + debug("ORA: Creating PK constraint on fields (" . join(', ', @fields) . ")"); # create a name if delay_constraints $name ||= mk_name( $table_name, 'pk' ) if $options->{delay_constraints}; @@ -736,7 +748,7 @@ sub create_constraint { else { $name = mk_name( $table_name, 'u' ); } - + debug("ORA: Creating UNIQUE constraint on fields (" . join(', ', @fields) . ") named $name"); $name = quote($name, $qf); for my $f ( $c->fields ) { @@ -756,6 +768,7 @@ sub create_constraint { $name ||= mk_name( $name || $table_name, 'ck' ); $name = quote($name, $qf); my $expression = $c->expression || ''; + debug("ORA: Creating CHECK constraint on fields (" . join(', ', @fields) . ") named $name"); $definition = "CONSTRAINT $name CHECK ($expression)"; } elsif ( $c->type eq FOREIGN_KEY ) { @@ -770,7 +783,7 @@ sub create_constraint { } my $ref_table = quote($c->reference_table,$qt); - + debug("ORA: Creating FK constraint on fields (" . join(', ', @fields) . ") named $name referencing $ref_table"); $definition .= " REFERENCES $ref_table"; if ( @rfields ) { From a32992dcda181e88963930e914cb6072cf72a089 Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Wed, 5 Oct 2022 11:38:00 -0400 Subject: [PATCH 16/19] Separated FK_defs out so things would play nice and modified 51-xml-to-oracle.t to work with slight changes to output. --- lib/SQL/Translator/Producer/Oracle.pm | 27 +++++++++++-------- t/51-xml-to-oracle.t | 37 ++++++++------------------- 2 files changed, 28 insertions(+), 36 deletions(-) diff --git a/lib/SQL/Translator/Producer/Oracle.pm b/lib/SQL/Translator/Producer/Oracle.pm index 433f82d4..b05ab06a 100644 --- a/lib/SQL/Translator/Producer/Oracle.pm +++ b/lib/SQL/Translator/Producer/Oracle.pm @@ -195,7 +195,7 @@ sub produce { my $schema = $translator->schema; my $oracle_version = $translator->producer_args->{oracle_version} || 0; my $delay_constraints = $translator->producer_args->{delay_constraints}; - my ($output, $create, @table_defs, @trigger_defs, @index_defs, @constraint_defs); + my ($output, $create, @table_defs, @fk_defs, @trigger_defs, @index_defs, @constraint_defs); debug("ORA: Beginning production"); $create .= header_comment unless ($no_comments); @@ -214,7 +214,7 @@ sub produce { for my $table ( $schema->get_tables ) { debug("ORA: Producing for table " . $table->name); - my ( $table_def, $trigger_def, $index_def, $constraint_def ) = create_table( + my ( $table_def, $fk_def, $trigger_def, $index_def, $constraint_def ) = create_table( $table, { add_drop_table => $add_drop_table, @@ -226,6 +226,7 @@ sub produce { } ); push @table_defs, @$table_def; + push @fk_defs, @$fk_def; push @trigger_defs, @$trigger_def; push @index_defs, @$index_def; push @constraint_defs, @$constraint_def; @@ -244,10 +245,10 @@ sub produce { } if (wantarray) { - return defined $create ? $create : (), @table_defs, @view_defs, @trigger_defs, @index_defs, @constraint_defs; + return defined $create ? $create : (), @table_defs, @view_defs, @fk_defs, @trigger_defs, @index_defs, @constraint_defs; } else { - $create .= join (";\n\n", @table_defs, @view_defs, @index_defs, @constraint_defs); + $create .= join (";\n\n", @table_defs, @view_defs, @fk_defs, @index_defs, @constraint_defs); $create .= ";\n\n"; # If wantarray is not set we have to add "/" in this statement # DBI->do() needs them omitted @@ -267,7 +268,7 @@ sub create_table { my $item = ''; my $drop; - my (@create, @field_defs, @constraint_defs, @trigger_defs); + my (@create, @field_defs, @constraint_defs, @fk_defs, @trigger_defs); push @create, "--\n-- Table: $table_name\n--" unless $options->{no_comments}; push @create, qq[DROP TABLE $table_name_q CASCADE CONSTRAINTS] if $options->{add_drop_table}; @@ -311,7 +312,14 @@ sub create_table { # for my $c ( $table->get_constraints ) { my $constr = create_constraint($c, $options); - push @constraint_defs, $constr if ($constr); + if ($constr) { + if ($c->type eq FOREIGN_KEY) { # FK defs always come later as alters + push @fk_defs, sprintf("ALTER TABLE %s ADD %s", $table_name_q, $constr); + } + else { + push @constraint_defs, $constr; + } + } } # @@ -374,7 +382,6 @@ sub create_table { my $table_options = @table_options ? "\n".join("\n", @table_options) : ''; debug("Create is @create"); debug("Constraints are @constraint_defs"); - debug("Delay_constraints is " . $options->{delay_constraints}); debug("Field DEFS: " . join(', ', @field_defs)); push @create, "CREATE TABLE $table_name_q (\n" . join( ",\n", map { " $_" } @field_defs, @@ -382,9 +389,9 @@ sub create_table { "\n)$table_options"; debug("NOW Create is @create"); - @constraint_defs = map { "ALTER TABLE $table_name_q ADD $_" } - @constraint_defs; + @constraint_defs = map { "ALTER TABLE $table_name_q ADD $_" } @constraint_defs; + debug("Now constraint defs are " . join(', ', @constraint_defs)); if ( $WARN ) { if ( %truncated ) { warn "Truncated " . keys( %truncated ) . " names:\n"; @@ -392,7 +399,7 @@ sub create_table { } } - return \@create, \@trigger_defs, \@index_defs, ($options->{delay_constraints} ? \@constraint_defs : []); + return \@create, \@fk_defs, \@trigger_defs, \@index_defs, ($options->{delay_constraints} ? \@constraint_defs : []); } sub alter_field { diff --git a/t/51-xml-to-oracle.t b/t/51-xml-to-oracle.t index 2486246c..51c95600 100644 --- a/t/51-xml-to-oracle.t +++ b/t/51-xml-to-oracle.t @@ -39,7 +39,7 @@ my $sql_string = $sqlt->translate( to => 'Oracle', filename => $xmlfile, ) or die $sqlt->error; - +warn "SQL: " . join("\n", @sql) . "\n"; my $want = [ 'DROP TABLE Basic CASCADE CONSTRAINTS', 'DROP SEQUENCE sq_Basic_id', @@ -58,19 +58,19 @@ my $want = [ CONSTRAINT u_Basic_emailuniqueindex UNIQUE (email), CONSTRAINT u_Basic_very_long_index_name_o UNIQUE (title) )', - 'DROP TABLE Another CASCADE CONSTRAINTS', - 'DROP SEQUENCE sq_Another_id', - 'CREATE SEQUENCE sq_Another_id', - 'CREATE TABLE Another ( +'DROP TABLE Another CASCADE CONSTRAINTS', +'DROP SEQUENCE sq_Another_id', +'CREATE SEQUENCE sq_Another_id', +'CREATE TABLE Another ( id number(10) NOT NULL, num number(10,2), PRIMARY KEY (id) )', 'DROP VIEW email_list', - 'CREATE VIEW email_list AS +'CREATE VIEW email_list AS SELECT email FROM Basic WHERE (email IS NOT NULL)', - 'ALTER TABLE Basic ADD CONSTRAINT Basic_another_id_fk FOREIGN KEY (another_id) REFERENCES Another (id)', - 'CREATE OR REPLACE TRIGGER ai_Basic_id +'ALTER TABLE Basic ADD CONSTRAINT Basic_another_id_fk FOREIGN KEY (another_id) REFERENCES Another (id)', +'CREATE OR REPLACE TRIGGER ai_Basic_id BEFORE INSERT ON Basic FOR EACH ROW WHEN ( new.id IS NULL OR new.id = 0 @@ -81,14 +81,7 @@ BEGIN FROM dual; END; ', - 'CREATE OR REPLACE TRIGGER ts_Basic_timest -BEFORE INSERT OR UPDATE ON Basic -FOR EACH ROW WHEN (new.timest IS NULL) -BEGIN - SELECT sysdate INTO :new.timest FROM dual; -END; -', - 'CREATE OR REPLACE TRIGGER ai_Another_id +'CREATE OR REPLACE TRIGGER ai_Another_id BEFORE INSERT ON Another FOR EACH ROW WHEN ( new.id IS NULL OR new.id = 0 @@ -99,7 +92,7 @@ BEGIN FROM dual; END; ', -'CREATE INDEX titleindex on Basic (title)']; +'CREATE INDEX titleindex ON Basic (title)']; is_deeply(\@sql, $want, 'Got correct Oracle statements in list context'); @@ -143,7 +136,7 @@ SELECT email FROM Basic WHERE (email IS NOT NULL); ALTER TABLE Basic ADD CONSTRAINT Basic_another_id_fk01 FOREIGN KEY (another_id) REFERENCES Another (id); -CREATE INDEX titleindex01 on Basic (title); +CREATE INDEX titleindex ON Basic (title); CREATE OR REPLACE TRIGGER ai_Basic_id01 BEFORE INSERT ON Basic @@ -157,14 +150,6 @@ BEGIN END; / -CREATE OR REPLACE TRIGGER ts_Basic_timest01 -BEFORE INSERT OR UPDATE ON Basic -FOR EACH ROW WHEN (new.timest IS NULL) -BEGIN - SELECT sysdate INTO :new.timest FROM dual; -END; -/ - CREATE OR REPLACE TRIGGER ai_Another_id01 BEFORE INSERT ON Another FOR EACH ROW WHEN ( From 090b5b471335d9136c6f273d414a6a2929976cd2 Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Wed, 5 Oct 2022 12:01:48 -0400 Subject: [PATCH 17/19] Corrected quoting on index fields, updated 51-xml-to-oracle_quoted.t for slight changes in output and fixed tabbing to be more readable. --- lib/SQL/Translator/Producer/Oracle.pm | 6 ++-- t/51-xml-to-oracle.t | 4 +-- t/51-xml-to-oracle_quoted.t | 41 +++++++++------------------ 3 files changed, 19 insertions(+), 32 deletions(-) diff --git a/lib/SQL/Translator/Producer/Oracle.pm b/lib/SQL/Translator/Producer/Oracle.pm index b05ab06a..816d99a4 100644 --- a/lib/SQL/Translator/Producer/Oracle.pm +++ b/lib/SQL/Translator/Producer/Oracle.pm @@ -672,15 +672,17 @@ sub create_index { $index_options = $index_options || ''; my $qf = $options->{quote_field_names} || $options->{quote_identifiers}; my $qt = $options->{quote_table_names} || $options->{quote_identifiers}; + my $index_name = $index->name || ''; + $index_name = $index_name ? mk_name( $index_name ) : mk_name( $index->table, $index_name || 'i' ); return join( ' ', map { $_ || () } 'CREATE', lc $index->type eq 'normal' ? 'INDEX' : $index->type . ' INDEX', - $index->name ? quote($index->name, $qf): '', + $index_name ? quote($index_name, $qf): '', 'ON', quote($index->table, $qt), - '(' . join( ', ', map { quote($_) } $index->fields ) . ")$index_options" + '(' . join( ', ', map { quote($_, $qf) } $index->fields ) . ")$index_options" ); } diff --git a/t/51-xml-to-oracle.t b/t/51-xml-to-oracle.t index 51c95600..d6049c1d 100644 --- a/t/51-xml-to-oracle.t +++ b/t/51-xml-to-oracle.t @@ -39,7 +39,7 @@ my $sql_string = $sqlt->translate( to => 'Oracle', filename => $xmlfile, ) or die $sqlt->error; -warn "SQL: " . join("\n", @sql) . "\n"; + my $want = [ 'DROP TABLE Basic CASCADE CONSTRAINTS', 'DROP SEQUENCE sq_Basic_id', @@ -136,7 +136,7 @@ SELECT email FROM Basic WHERE (email IS NOT NULL); ALTER TABLE Basic ADD CONSTRAINT Basic_another_id_fk01 FOREIGN KEY (another_id) REFERENCES Another (id); -CREATE INDEX titleindex ON Basic (title); +CREATE INDEX titleindex01 ON Basic (title); CREATE OR REPLACE TRIGGER ai_Basic_id01 BEFORE INSERT ON Basic diff --git a/t/51-xml-to-oracle_quoted.t b/t/51-xml-to-oracle_quoted.t index 6f3a1021..5bbacf14 100644 --- a/t/51-xml-to-oracle_quoted.t +++ b/t/51-xml-to-oracle_quoted.t @@ -42,9 +42,9 @@ my $sql_string = $sqlt->translate( my $want = [ 'DROP TABLE "Basic" CASCADE CONSTRAINTS', - 'DROP SEQUENCE "sq_Basic_id"', - 'CREATE SEQUENCE "sq_Basic_id"', - 'CREATE TABLE "Basic" ( +'DROP SEQUENCE "sq_Basic_id"', +'CREATE SEQUENCE "sq_Basic_id"', +'CREATE TABLE "Basic" ( "id" number(10) NOT NULL, "title" varchar2(100) DEFAULT \'hello\' NOT NULL, "description" clob DEFAULT \'\', @@ -58,19 +58,19 @@ my $want = [ CONSTRAINT "u_Basic_emailuniqueindex" UNIQUE ("email"), CONSTRAINT "u_Basic_very_long_index_name_o" UNIQUE ("title") )', - 'DROP TABLE "Another" CASCADE CONSTRAINTS', - 'DROP SEQUENCE "sq_Another_id"', - 'CREATE SEQUENCE "sq_Another_id"', - 'CREATE TABLE "Another" ( +'DROP TABLE "Another" CASCADE CONSTRAINTS', +'DROP SEQUENCE "sq_Another_id"', +'CREATE SEQUENCE "sq_Another_id"', +'CREATE TABLE "Another" ( "id" number(10) NOT NULL, "num" number(10,2), PRIMARY KEY ("id") )', 'DROP VIEW "email_list"', - 'CREATE VIEW "email_list" AS +'CREATE VIEW "email_list" AS SELECT email FROM Basic WHERE (email IS NOT NULL)', - 'ALTER TABLE "Basic" ADD CONSTRAINT "Basic_another_id_fk" FOREIGN KEY ("another_id") REFERENCES "Another" ("id")', - 'CREATE OR REPLACE TRIGGER "ai_Basic_id" +'ALTER TABLE "Basic" ADD CONSTRAINT "Basic_another_id_fk" FOREIGN KEY ("another_id") REFERENCES "Another" ("id")', +'CREATE OR REPLACE TRIGGER "ai_Basic_id" BEFORE INSERT ON "Basic" FOR EACH ROW WHEN ( new."id" IS NULL OR new."id" = 0 @@ -81,14 +81,7 @@ BEGIN FROM dual; END; ', - 'CREATE OR REPLACE TRIGGER "ts_Basic_timest" -BEFORE INSERT OR UPDATE ON "Basic" -FOR EACH ROW WHEN (new."timest" IS NULL) -BEGIN - SELECT sysdate INTO :new."timest" FROM dual; -END; -', - 'CREATE OR REPLACE TRIGGER "ai_Another_id" +'CREATE OR REPLACE TRIGGER "ai_Another_id" BEFORE INSERT ON "Another" FOR EACH ROW WHEN ( new."id" IS NULL OR new."id" = 0 @@ -99,7 +92,7 @@ BEGIN FROM dual; END; ', -'CREATE INDEX "titleindex" on "Basic" ("title")']; +'CREATE INDEX "titleindex" ON "Basic" ("title")']; is_deeply(\@sql, $want, 'Got correct Oracle statements in list context'); @@ -143,7 +136,7 @@ SELECT email FROM Basic WHERE (email IS NOT NULL); ALTER TABLE "Basic" ADD CONSTRAINT "Basic_another_id_fk01" FOREIGN KEY ("another_id") REFERENCES "Another" ("id"); -CREATE INDEX "titleindex01" on "Basic" ("title"); +CREATE INDEX "titleindex01" ON "Basic" ("title"); CREATE OR REPLACE TRIGGER "ai_Basic_id01" BEFORE INSERT ON "Basic" @@ -157,14 +150,6 @@ BEGIN END; / -CREATE OR REPLACE TRIGGER "ts_Basic_timest01" -BEFORE INSERT OR UPDATE ON "Basic" -FOR EACH ROW WHEN (new."timest" IS NULL) -BEGIN - SELECT sysdate INTO :new."timest" FROM dual; -END; -/ - CREATE OR REPLACE TRIGGER "ai_Another_id01" BEFORE INSERT ON "Another" FOR EACH ROW WHEN ( From 74c0312411dac7710992d136d9856eff1fc356b0 Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Wed, 5 Oct 2022 12:09:29 -0400 Subject: [PATCH 18/19] Corrected changes previously made to 55-oracle-producer.t in error. --- t/55-oracle-producer.t | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/t/55-oracle-producer.t b/t/55-oracle-producer.t index beeb294a..f22f03a1 100644 --- a/t/55-oracle-producer.t +++ b/t/55-oracle-producer.t @@ -69,12 +69,17 @@ use SQL::Translator::Producer::Oracle; type => FOREIGN_KEY, ); - my ($table1_def, $trigger1_def, + my ($table1_def, $fk1_def, $trigger1_def, $index1_def, $constraint1_def ) = SQL::Translator::Producer::Oracle::create_table($table1); - like($table1_def->[1], '/CONSTRAINT table1_fk_col1_fk_col2_fk FOREIGN KEY \(fk_col1, fk_col2\) REFERENCES table2 \(fk_col1, fk_col2\)/', 'correct "CONSTRAINT" SQL'); - + is_deeply( + $fk1_def, + [ 'ALTER TABLE table1 ADD CONSTRAINT table1_fk_col1_fk_col2_fk FOREIGN KEY (fk_col1, fk_col2) REFERENCES table2 (fk_col1, fk_col2)' + ], + 'correct "CREATE CONSTRAINT" SQL' + ); + my $materialized_view = SQL::Translator::Schema::View->new( name => 'matview', sql => 'SELECT id, name FROM table3', From 2a66d69c4dfd53ffc29b7637ff56d56c3bd364ec Mon Sep 17 00:00:00 2001 From: Ben Vallerand Date: Wed, 5 Oct 2022 14:39:47 -0400 Subject: [PATCH 19/19] Removed some unnecessary debug comments. --- lib/SQL/Translator/Producer/Oracle.pm | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/SQL/Translator/Producer/Oracle.pm b/lib/SQL/Translator/Producer/Oracle.pm index 816d99a4..dae39e5f 100644 --- a/lib/SQL/Translator/Producer/Oracle.pm +++ b/lib/SQL/Translator/Producer/Oracle.pm @@ -380,18 +380,13 @@ sub create_table { } my $table_options = @table_options ? "\n".join("\n", @table_options) : ''; - debug("Create is @create"); - debug("Constraints are @constraint_defs"); - debug("Field DEFS: " . join(', ', @field_defs)); push @create, "CREATE TABLE $table_name_q (\n" . join( ",\n", map { " $_" } @field_defs, ($options->{delay_constraints} ? () : @constraint_defs) ) . "\n)$table_options"; - debug("NOW Create is @create"); @constraint_defs = map { "ALTER TABLE $table_name_q ADD $_" } @constraint_defs; - debug("Now constraint defs are " . join(', ', @constraint_defs)); if ( $WARN ) { if ( %truncated ) { warn "Truncated " . keys( %truncated ) . " names:\n";