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

feat: Add support for nulls_distinct in custom indexes #175

Closed
wants to merge 4 commits into from

Conversation

sezaru
Copy link
Contributor

@sezaru sezaru commented Oct 31, 2023

This will add support for the nulls_distinct option for PG 15

More information in https://hexdocs.pm/ecto_sql/Ecto.Migration.html#index/3-the-nulls_distinct-option

Contributor checklist

  • Bug fixes include regression tests
  • Features include unit/acceptance tests

Old snapshots can come without the `nulls_distinct` field, because of
that we return null if the key doesn't exists
@sezaru
Copy link
Contributor Author

sezaru commented Oct 31, 2023

Not sure why the mix test step fails, it runs just fine locally and the logs don't seem to give me a good reason

@jimsynz
Copy link
Contributor

jimsynz commented Oct 31, 2023

It's failing because there are migrations that should be generated.

@zachdaniel
Copy link
Contributor

So the fact that this generates migrations for existing unchanged things is problematic. To address this, when loading a snapshot, we need to set the nulls_distinct? option to false if the option is not set.

@zachdaniel
Copy link
Contributor

  defp load_custom_indexes(custom_indexes) do
    Enum.map(custom_indexes || [], fn custom_index ->
      custom_index
      |> Map.put_new(:fields, [])
      |> Map.put_new(:include, [])
      |> Map.put_new(:message, nil)
      |> Map.put_new(:nulls_distinct?, false) # something like this
    end)
  end

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