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

[Veesh Goldman]

@rabbiveesh requested changes on this pull request.

Hey, I only ran through a little bit, but then I saw that tests are failing on my box.
I suggest you make sure that all tests pass.

Here's the list of failing files:
t/54-oracle-sql-diff.t      
t/30sqlt-new-diff-mysql.t   
t/30sqlt-new-diff-pgsql.t   
t/30sqlt-new-diff-sqlite.t  
t/51-xml-to-oracle.t        
t/51-xml-to-oracle_quoted.t 

> @@ -92,7 +92,7 @@ sub schema_diff {
     $options ||= {};
 
     my $obj = SQL::Translator::Diff->new( {
-      %$options,
+      sqlt_args => $options,

hey, why did you change this line?
It looks to me like the way it's meant to be used is to passthrough args to SQLT::Diff, one arg of which is the args which are passed down to the inner SQLT instance

> @@ -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

just to get this straight: this is special casing CURRENT_TIMESTAMP b/c it's a magic constant in the Oracle grammar, and you don't want SQLT to quote it down the line?

If that's the case, I don't know if this is the right place to take care of special casing it here. If you look at the SQLite parser, they special case it in the rule for `VALUE`, which makes more sense to me.

> @@ -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

ah, looking at it again, I see that's exactly what the MySQL parser does. So then I'm okay w/ this, though I think the SQLite way makes more sense

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

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