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

[Feature] Leverage IF NOT EXISTS / IF EXISTS syntax with create index / drop index (Postgres) #247

Closed
tlvenn opened this issue Jul 13, 2020 · 6 comments · Fixed by #253
Closed

Comments

@tlvenn
Copy link
Contributor

tlvenn commented Jul 13, 2020

As discussed in jumpn/ecto_cockroachdb#4, the way Ecto deal with creating / dropping indexes when if_not_exists / if_exists is used does not leverage support for "IF NOT EXISTS" / "IF EXISTS" added to Postgres 9.5 when dealing with indexes.

Instead it uses a workaround using the $$ do syntax which is not supported by CockroachDB.

I noticed that the usage of "IF NOT EXISTS" / "IF EXISTS" when dealing with tables was added to Postgres with the 9.1 version and is already being used by the adaptor:

if_do(command == :create_if_not_exists, "IF NOT EXISTS "),

We could simply do the same for indexes but this would introduce a breaking change for anyone leveraging this feature and still using any Postgres version below 9.5.

But 9.5 is the oldest version actually still supported by Postgres so that might be ok ?
It's not clear what compatibility with Postgres is actually currently targeted by the adaptor. As noted above, given the usage of IF NOT EXISTS with tables, it seems to indicate it is at least 9.1.

@josevalim
Copy link
Member

Next version will be v3.5 so it may be fine to drop support for Postgres earlier than v9.5 as they are no longer supported. I have sent a PR here: #248

Meanwhile, a PR to update the syntax we use for indexes is welcome. Thank you!

@tlvenn
Copy link
Contributor Author

tlvenn commented Jul 13, 2020

Awesome, I will take care of that, thanks @josevalim !

@josevalim
Copy link
Member

Ping! :)

@tlvenn
Copy link
Contributor Author

tlvenn commented Jul 25, 2020

I will pong back very soon @josevalim ;)

@tlvenn
Copy link
Contributor Author

tlvenn commented Jul 26, 2020

Any idea of the "concurrent index and create_if_not_exists is not supported by the Postgres adapter" was a product of using the $$ do syntax or something else is at play here ?

@josevalim
Copy link
Member

@tlvenn see discussion here: elixir-ecto/ecto#2703 (comment)

It seems we can enable this functionality once we rewrite since we now provide a mechanism to disable the lock too.

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 a pull request may close this issue.

2 participants