Skip to content

Respect index option param.#288

Merged
jinzhu merged 1 commit intogo-gorm:masterfrom
abbyssoul:master
Nov 21, 2024
Merged

Respect index option param.#288
jinzhu merged 1 commit intogo-gorm:masterfrom
abbyssoul:master

Conversation

@abbyssoul
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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

@gagnonalex
Copy link
Copy Markdown

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
@JacobPotter
Copy link
Copy Markdown

@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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