Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for MS database tools and assemblies #1272

Merged
merged 6 commits into from Feb 13, 2017

Conversation

JeanRev
Copy link
Contributor

@JeanRev JeanRev commented Apr 5, 2016

This pull request targets #1095. Some of the changes made are however going a bit beyond the scope of this issue.

I divided the resulting branch into multiple commits to keep a better overview.

The first two commits (4941aab and e66fc54) extends the current unit tests to include some specific sub-type of database objects, that are handled explicitly in the new clean-up logic implemented in 49a3c99:

  • 4941aab: test inlined and table-valued function.
  • e66fc54: test functions and procedure based on CLR assemblies.

All the revised tests are passing with no modification on the productive code.

49a3c99: add the effective handling of objects generated from Microsoft database tools, along with some specific unit tests.

0cb8f35: add support for cleaning CLR assemblies as well as an additional SQL server object type: user-defined aggregates.

@@ -0,0 +1,173 @@
/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CLR assembly source code. Must be manually compiled and included in the test script as an hexadecimal string (see generate_assembly.txt).

@JeanRev
Copy link
Contributor Author

JeanRev commented Apr 5, 2016

One possible improvement I'm seeing, would be to divide the new tests assets into separate unit test, in order to test more finely the behavior of doClean/doEmpty...

/*
Create Visual Studio tools in schema. Profiled from SQL Server Management studio 2014. For testing purpose only.
*/
IF OBJECT_ID(N'dbo.sp_upgraddiagrams') IS NULL and IS_MEMBER('db_owner') = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the license of these stored procedures? It seems you copied them one to one from somewhere. Can they freely be cloned? What is the original source?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They were profiled from a running MS SQL Studio instance (that is what I intended to say with the comment at the top of the file).

If you think that it could be a problem, I can also write a test script from scratch...

Copy link

Choose a reason for hiding this comment

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

They are provided by Microsoft and are built automatically when creating diagrams using MS SQL Studio.

IP concerns aside, this is potentially a versioning nightmare (Which version of SQL Server were these extracted from? Which versions are they compatible with?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, new test scripts were committed in fc4aff1 (the old scripts will need to be removed from history before merging).

@axelfontaine
Copy link
Contributor

Thanks for the update and the removal of the sources. Looks good now. Could you make this mergeable again?

@JeanRev
Copy link
Contributor Author

JeanRev commented Feb 7, 2017

@axelfontaine of course! I'll need some time to restore my test infrastructure and to review the code once more before merging (it's been awhile).

@JeanRev
Copy link
Contributor Author

JeanRev commented Feb 9, 2017

Ok, I have rebased the PR on master (2d472b8) and removed the old test script from history.

In addition, two conflicts were encountered in the SQLServerSchema class, reflecting two upstream changes:

Otherwise, this PR is unchanged.

@axelfontaine axelfontaine added this to the Flyway 5.0 milestone Feb 9, 2017
@axelfontaine
Copy link
Contributor

Thanks! Looks great now! 👍

This will be part of the next Flyway release.

@JeanRev
Copy link
Contributor Author

JeanRev commented Feb 9, 2017

Great :-) Looking forward to it!

@axelfontaine axelfontaine merged commit 3df2d9b into flyway:master Feb 13, 2017
@axelfontaine
Copy link
Contributor

Merged. Thanks!

dohrayme pushed a commit to dohrayme/flyway that referenced this pull request Feb 3, 2020
Support for MS database tools and assemblies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants