Issue-338: Adjust handling of ban-concurrent-index-creation-in-transaction

[janrueth]

This changes the logic of ban-concurrent-index-creation-in-transaction when assume_in_transaction is set. Tools like golang-migrate always wrap migrations in transactions but support individual statements like CREATE INDEX CONCURRENTLY by not wrapping them in transactions.

To support this, this commit alters the logic of ban-concurrent-index-creation-in-transaction to allow a standalone index create command even when assume_in_transaction is true.

[netlify]

👷 Deploy request for squawkhq pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	bce0bab

[chdsbd]

Thanks for the quick PR! And the tests and documentation!

[lmarburger]

This is very useful. Thanks for making this check more robust!

Sadly, our migration tool isn't as clever as golang-migrate and everything's always run inside a transaction. 😕 I'm not familiar with Rust or the squawk codebase. Would there be a simple way to add a flag to revert to the previous behavior?

[chdsbd]

@lmarburger Maybe we can add a flag for this special behavior and default to the old behavior.

As a workaround you can skip this rule for now via:

squawk -e ban-concurrent-index-creation-in-transaction

[lmarburger]

I wouldn't add this if I'm the only one affected. I did some more digging and it turns out we have more issues that complicate linting. The internal database migration tool I inherited is heavily based on dbmate. A migration can add transaction:false to the magic comment to run the migration outside of a transaction. It'd be way too much work to get this supported "correctly". We get enough value simply running squawk --assume-in-transaction upload-to-github ... and treating the reports as advisory knowing there might be some false positives or negatives.

Thanks for the help and especially for maintaining this tool. It's been great!