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

New ban-concurrent-index-creation-in-transaction in v0.27 breaks workflow #338

Closed
janrueth opened this issue Jan 12, 2024 · 2 comments
Closed

Comments

@janrueth
Copy link
Contributor

Hey,

I like the ban-concurrent-index-creation-in-transaction (Added by @alixlahuec in #335). However, I think the rule needs some adjustment.

I use go-migrate which usually wraps migrations in transactions. As such I'm setting

assume_in_transaction = true

go-migrate is either smart enough to see that single statements don't need to be wrapped in a transaction or that CREATE INDEX CONCURRENTLY must not be wrapped. Either way, having it in a single migration with go-migrate (which usually wraps transactions) works without any issues, it starts failing as soon as I have additional statements in the migration (which makes sense as it need a transaction then).

With the new rule, the linter complains on all concurrent index creations. I don't want to disable assume_in_transaction and I actually want ban-concurrent-index-creation-in-transaction to work here too.

I think the rule could treat assume_in_transaction differently:

For me, a problem only comes when I have a concurrent index creation together with further statements in a migration.

So, I'd propose to change the rule such that if assume_in_transaction is on, that the rule requires more than 1 statement to fail (which would be a useful rule for me to lint on such cases that will break with go-migrate).

Happy to provide a PR if you agree that the behavior should change.

@chdsbd
Copy link
Collaborator

chdsbd commented Jan 12, 2024

Sounds good to me. I'd accept a PR with this change

@chdsbd
Copy link
Collaborator

chdsbd commented Jan 13, 2024

Your PR #339 has been released in v0.28.0. Thanks!

@chdsbd chdsbd closed this as completed Jan 13, 2024
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

No branches or pull requests

2 participants