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

no way to disable transactions in knex 0.8.0+ migrations #834

Closed
jurko-gospodnetic opened this Issue May 19, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@jurko-gospodnetic
Collaborator

jurko-gospodnetic commented May 19, 2015

knex 0.8.0 started wrapping all knex migration steps in separate transactions, and there seems to be no way to turn this behaviour off.

Problem with that is that there are some databases and some commands that just can not be run under an active database transaction. For example, running a ALTER TYPE ... ADD VALUE command on Postgres will fail in such scenario (http://www.postgresql.org/docs/9.4/static/sql-altertype.html).

This is also related to a problem of knex reporting such failures by just logging an end-user warning message and not reporting it on as a regular promise rejection, but I'll log that as a separate issue.

I have not yet tested whether and how I can manually stop and restart knex's database transaction around such problematic commands, but since the whole migration steps can not in theory be safely run under a transaction, there really should be a way to turn this behaviour off.

@tgriesser

This comment has been minimized.

Owner

tgriesser commented May 19, 2015

Ah wow, that's interesting - didn't realize you couldn't do that. Did a little digging and this seems to be how rails fixed it - via disabling ddl transactions on a per-migration basis

I'll look to get something in there to turn this off... would you prefer a flag to have them disabled for all migrations or one which disables on a per-file basis?

@jurko-gospodnetic

This comment has been minimized.

Collaborator

jurko-gospodnetic commented May 19, 2015

Well, for easy backward compatibility, it would definitely be good to be able to disable them all together.

Allowing disabling them for specific files seems like a logical next step, but is not really something I need at this moment so haven't given much thought to how the interface for that would function.

tgriesser added a commit that referenced this issue May 20, 2015

@tgriesser

This comment has been minimized.

Owner

tgriesser commented May 20, 2015

Should now be able to add a disableTransactions option in the migration config and it will not try to wrap things in a transaction.

@tgriesser tgriesser closed this May 20, 2015

@jurko-gospodnetic

This comment has been minimized.

Collaborator

jurko-gospodnetic commented May 20, 2015

Thanks, will give this a try!

Commit's a b*tch to review though, thanks to overwhelming ES5 --> ES6 code changes. :-)

@tgriesser

This comment has been minimized.

Owner

tgriesser commented May 20, 2015

Sorry about that, only real functional change was here

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