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

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

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

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

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

Comments

@jurko-gospodnetic
Copy link
Collaborator

@jurko-gospodnetic 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
Copy link
Member

@tgriesser 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
Copy link
Collaborator Author

@jurko-gospodnetic 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
Copy link
Member

@tgriesser 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
Copy link
Collaborator Author

@jurko-gospodnetic 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
Copy link
Member

@tgriesser 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
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants