Oracle producer add missing functions

[hazardv]

Adds functionality to Producer/Oracle.pm to:

• Alter create constraints
• Alter drop constraints
• Drop fields
• Drop tables
• Alter create index
• Alter drop index

Related tests have been added to prove changes and all existing tests pass.
Automatic creation of trigger to insert sysdate to all timestamp fields has been commented out. That should not be done automatically.

Changes were modeled after the structure of Producer/MySQL.pm.

This branch also includes changes from pull request #142

[hazardv]

@mohawk2 The code you reviewed was from a few commits back. This is the first time I am attempting to contribute to a project like this so I am betting that I messed a few things up.

I have been using DBIx::Class and DBIx::Class::Schema::Versioned for another project and while working on that I discovered functionality that was missing from the Oracle producer. I added in the missing functionality and submitted the pull request. I continued with my project, made changes to the data model, attempted to generate the DDL, and discovered a few more things missing from the Oracle producer. So I added those things into the same branch because it seemed reasonable and pushed the changes. I did that whole process one more time and now I am like 97% sure that all of the missing functionality has been implemented and I won't make any more changes to the branch.

Please check out the latest commit b14da74 and let me know if you see any issues.

[mohawk2]

Thanks for the updates.

The current list of commits sort of meanders, which isn't a criticism, but does highlight a possible process-improvement, especially to help reviewers. Since this is on a branch belonging to you, it's safe to force-push it. The way I work is to end up with the branch I'm making having a series of small commits, each of which meaningfully transforms the starting code towards the desired end-state. This often involves squashing together (using rebase "fixup" command) previous versions of a change, with a correction. This means the final list of commits doesn't have things like commented code, then removal of that (as here). Having adjusted the list of commits to a new one, you'd then need to force-push it.

The alternative (which isn't fatal) is that this set of commits will either (if merged in as-is) be slightly untidy for future people looking to see why a piece of code is as it is, or (if squash-merged) a single quite large commit which can take longer to comprehend.

Tagging in @rabbiveesh.

[hazardv]

@mohawk2 Thank you for the pointers. I will try to clean up my commits to be more concise in the future. I found one last issue with altering constraints for Oracle while working on my personal project (dropping an unnamed primary key constraint) and have pushed the change to this branch. At this point, I have personally tested all of the constraint alteration functionality in my other project (as well as adding tests for this project) and am confident in them.

[rabbiveesh]

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

[hazardv]

@rabbiveesh after reverting the change in Diff.pm (and fixing one of my test plans) all of the tests are passing on my box except for t/60roundtrip.t. It is looking for t/data/roundtrip_autogen.yaml which I am unable to locate in my branch or in the current project master.

[rabbiveesh]

Yeah, to get the roundtrip test working you need some script. You can run Makefile.PL, and then run make test(you only need to run Makefile.PL), it should initialize whatever you need.

[hazardv]

@rabbiveesh Sorry about that, I did not realize I had introduced an issue. With the latest commit (74c0312) the tests all pass on my box (including roundtrip).

[rabbiveesh]

could you just remove the debug statements from the Oracle producer?

I mean, unless you think we should keep them?

[hazardv]

@rabbiveesh I added them following how they were used in the MySQL producer, tagged them with "ORA" at the start, and included them sparingly figuring they could be useful for someone debugging their code in the future. Since they only generate output if the user specifies $DEBUG = 1 I didn't think they hurt anything to be there but if you want them removed I will remove them.

[rabbiveesh]

K, wasnt sure if you had missed some in the last commit.
@mohawk2 any thoughts?

[hazardv]

Hey @mohawk2, can you please take a look at this when you get a minute?

[hazardv]

@mohawk2 Happy New Year!!!

[mohawk2]

I'm a bit surprised by the tests being removed from the XML-related tests. If @rabbiveesh is content it's for a good reason then so am I. Otherwise, all looks fine.

[hazardv]

I'm a bit surprised by the tests being removed from the XML-related tests. If @rabbiveesh is content it's for a good reason then so am I. Otherwise, all looks fine.

@mohawk2 The only things I removed from the XML tests were the checks looking for the triggers that previously got created for all timestamp fields. Since there is no reason to create that trigger for all timestamp fields I commented out the code in commit ab42ef8. I then removed the commented-out code in commit b14da74.

[rabbiveesh]

sorry for the delay on this; we're good to go