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

Fixed knex #1577 #1578

Merged
merged 5 commits into from Jul 20, 2016
Merged

Fixed knex #1577 #1578

merged 5 commits into from Jul 20, 2016

Conversation

statyan
Copy link
Contributor

@statyan statyan commented Jul 13, 2016

Also fix an issue with dropPrimary for MSSQL and fix test for primary key - now column test2 is set to notNullable. Without it primarykey test fails 'cause nullable column can't be set for primary key.

@statyan
Copy link
Contributor Author

statyan commented Jul 18, 2016

Oh, don't matter about 3 commits "Fix for installing from github". It do nothing to code. Have no idea why they are listed for this PR, I thought I've added them later.

}).toSQL();

equal(1, tableSql.length);
expect(tableSql[0].sql).to.equal('ALTER TABLE [users] DROP PRIMARY KEY');
expect(tableSql[0].sql).to.equal('ALTER TABLE [users] DROP CONSTRAINT [testconstraintname]');
Copy link
Member

Choose a reason for hiding this comment

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

Is DROP PRIMARY not supported by MSSQL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, DROP CONSTRAINT only. See this article, section "Using Transact-SQL"
https://msdn.microsoft.com/en-us/library/ms190621.aspx

@rhys-vdw
Copy link
Member

@statyan Looks good, I don't know much about MSSQL, but I'll cross reference these changes with the docs in future. Would you be able to squash these commits into one please?

@statyan
Copy link
Contributor Author

statyan commented Jul 19, 2016

Ok,I'll try do it!

@elhigu
Copy link
Member

elhigu commented Jul 19, 2016

@rhys-vdw when you click "merge pull request" button there is option to do squash merge if one likes.

@rhys-vdw rhys-vdw merged commit 6ecd8f3 into knex:master Jul 20, 2016
@rhys-vdw
Copy link
Member

@elhigu Of course, cheers.

Thanks @statyan!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants