Remove `and true` in dynamic query reduce

[joehua87]

When using reduce in dynamic query, like discussed in the Ecto docs here, it produce output like this:

where: true and (true and e0.avg_rating > ^3)

The problems it that it make expression longer and harder to maintain test
Does anyone have ideas to reduce this to:

where: e0.avg_rating > ^3

[sb8244]

I’ve thought about this for a recent project but didn’t implement it, because I decided it wasn’t worth the code for SQL vanity purpose.

In your reducer function, you could case on the accumulator condition and, if nil, produce a query without the and. The benefit of using “true and” is that your reducer doesn’t need to care what the accumulator is.

I haven’t tried it but I think this would work.

[joehua87]

Thank you @sb8244! It’s simpler than I thought, just made it

Before:

defp parse_item({compare_type, value}, acc, field_name) do
  case compare_type do
    :eq -> dynamic([p], ^acc and field(p, ^field_name) == ^value)
    :gt -> dynamic([p], ^acc and field(p, ^field_name) > ^value)
    :lt -> dynamic([p], ^acc and field(p, ^field_name) < ^value)
    :gte -> dynamic([p], ^acc and field(p, ^field_name) >= ^value)
    :lte -> dynamic([p], ^acc and field(p, ^field_name) <= ^value)
    _ -> acc
  end
end

After:

defp parse_item({compare_type, value}, acc, field_name) do
  case compare_type do
    :eq -> join_exp(acc, dynamic([p], field(p, ^field_name) == ^value))
    :gt -> join_exp(acc, dynamic([p], field(p, ^field_name) > ^value))
    :lt -> join_exp(acc, dynamic([p], field(p, ^field_name) < ^value))
    :gte -> join_exp(acc, dynamic([p], field(p, ^field_name) >= ^value))
    :lte -> join_exp(acc, dynamic([p], field(p, ^field_name) <= ^value))
    _ -> acc
  end
end

defp join_exp(acc, exp) do
  case inspect(acc) do
    "dynamic([], true)" ->
      exp

    _ ->
      dynamic(^acc and ^exp)
  end
end

[dimitarvp]

Relying on inspect's output is awful but sometimes a fact of life.

[chrismcg]

@joehua87

case inspect(acc) do

I wouldn’t use inspect here as it’s representation could change (see [1] for an example of this).

I think you could make the initial value of reduce nil and then pattern match on that rather than dynamic(true) representation.

e.g.

defp join_exp(nil, exp) -> exp
defp join_exp(acc, exp) -> dynamic(^acc and ^exp)

[1] https://github.com/elixir-lang/elixir/pull/9826#discussion_r380359608

[joehua87]

Thanks