korma / Korma

Tasty SQL for Clojure.
http://sqlkorma.com
1.48k stars 222 forks source link

with and many-to-many generates n+1 queries #371

Open erez-rabih opened 7 years ago

erez-rabih commented 7 years ago

We have a many to many connection between users and accounts There is a join table of users_accounts When we (select users (with accounts)):

  1. There is a select query on the users table
  2. For each user, there is a separate select query on the accounts table to get all users accounts. This is the dry-run result:
    dry run :: SELECT "users".* FROM "users" WHERE ("users"."source" = ?) ORDER BY "users"."id" DESC LIMIT 10 :: [sd]
    dry run :: SELECT "accounts".* FROM "accounts" INNER JOIN "accounts_users" ON "accounts_users"."account_id" = "accounts"."id" WHERE ("accounts_users"."user_id" = ?) :: [1]

    We only see here one of the second query but in reality the second query repeats for each user.

Isn't this N+1 queries? Is there a way to avoid it?

immoh commented 7 years ago

Yes, Korma fetches relations lazily for each entity so this is n+1 queries. You should be able to avoid it by using join instead of with.

erez-rabih commented 7 years ago

I used join but the returned map is really hard to understand:

  1. Columns that belong to both entities are named column and column_2
  2. The map is flat and there is no way to distinct between columns of entity 1 and entity 2

I ended up with two queries, much similar to the way you handle has_many relations. Is there a reason the same trick is not done on many_to_many relations?

immoh commented 7 years ago

Yes, join has that drawback.

I am not sure what you mean by "the way Korma handles has-many relations". Has-many generates n+1 queries as well but without join table.

erez-rabih commented 7 years ago

There is a with-batch directive which allows you to fetch with two queries instead of n+1 but it only applies to has-many relations. I was wondering if there's anything preventing us from implementing the same for many-to-many.

erez-rabih commented 7 years ago

See here: https://github.com/korma/Korma/blob/5bb4e634ed27116bb316a36318c7320cc33163d2/src/korma/core.clj#L931

immoh commented 7 years ago

Right. I am not sure why it hasn't been implemented for many-to-many. PR welcome :)

mrkam2 commented 7 years ago

I guess similar problem is also meant in this issue: https://github.com/korma/korma.incubator/issues/7

venantius commented 6 years ago

A PR addressing this issue would be welcome.

I'm closing older tickets. Is this still a priority?

If there hasn't been a response to this issue in 2 weeks, I'll close the ticket.

mrkam2 commented 6 years ago

I believe it is still relevant.