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

How to change nullable constraint in column inside a migration? #1218

Closed
Rush opened this issue Feb 22, 2016 · 11 comments · Fixed by #4657
Closed

How to change nullable constraint in column inside a migration? #1218

Rush opened this issue Feb 22, 2016 · 11 comments · Fixed by #4657

Comments

@Rush
Copy link

Rush commented Feb 22, 2016

I am trying to change a contraint on a column but this just appears to try to re-create the column and fails.

exports.up = function(knex, Promise) {  
  return knex.schema.table("images", function(table) {
    table.string("checksum").notNullable();
  });
};

exports.down = function(knex, Promise) {
  return knex.schema.table("images", function(table) {
    table.string("checksum").nullable();
  });
};
@wubzz
Copy link
Member

wubzz commented Feb 22, 2016

@Rush I really don't think .nullable() and .notNullable() are meant to be used for this. It seems to me you're wanting to run something like

ALTER TABLE tableName ALTER COLUMN columnName DROP NOT NULL
ALTER TABLE tableName ALTER COLUMN columnName SET NOT NULL

And currently I can't find anything in the API aside from .raw that could handle this.

@rhys-vdw What's your take on this? Could .dropNotNull()/.setNotNull() or similar be a worthy addition to the schemaBuilder? It would be very similar to .dropForeign()'s implementation.

@rhys-vdw
Copy link
Member

Could .dropNotNull()/.setNotNull() or similar be a worthy addition to the schemaBuilder?

Yes, but probably as .dropNullable(...columnNames) and .setNullable(...columnNames).

@wubzz
Copy link
Member

wubzz commented Feb 26, 2016

@rhys-vdw On second thought, I feel like this will be too much of an inconvenience for any dialect other than postgres. Personally I see no good solution to the following:

Postgres dropNullable: ALTER TABLE table ALTER COLUMN columnName DROP NOT NULL
Postgres setNullable: ALTER TABLE table ALTER COLUMN columnName SET NOT NULL

MSSQL dropNullable: ALTER TABLE table ALTER COLUMN columnName columnType NULL
MSSQL setNullable: ALTER TABLE table ALTER COLUMN columnName columnType NOT NULL

MySQL/Maria/Oracle dropNullable: ALTER TABLE table MODIFY columnName columnType NULL;
MySQL/Maria/Oracle setNullable: ALTER TABLE table MODIFY columnName columnType NOT NULL;

Sqlite: No support?

It would result in inconsistent arguments based on the dialect you're running. Postgres only wanting name, while others want name + datatype (+ default value?).

What do you think, is it worth?

@Rush
Copy link
Author

Rush commented Feb 26, 2016

We used to support both SQLite and Postgres but SQLite is so different that no amount of abstraction can hide it ... so throwing an exception is probably the right thing in such cases. Rails' ActiveRecord migrations do support changing constraints so maybe that's the place to take inspiration?

@wubzz
Copy link
Member

wubzz commented Apr 2, 2016

This was possible to implement after all. I've made a PR.

@morungos
Copy link

morungos commented Jan 4, 2017

+1 -- I've just come across this issue too, and it's something of a blocker. Any time you need to do a migration that maps representations for non-nullable columns you'll need this.

@elhigu
Copy link
Member

elhigu commented Jan 4, 2017

@morungos not really a blocker, since you can always drop to knex.raw for stuff which for knex doesn't have specialized APIs (lots of stuff when writing migrations).

If this pull request is finished this issue can also be closed #1759

@morungos
Copy link

morungos commented Jan 4, 2017

Yeah, I've used .raw a few times, but if it turns into entire statements, it begins to question the whole use of knee migrations. In the end, I did use a workaround: just default to some non-null weird value, then apply mapping rules to update the values, then drop the original. But it can get hard to do that if you're enforcing integrity, especially with MySQL and it's nasty habit of requiring integrity even during intermediate steps.

@et
Copy link

et commented Feb 20, 2018

For those of you coming here from google, the syntax is:

Converting an existing nonNullable column to be nullable (reverse the up/down if you want a nonNullable to be nullable).

exports.up = knex => {
  return knex.schema
    .alterTable('images', (table) => {
      table.string('checksum').nullable().alter();
    });
};

exports.down = knex => {
  return knex.schema
    .alterTable('images', table => {
      table.string('checksum').notNullable().alter();
    });
};

@knex knex deleted a comment from theanimashaun Mar 12, 2019
@elhigu elhigu closed this as completed Mar 12, 2019
@zizzfizzix
Copy link

Just FYI, in case you're trying to change nullable to nonNullable you first need to fill that column with something, otherwise you'll get an error.

For those of you coming here from google, the syntax is:

Converting an existing nonNullable column to be nullable (reverse the up/down if you want a nonNullable to be nullable).

exports.up = knex => {
  return knex.schema
    .alterTable('images', (table) => {
      table.string('checksum').nullable().alter();
    });
};

exports.down = knex => {
  return knex.schema
    .alterTable('images', table => {
      table.string('checksum').notNullable().alter();
    });
};

@kibertoad
Copy link
Collaborator

Released in 0.95.11

tx0c added a commit to thematters/matters-server that referenced this issue Jan 5, 2022
resolves #2403

need knex@^0.95.11 for `setNullable` `dropNullable`
knex/knex#1218 (comment)
tx0c added a commit to thematters/matters-server that referenced this issue Jan 5, 2022
resolves #2403

need knex@^0.95.11 for `setNullable` `dropNullable`
knex/knex#1218 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants