How do you compose Ecto queries when each queryable function might use or_where / where independently?

The problem is I introduced a 2nd composeable query that uses where along with or_where and now Ecto is composing it with a bunch of other composed queries, but it’s putting the OR in an unexpected spot when it composes everything together.

The parenthesis are ending up in an unfavorable spot in the query and I don’t know where to being to solve this problem.

Here’s the fully composed function:

  def eligible_discounts(package_id, code, select_code \\ false) do
    __MODULE__
    |> for_package(package_id)
    |> with_discount()
    |> enabled()
    |> usage_count_less_than_usage_limit()
    |> maybe_code(code)
    |> maybe_select_code(select_code)
  end

And it is being composed of these functions:

  def for_package(query \\ __MODULE__, package_id) do
    from(q in query,
      where: is_nil(q.package_id),
      or_where: q.package_id == ^package_id
    )
  end

  def with_discount(query \\ __MODULE__) do
    from(q in query, join: d in assoc(q, :discount))
  end

  def enabled(query \\ __MODULE__) do
    from([_q, d] in query, where: d.is_enabled)
  end

  def usage_count_less_than_usage_limit(query \\ __MODULE__) do
    from([q, d] in query,
      where: d.usage_limit == 0,
      or_where: q.usage_count < d.usage_limit
    )
  end

  def maybe_code(query \\ __MODULE__, code) do
    if code do
      from([_q, d] in query, where: d.code == ^String.upcase(code))
    else
      query
    end
  end

  def maybe_select_code(query \\ __MODULE__, code) do
    if code do
      from([_q, d] in query, select: d.code)
    else
      from([_q, d] in query, preload: [discount: d])
    end
  end

The usage_count_less_than_usage_limit bit is the newest edition to the pipeline, and it’s causing issues because the final query being executed is:

SELECT d0."id", d0."usage_count", d0."inserted_at", d0."updated_at", d0."discount_id", d0."package_id", d1."id", d1."code", d1."is_enabled", d1."usage_limit", d1."inserted_at", d1."updated_at" FROM "discount_packages" AS d0 INNER JOIN "discounts" AS d1 ON d1."id" = d0."discount_id" WHERE ((((d0."package_id" IS NULL)) OR (d0."package_id" = $1)) AND (d1."is_enabled") AND (d1."usage_limit" = 0)) OR (d0."usage_count" < d1."usage_limit") [9999999999999]

Ecto is slapping this to the end of the query AND (d1."usage_limit" = 0)) OR (d0."usage_count" < d1."usage_limit"). It’s attaching the usage_limit = 0 condition in the AND near the end, but then it finishes it off with OR (d0."usage_count" < d1."usage_limit").

That’s not the intended result because that returns back discount codes even with invalid package_ids (as seen with 9999999999999 example input) since the last condition is true and the final OR isn’t in the correct parenthesis. I’d really like the usage_count function’s where and or_where to be isolated to only its composed bits, not the entire query.

How do you do that? I can’t find an example of this anywhere.

Composing where conditions in Ecto.Query structs works like this for each new additional (or_)where: added: (prev_condition) AND|OR (added_condition). So it’s not very flexible to compose complex where conditions, which is especially obvious for OR relationships. If you need the complexity consider using dynamic to build up the condition and only if that’s done attach it to the query like where: ^dynamic.

1 Like

Where does dynamic go? I looked at the docs and it’s not very clear on how to convert a bunch of composed query functions to using dynamic.

Would I end up replacing all of those composed functions with 1 massive function that has a bunch of conditions that keeps building onto itself? Even after looking at the docs I’m not sure what that would look like in this case.

Also in this case, I’m not building this up dynamically. It’s just 1 query with all of those rules every time. I broke it up because it felt more readable / reusable having each thing independently broken out.

Yeah, you’d basically need to split up the “compose ecto query” part and the “compose conditional”. The thing dynamic gives you over composing directly in the query is that you can compose dynamics like so: dynamic([a], ^prev_dynamic or a.id = 3 and you can manage precedence of conditions on your own, as there won’t be any automatically added parenthesis.

2 Likes

What does an example of this look like?

I’m still not sure if I have to remove all of those small functions and replace everything with 1 gigantic SQL query where I manually place the parenthesis.

I don’t think I’m smart enough to follow the docs. The docs look like it’s just passing a boolean to where in the end. I’m not sure how to convert a boolean to a queryable.

from(
  q in query,
  where: is_nil(q.package_id),
  or_where: q.package_id == ^package_id
)

I fully stand behind @LostKobrakai’s suggestions. But if your logic is simple enough and allows it then rewriting the above might work for you:

from(
  q in query,
  where: is_nil(q.package_id) or q.package_id == ^package_id
)
3 Likes

Found a dynamic example, right here on the forum Dynamic Queries in Ecto

Hope this helps.

1 Like

Thanks, I read that one. That example is similar to the docs in that it doesn’t show case how to convert small composable query functions that may or may use AND and OR into using dynamic.

Thanks! This works. If I get rid of all of the or_where examples and use your recommended style then the paranthesis end up in the correct spot, even if I add more composed functions into the mix that use or.

I think I’ll stick with this solution because dynamic is like 1,000 miles above my pay grade.

1 Like

Just one addition: You can recursively build up the dynamic condition like you can build up an ecto query, just that you’ll only handle the condition and which one has precedence over the other. This can happen completely unrelated to joining tables and all the other stuff, which might be needed for a complete query.

1 Like

FWIW, in a case where you build the whole OR expression in one place you can avoid the behavior of or_where by writing the or explicitly:

  def usage_count_less_than_usage_limit(query \\ __MODULE__) do
    from([q, d] in query,
      where: d.usage_limit == 0 or q.usage_count < d.usage_limit
    )
  end
1 Like

I’m sure dynamic is great, but after reading what you wrote, looking at the docs and reading a few forum posts, I couldn’t find a single example of how to convert 1 method to the other and reading the explanations alone wasn’t enough to be able to apply it so I won’t be able to end up using it.

Can you provide a minimal example of a case where your current code does result in the OR being in a incorrect place?

Check out the raw SQL in the original post.

I’m not sure if it’s incorrect from Ecto’s POV, but it’s incorrect in that the query produces unintended results while using or without using or_where produces the correct results based on what @dimitarvp and @al2o3cr wrote.

Ecto seems to want to combine the ANDs and ORs based on the final query’s output instead of at the individual composed function’s case.

For example, AND (d1."usage_limit" = 0)) was attached near the end which is part of the usage_count_less_than_usage_limit composed function but then it wrapped that in paranthesis with the other non-OR queries.

