Please peer review my controller action

Heres is the controller action in question.

def show(conn, %{"id" => id}) do
    client =
      Intake.get_client!(id)
      |> Repo.preload([
           :contacts,
           :groups,
           groups: :clients,
           addresses: from(a in MyApp.Intake.Client.Address, order_by: a.as_of)
         ])

    [group | _t] = client.groups
    render(conn, "show.html", client: client, group: group)
  end

As you see I have a few related entities I preload,

What I dont like about this is it does quite a bit of sql querying.

[debug] Processing with MyAppWeb.ClientController.show/2
  Parameters: %{"id" => "11"}
  Pipelines: [:browser, :secure]
[debug] QUERY OK source="clients" db=0.4ms
SELECT c0."id", c0."birthday", c0."city", c0."employeed", c0."first_name", c0."middle_name", c0."last_name", c0."race", c0."state", c0."street_address", c0."zip", c0."email", c0."gender", c0."phone_numbers", c0."inserted_at", c0."updated_at" FROM "clients" AS c0 WHERE (c0."id" = $1) [11]
[debug] QUERY OK source="client_addresses" db=0.4ms
SELECT c0."id", c0."street", c0."city", c0."state", c0."zip", c0."as_of", c0."till", c0."client_id", c0."inserted_at", c0."updated_at", c0."client_id" FROM "client_addresses" AS c0 WHERE (c0."client_id" = $1) ORDER BY c0."client_id", c0."as_of" [11]
[debug] QUERY OK source="client_contacts" db=0.4ms
SELECT c0."id", c0."name", c0."email", c0."street", c0."city", c0."state", c0."zip", c0."notes", c0."emergency", c0."rio_ok", c0."message", c0."client_id", c0."inserted_at", c0."updated_at", c0."client_id" FROM "client_contacts" AS c0 WHERE (c0."client_id" = $1) ORDER BY c0."client_id" [11]
[debug] QUERY OK source="client_groups" db=1.1ms
SELECT c0."id", c0."inserted_at", c0."updated_at", c1."id" FROM "client_groups" AS c0 INNER JOIN "clients" AS c1 ON c1."id" = ANY($1) INNER JOIN "clients_client_groups" AS c2 ON c2."client_id" = c1."id" WHERE (c2."client_group_id" = c0."id") ORDER BY c1."id" [[11]]
[debug] QUERY OK source="clients" db=0.5ms
SELECT c0."id", c0."birthday", c0."city", c0."employeed", c0."first_name", c0."middle_name", c0."last_name", c0."race", c0."state", c0."street_address", c0."zip", c0."email", c0."gender", c0."phone_numbers", c0."inserted_at", c0."updated_at", c1."id" FROM "clients" AS c0 INNER JOIN "client_groups" AS c1 ON c1."id" = ANY($1) INNER JOIN "clients_client_groups" AS c2 ON c2."client_group_id" = c1."id" WHERE (c2."client_id" = c0."id") ORDER BY c1."id" [[7]]

Am I wrong in assuming preloading was doing more to join the queries?
Specifically QUERY OK source="clients" I see happen twice where once it looks like it just gets the clients but then latter it does what looks like the join with the groups.

Its possible the first query is happening else where but I’ve yet to find where.
Any how I’m working alone in a vacuum at my current job so this is the only place I can get feedback on my code as I’m learning.

Thoughts?

1 Like

Using Repo.preload will not do joins, instead it will do a SQL per table, this can actually be quite a bit more efficient on fat tables. If however the tables are not fat then you can use preload: inside a query linking to a join in that same query and it will preload based on the join instead, this can be more efficient on thin tables or few results (but not if you have many shared results or fat tables). Thus it all depends on the usage. :slight_smile:

2 Likes

I would avoid all of this by using pure SQL :slight_smile:

1 Like

Then you lose typechecks, increase your attack surface, and depending on the data stored you would also lose efficiency, and that is assuming you still cache the query properly. :wink:

3 Likes