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

Support :prefix on index rename #573

Merged

Conversation

maltoe
Copy link
Contributor

@maltoe maltoe commented Nov 14, 2023

Hi 👋

we wanted to rename an index in a schema on Postgres like the following but the prefix was ignored. This patch adds support for :prefix for Postgres.

rename index(:foo, [:column], prefix: "bar", name: "old"), to: "new"

@maltoe
Copy link
Contributor Author

maltoe commented Nov 14, 2023

The mysql integration test failure appears flaky, dependent on order of selected comments_authors assoc. Would you like me to fix this in this PR?

[edit] nevermind, the test case lives in ecto

@josevalim
Copy link
Member

A separate PR is welcome, please!

@maltoe
Copy link
Contributor Author

maltoe commented Nov 14, 2023

A separate PR is welcome, please!

The flaky test already been fixed on Ecto's master elixir-ecto/ecto@ddaa2cf

🤔 It seems the :git dependency on Ecto is cached in the GithubActions workflow and hence isn't always up to date? Maybe deps/ecto should be excluded from the cache?

@josevalim
Copy link
Member

We would have to update the lock file, but we did that on main anyway as part of v3.11.0. :)

@maltoe
Copy link
Contributor Author

maltoe commented Nov 14, 2023

We would have to update the lock file, but we did that on main anyway as part of v3.11.0. :)

Right, the lock file is in the cache key, didn't see that.

Not sure I get the rest of your statement though 🙈 - is there anything else I can do for this PR? Are you saying I should update the lock on ecto in this one?

@josevalim
Copy link
Member

This PR is good, I am just waiting for a second opinion (who can go ahead and merge it!).

@greg-rychlewski
Copy link
Member

Jose I think you may have forgotten to push the 3.11 commit to github for Ecto SQL

@josevalim
Copy link
Member

Jose I think you may have forgotten to push the 3.11 commit to github for Ecto SQL

Done!

@maltoe maltoe force-pushed the support-prefix-on-index-rename branch from fe3aaec to e0fbf45 Compare November 15, 2023 08:08
@greg-rychlewski greg-rychlewski merged commit 43bc08d into elixir-ecto:master Nov 15, 2023
10 checks passed
@greg-rychlewski
Copy link
Member

Thanks!

@maltoe maltoe deleted the support-prefix-on-index-rename branch November 15, 2023 08:53
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.

3 participants