[DBIx-Class-Devel] [dbsrgits/sql-translator] Oracle producer add missing functions (PR #143)

[mohawk2]

@mohawk2 requested changes on this pull request.



> -                $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";

"CAlling" typo

> @@ -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' ) {

I don't think it's good practice to leave slabs of commented code. If you want to delete it, delete it.

> +
+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);

I don't think `warn` in tests is good practice?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/dbsrgits/sql-translator/pull/143#pullrequestreview-1102428834
You are receiving this because you are subscribed to this thread.

Message ID: <dbsrgits/sql-translator/pull/143/review/1102428834 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.scsys.co.uk/pipermail/dbix-class-devel/attachments/20220909/6c1bda21/attachment.htm>