fix: tenant migrations should honor module attributes #643
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
71577c9 to
3073ca0
Compare
such as @disable_ddl_transaction and @disable_migration_lock
3073ca0 to
89c913b
Compare
Contributor
Author
|
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 Maybe the migration generator needs to do something like this create index(:posts, [:title], concurrently: Mix.env() != :test) |
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? |
Contributor
Author
|
Yeah, it will error in a transaction so at least you’ll know it failed. What about changing the generated migration to include some helpers?
For example you only need it to be concurrent if there are existing records. So a new tenant doesn’t need to concurrency on for the initial migrations.
…On Oct 20, 2025 at 6:13 PM -0700, Zach Daniel ***@***.***>, wrote:
zachdaniel left a comment (ash-project/ash_postgres#643)
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?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you modified the open/close state.Message ID: ***@***.***>
|
Contributor
|
That is a good point. Then future migrations are applied "out of band" out of a transaction. Smart 👍 |
…tributes * main: (23 commits) fix: do not build query filter with or: [] when all records are successfully inserted, for return_skipped_upsert?: true (#659) test: load aggregate with select and sort on from_many relationship (#661) test: add failing test for expr(has_one.function_calc) nil poisoning in batch (#660) test: aggregate with parent ref in relationship filter and sorting on relationship field (#658) test: binding error when loading aggregate (#657) chore(deps): bump ash_sql in the production-dependencies group (#656) test: multi relationship problems (#654) chore: release version v2.6.26 chore: update ash_sql, use new select binding test: add complex calculation tests with filtered aggregates (#652) improvement: add generator to tsvector type (#655) test: add failing test for aggregate filtering on nested first aggregate (#653) chore: update ash_sql and add test docs: Mention :define_ecto_repo? option of use AshPostgres.Repo macro in getting started guide (#648) improvement: verify check constraint attributes at compile time chore: update ash chore: reuse compliance improvement: update deps for bug fixes chore: release version v2.6.25 test: Add failing test for aggregate with parent() + select() + limit() (#644) ...
Contributor
Author
|
@zachdaniel I made some updates changes here to get concurrent indexes working for tenants. I need to add tests and QA this in a personal project, but wanted to get feedback when you're back from vacation. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.