Add MSSQL support for :cascade option in drop_column #518

Merged
merged 1 commit into from Jul 16, 2012

Conversation

Projects
None yet
2 participants
@munkyboy

When :cascade option is given to #drop_column, the MSSQL adapter will attempt to remove the default value constraint before dropping the column.

@jeremyevans

This comment has been minimized.

Show comment
Hide comment
@jeremyevans

jeremyevans Jul 16, 2012

Owner

I think it's probably best to not require a separate :cascade option. Anytime you are dropping the column, you are going to want to drop the constraint. On all other databases, dropping a column with default does not require a separate option, so I don't think it should require one on MSSQL.

This is going to conflict with one of my local patches that I'm currently testing, but I'll merge it and fix the conflicts.

Thanks for the help!

Owner

jeremyevans commented Jul 16, 2012

I think it's probably best to not require a separate :cascade option. Anytime you are dropping the column, you are going to want to drop the constraint. On all other databases, dropping a column with default does not require a separate option, so I don't think it should require one on MSSQL.

This is going to conflict with one of my local patches that I'm currently testing, but I'll merge it and fix the conflicts.

Thanks for the help!

@munkyboy

This comment has been minimized.

Show comment
Hide comment
@munkyboy

munkyboy Jul 16, 2012

I agree. I felt weird writing this test

I agree. I felt weird writing this test

@jeremyevans

This comment has been minimized.

Show comment
Hide comment
@jeremyevans

jeremyevans Jul 16, 2012

These lines use Symbol#[]. That is part of the core extensions, which should not be used internally. Even then, it only works on ruby 1.8 (as ruby 1.9 adds Symbol#[] and Sequel doesn't override it).

I've made some post 3.37.0 changes so that the specs aren't run with the core extensions, which will make it easier to catch internal usage.

These lines use Symbol#[]. That is part of the core extensions, which should not be used internally. Even then, it only works on ruby 1.8 (as ruby 1.9 adds Symbol#[] and Sequel doesn't override it).

I've made some post 3.37.0 changes so that the specs aren't run with the core extensions, which will make it easier to catch internal usage.

@jeremyevans jeremyevans merged commit c956375 into jeremyevans:master Jul 16, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment