Best Style for conditional method calls with Ecto

Questions / Help
1

This isn’t a Phoenix-specific question, rather a general Elixir style one. I am mainly been working in Ruby…

Anyhow, what are better ways to write this:

  ....
  # Insert a state only if the country is "usa"
  state_id = if country == 'usa', do: fetch_or_insert_state(state), else: nil

  def fetch_or_insert_state(state) do
    (Repo.get_by(State, state: state) || insert_state(state)).state_id
  end

  def insert_state(state) do
    case Repo.insert(State.changeset(%State{}, %{state: state})) do
      {:ok, struct} ->
        struct
      {:error, error} ->
        IO.puts "!!! Could not insert State #{error}"
    end
  end

Somehow the state does not populate at all even if country is usa. If I remove this condition, then all inserts right (but then non-US states).

Just looking for better patterns to write this stuff with. I hate how fetch_or_insert_state is written too…

Thank you

2

does it work if you change the single quotes to double quotes? in elixir these are two different data structures

3

That seems like a piece of a bigger logic. What are you trying to do?

4

Basically, only insert a state if country is usa. But I want to write it in an Elixir way, and not Ruby-ish etc. Pattern matching, but I wouldn’t know how - on country being usa or something else…

5

Okay wow, yeah

    country_id = if country == 'usa', do: usa_country_id, else: fetch_or_insert_country(country).country_id
    state_id = if country == 'usa', do: fetch_or_insert_state(state), else: nil

The first one works with the 'usa' but the second one works only with "usa". Why is there this difference? Same variable, and even same style of conditional… Now I’m really confused.

6

'usa' is not a string, but a charlist: Binaries, strings, and charlists — Elixir v1.16.0

1 Like
7

You can check like this, by returning a tuple of multiple

{country_id, state_id} = if country == 'usa', do: {usa_country_id, fetch_or_insert_state(state)}, else: {fetch_or_insert_country(country).country_id, nil}
8

To use pattern matching you could start off like this:

def fetch_state_id("usa", state) do
  found = case Repo.get_by(State, state: state) do
    nil -> insert_state(state)
    state -> state
  end
  found.id
end

def fetch_state_id(_other_country, _state), do: nil

then replace the state_id line with

state_id = fetch_state_id(country, state)

If you have a unique index on the :name field in your database then you can replace the Repo.get_by/2, Repo.insert/2 sequence with an upsert.

And in the above solution you’ll get an error on found.id if the insert fails, your insert_state will return nil.

3 Likes
9

Seems like you have found good suggestions already, but I want to point to the docs where we can find an example of an upsert: Constraints and Upserts — Ecto v3.11.1

defp get_or_insert_tag(name) do
  Repo.get_by(MyApp.Tag, name: name) ||
    Repo.insert!(%Tag{name: name})
end

Good luck!

1 Like
10

Wow this is beautiful. This is saying:

nil -> insert_state(state)
state -> state

some more pattern matching here? Essentially saying if it’s nil, run the insert_state function, otherwise state -> state is using state?

way better than the if-else.

Ended up using yours and @pdgonzalez872 suggestion:

  def fetch_state_id("usa", state) do
    found = case Repo.get_by(State, state: state) do
      nil -> Repo.insert!(%State{state: state})
      state -> state
    end
    found.state_id
  end

Thank you all! Very elegant!

11

If you are using Postgres (not sure about other DB support) you can make it even more elegant with a “true” upsert:

def fetch_state_id("usa", state) do
  state = Repo.insert!(%State{state: state}, on_confict: :nothing)

  state.id
end
3 Likes
12

This is a dangerous approach, as it will not work as expected in a highly concurrent environment, there are chances that get_by will return nil and you will start to have duplicates.

The correct way, as pointed by @tfwright, is to use upsert, since that will be synchronized always correctly by the database.

2 Likes
13

I’m not sure this will work. Ecto returns the original struct if the :returning option is not specified, which won’t have an id. And if nothing is actually upserted then :returning won’t send back anything. You would need to do a dirty hack like update state on conflict.

This has different negative implications depending on your situation, i.e. table bloat, triggers, etc… If we are talking about a highly concurrent environment then table bloat will matter as you’ll be creating many new row versions needlessly.

I would personally use the accepted solution except add a retry mechanism when both select and insert fail.