Skip to content

Conversation

@Munksgaard
Copy link
Contributor

As discussed on the mailing list, this PR add support for specifying the index direction when creating a new index.

Most of the changes are pretty straightforward, but in the Tds and Postgres adapters, I did have to split up the old index_expr/1 into two different functions: one to compute the index expression and one to compute the include expression. Previously they shared the same function, but now that the domains of the functions are different I don't think they should.

I also added two more types to Ecto.Migration to make sure the type of the columns field is correct.

Finally, I chose to ignore the sorting direction when generating the index name, so that functionality also had to be slightly amended.

@Munksgaard
Copy link
Contributor Author

Some of the tests seemed a bit flaky when I tried to run them locally, but the new ones I wrote works for me. It doesn't look like indexes are tested in our integration tests, so I haven't added anything there. The generated postgres queries look correct, but I'd greatly appreciate it if someone with more experience in MySQL and MSSQL could tell me if the queries for those databases are also correct.

@Munksgaard
Copy link
Contributor Author

Munksgaard commented Mar 24, 2025

Oh, and I should probably say that, whether or not to actually include this in ecto_sql is still up for discussion. I think it is useful, but maybe others disagree. This PR was mostly to figure out how intrusive the implementation would be.

@josevalim
Copy link
Member

I am happy with this addition. @greg-rychlewski, feel free to merge it if you are happy too.

As discussed [on the mailing list], this PR add support for specifying the index
direction when creating a new index.

[on the mailing list]:
https://groups.google.com/g/elixir-ecto/c/uYFE2evwsW4/m/2q5QwQH2BAAJ

Most of the changes are pretty straightforward, but in the Tds and Postgres
adapters, I did have to split up the old `index_expr/1` into two different
functions: one to compute the index expression and one to compute the include
expression. Previously they shared the same function, but now that the domains
of the functions are different I don't think they should.

I also added two more types to `Ecto.Migration` to make sure the type of the
`columns` field is correct.

Finally, I chose to ignore the sorting direction when generating the index name,
so that functionality also had to be slightly amended.
@greg-rychlewski greg-rychlewski merged commit 8dc5a77 into elixir-ecto:master Mar 26, 2025
11 checks passed
@greg-rychlewski
Copy link
Member

Thank you! This is a very nice feature =)

@Munksgaard
Copy link
Contributor Author

It was my pleasure ❤️

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