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

Respect index option param. #288

Merged
merged 1 commit into from
Nov 21, 2024
Merged

Respect index option param. #288

merged 1 commit into from
Nov 21, 2024

Conversation

abbyssoul
Copy link
Contributor

  • [X ] Do only one thing
  • [ X] Non breaking API changes
  • [ X] Tested

What did this pull request do?

When creating indexes, the option argument is taken into account.

User Case Description

When creating indexes, users might need greater control over the resulting DDL statement. For example, users need a way to specify "NULLS [ NOT ] DISTINCT" SQL option when working with Postgres as described here: https://www.postgresql.org/docs/current/sql-createindex.html
Despite documentation stating that option param is supported: https://gorm.io/docs/indexes.html, Postgres Mirgator implementation does not use it. Example of use opened issue that will be closed by this PR: go-gorm/gorm#6085

@abbyssoul
Copy link
Contributor Author

Hi @a631807682 ,
Thank you for approving this PR.
Unfortunately, I don't have write access to this repo. Would you or someone else with permissions, be able to merge it in plz.

@JacobPotter
Copy link

Can this be merged? This is a blocker on a project of mine.

@gagnonalex
Copy link

Can this be merged? This is a blocker on a project of mine.

same!

@jinzhu jinzhu merged commit 50307e2 into go-gorm:master Nov 21, 2024
1 check passed
@JacobPotter
Copy link

@abbyssoul @jinzhu this did not seem to resolve the issue. I have updated to version created from this PR and the options are still not being added when Automigrate is ran

@abbyssoul
Copy link
Contributor Author

Hi @JacobPotter,
Please check if a proper index with options is created for new tables. I suspect Gorm auto migration does not update indexes. That's something to look into next.

@dashrews78
Copy link
Contributor

This change broke the use of setting option:CONCURRENTLY. With this change if you use that option the SQL generated is
CREATE INDEX CONCURRENTLY IF NOT EXISTS "deploymentscontainers_idx" ON "deployments_containers" USING btree("idx") CONCURRENTLY
Blindly appending the options is not good. And why do we push a change without updating a test case?

I will try to follow a ticket with a reproduction later today.

@dashrews78
Copy link
Contributor

#293

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.

6 participants