Then, at the very end it put OR (d0."usage_count" < d1."usage_limit") as a separate OR query which will always evaluate to true (when the usage_count < usage_limit) which invalidates the other conditions due to the placement of the parenthesis.

However the intended result would be to have ... AND ((d1."usage_limit" = 0) OR (d0."usage_count" < d1."usage_limit")) which puts the usage_count logic together (0 for usage_count is ok but if it’s not zero then it must be less than the usage limit in order to be ok), and all of the other composed WHERE logic stands on its own and is separated by AND.

1 Like

@LostKobrakai it’s out of date, but here’s an executable example I made a few months back when my team hit similar issues with or_where and composition:

TL;DR the way that or_where combines with other where clauses can cause different composition orders to produce different queries.

1 Like
for_package(where):   
  WHERE (p.package_id IS NULL)

for_package(or_where): 
  WHERE ((p.package_id IS NULL)) 
        OR (q.package_id == 1)

enabled:
  WHERE (((p.package_id IS NULL)) OR (q.package_id == 1)) 
        AND (d.is_enabled)

usage_count_less_than_usage_limit (where):       
  WHERE (((p.package_id IS NULL)) OR (q.package_id == 1)) AND (d.is_enabled)) 
        AND (d.usage_limit = 0)

usage_count_less_than_usage_limit (or_where):       
  WHERE ((((p.package_id IS NULL) OR (q.package_id == 1)) AND (d.is_enabled)) AND (d.usage_limit = 0)) 
        OR (p."usage_count" < d."usage_limit")

This is how your query’s where condition evolves over the course of you building it up. All I did for those lines is wrap the prev. where condition in parenthesis – if existing - append AND|OR and append the supplied additional condition also wrapped in parenthesis. Voila the WHERE condition of your resulting query.

query = __MODULE__
query = from x in query, where: x
query = from x in query, where: y, or_where: z

This might look like WHERE X AND (Y OR Z) in code, but it’s WHERE (X AND Y) OR Z because or_where: … technically means “all the prev. conditions or the current one” and not “the last added condition or the current one”. Same for where, but usually AND composes more predictable than OR, so it’s less of a “problem”.

The only way to get to WHERE X AND (Y OR Z) is the following:

query = __MODULE__
query = from x in query, where: x
query = from x in query, where: y or z

In such a simple example it doesn’t seem like much of a change, but it actually is. It means ecto does it’s “wrap everything existing in parenthesis” only once and does keep y or z is one set of parenthesis. If your query conditions are hardcoded (as it seems like they are above) you should be able to simply refactor. Replace …, or_where: … with just … or ….

It get’s more complex if the conditions themselves are also assembled by user input.
Take e.g. a legacy db, where you can search for sport fields, but the different sports are in different columns in the legacy db: sport_1, sport_2, ….

cond = %{
  sports: ["volleyball", "football"]
  distance: 45,
  only: ["sport_1", "sport_2", "sport_3"]
}

# Query should become
# FROM x 
# WHERE x,distance < 45 
#   AND (x.sport_1 IN ["volleyball", "football"] OR x.sport_2 IN ["volleyball", "football"] OR x.sport_3 IN ["volleyball", "football"])

%{distance: distance, only: fields, sports: sports} = cond
query = from x in Fields, where: x.distance < ^distance
dynamic = 
  Enum.reduce(fields, nil, fn 
    field, nil -> dynamic([x], field(x, ^field) in ^sports)
    field, dyn -> dynamci([x], ^dyn or field(x, ^field) in ^sports
  end)
query = from x in query, where: ^dynamic

This is where dynamic comes into play, because it let’s you compose conditions together without the parenthesis adding of where and or_where. Basically it’s an escape hatch to “now I need to handle precedence differently than your safe way let’s me do it”.

3 Likes

Thanks for taking the time to put together this example. It just helped me