Implement schema .setNullable/.dropNullable. Fixes #1218 #1327
Conversation
wubzz
force-pushed
the
feature/schema_setnullable
branch
from
f1194e0
to
f3a0245
Compare
|
TODO
|
…Nullable(column) instead.
wubzz
changed the title
wubzz
force-pushed
the
feature/schema_setnullable
branch
from
a5e62a6
to
88a9c01
Compare
| let nullableType = nullable ? 'null' : 'not null'; | ||
| let columnType = columnInfo.type + (columnInfo.maxLength ? `(${columnInfo.maxLength})` : ''); | ||
| let defaultValue = (columnInfo.defaultValue !== null && columnInfo.defaultValue !== void 0) ? `default '${columnInfo.defaultValue}'` : ''; | ||
| let sql = `alter table ${tableName} modify column ${columnName} ${columnType} ${nullableType} ${defaultValue}`; |
There was a problem hiding this comment.
There is quite a lot copy-paste between this and mssql implementation... could "modify column" string be moved to separate variable and just that variable would be overridden in mssql dialect?
|
@elhigu Good points, fixed! 👍 @tgriesser I saw #46 earlier today and wish I had seen it earlier. You probably notice this code is very similar to |
|
@wubzz This is consistent with I could see the appeal of something more chainable: table.changeColumn('some_column').null(false).default('empty').unique(false);I think we can merge this though. What do you think? |
|
@rhys-vdw Yeah I read #46 after doing this PR, and I agree a lot of what is mentioned in that issue can and would be better chainable. Would also allow for less duplicated code. I think we should merge it for now, and then move the code out later on if we end up doing some alter-column specific chaining. |
|
Any update on this pull request? This feature would be very nice to have. |
|
@nrempel agree here. Any updates? Looking forward for a merge. |
|
Leaving the decision up to @tgriesser on this one, considering #46 . |
|
Hi guys! any updates here? |
|
I think this was implemented different way by .alter() column syntax |
This adds.setNullable(column, [true|false])to alter a columns nullable constraint.This adds
.setNullable(column)and.dropNullable(column)to alter a columns nullable constraint.I'm not too happy with the implementation having to call
SELECT 1, but this was the only way I found to support async actions, which are required in this case. See for examplerenameColumnfor a similar case.This is built around the information gathered in #1218 , so feel free to correct if anything is wrong.