Skip to content

Conversation

rgraff
Copy link
Contributor

@rgraff rgraff commented Oct 20, 2025

such as @disable_ddl_transaction and @disable_migration_lock

Fixes #610

I was not able to get a failing test for this bug. Ecto would not error, but would give a warning. I have two tests and one generates a warning and the other does not. Since they both pass (even with warnings), it's not a reliable regression test.

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

@rgraff rgraff force-pushed the fix/tenant-migrations-should-honor-module-attributes branch from 71577c9 to 3073ca0 Compare October 20, 2025 23:22
such as @disable_ddl_transaction and @disable_migration_lock
@rgraff rgraff force-pushed the fix/tenant-migrations-should-honor-module-attributes branch from 3073ca0 to 89c913b Compare October 20, 2025 23:26
@rgraff rgraff closed this Oct 20, 2025
@rgraff rgraff reopened this Oct 20, 2025
@rgraff
Copy link
Contributor Author

rgraff commented Oct 20, 2025

While this fix may work--I'm not 100% certain given the difficulty in testing.

And there's another complication. You still can't do do schema migrations that use the concurrently: true option in test suites, since sandboxes are always in a transaction.

Maybe the migration generator needs to do something like this

create index(:posts, [:title], concurrently: Mix.env() != :test)

@zachdaniel
Copy link
Contributor

Yeah, that makes sense 🤔 We probably will need to figure some other kind of thing out here because you could also be running this in an action that is in a transaction. I'm not sure what the right answer is? Maybe some kind of error if you are in a transaction currently, that can be silenced in test?

@rgraff
Copy link
Contributor Author

rgraff commented Oct 21, 2025 via email

@zachdaniel
Copy link
Contributor

That is a good point. Then future migrations are applied "out of band" out of a transaction. Smart 👍

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

Successfully merging this pull request may close these issues.

Creating indexes concurrently fails for schema-driven multi-tenancy

2 participants