fewer parens

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+- remove unnecessary parens from generated SQL to avoid hitting TOO_DEEP_RECURSION https://github.com/plausible/ecto_ch/pull/206
+
 ## 0.4.0 (2024-10-15)
 
 - use `UNION DISTINCT` instead of `UNION` for `Ecto.Query.union/2` https://github.com/plausible/ecto_ch/pull/204

lib/ecto/adapters/clickhouse.ex

@@ -365,13 +365,13 @@ defmodule Ecto.Adapters.ClickHouse do
 
       iex> query = from n in fragment("numbers(10)"), where: n.number == ^-1, select: n.number
       iex> Ecto.Adapters.ClickHouse.to_inline_sql(:all, query)
-      ~s{SELECT f0."number" FROM numbers(10) AS f0 WHERE (f0."number" = -1)}
+      ~s{SELECT f0."number" FROM numbers(10) AS f0 WHERE f0."number" = -1}
 
   Compare it with the output of `to_sql/2`
 
       iex> query = from n in fragment("numbers(10)"), where: n.number == ^-1, select: n.number
       iex> Ecto.Adapters.ClickHouse.to_sql(:all, query)
-      {~s[SELECT f0."number" FROM numbers(10) AS f0 WHERE (f0."number" = {$0:Int64})], [-1]}
+      {~s[SELECT f0."number" FROM numbers(10) AS f0 WHERE f0."number" = {$0:Int64}], [-1]}
 
   """
   @spec to_inline_sql(:all | :delete_all | :update_all, Ecto.Queryable.t()) :: String.t()

lib/ecto/adapters/clickhouse/connection.ex

@@ -301,14 +301,9 @@ defmodule Ecto.Adapters.ClickHouse.Connection do
     and: " AND ",
     or: " OR ",
     ilike: " ILIKE ",
-    like: " LIKE ",
-    # TODO these two are not in binary_ops in sqlite3 adapter
-    in: " IN ",
-    is_nil: " WHERE "
+    like: " LIKE "
   ]
 
-  @binary_ops Keyword.keys(binary_ops)
-
   for {op, str} <- binary_ops do
     defp handle_call(unquote(op), 2), do: {:binary_op, unquote(str)}
   end
@@ -594,22 +589,21 @@ defmodule Ecto.Adapters.ClickHouse.Connection do
 
   defp boolean(_name, [], _sources, _params, _query), do: []
 
-  defp boolean(name, [%{expr: expr, op: op} | exprs], sources, params, query) do
+  defp boolean(name, exprs, sources, params, query) do
+    [%BooleanExpr{expr: first_expr} | other_exprs] = exprs
+
     result =
-      Enum.reduce(exprs, {op, paren_expr(expr, sources, params, query)}, fn
-        %BooleanExpr{expr: expr, op: op}, {op, acc} ->
-          {op, [acc, operator_to_boolean(op) | paren_expr(expr, sources, params, query)]}
+      Enum.reduce(other_exprs, expr(first_expr, sources, params, query), fn
+        %BooleanExpr{op: :or, expr: expr}, acc ->
+          ["((", acc, ?), " OR ", ?(, expr(expr, sources, params, query), "))"]
 
-        %BooleanExpr{expr: expr, op: op}, {_, acc} ->
-          {op, [?(, acc, ?), operator_to_boolean(op) | paren_expr(expr, sources, params, query)]}
+        %BooleanExpr{op: :and, expr: expr}, acc ->
+          [acc, " AND ", expr(expr, sources, params, query)]
       end)
 
-    [name | elem(result, 1)]
+    [name | result]
   end
 
-  defp operator_to_boolean(:and), do: " AND "
-  defp operator_to_boolean(:or), do: " OR "
-
   defp parens_for_select([first_expr | _] = expression) do
     if is_binary(first_expr) and String.match?(first_expr, ~r/^\s*select/i) do
       [?(, expression, ?)]
@@ -618,10 +612,6 @@ defmodule Ecto.Adapters.ClickHouse.Connection do
     end
   end
 
-  defp paren_expr(expr, sources, params, query) do
-    [?(, expr(expr, sources, params, query), ?)]
-  end
-
   defp expr({_type, [literal]}, sources, params, query) do
     expr(literal, sources, params, query)
   end
@@ -683,7 +673,7 @@ defmodule Ecto.Adapters.ClickHouse.Connection do
   end
 
   defp expr({:is_nil, _, [arg]}, sources, params, query) do
-    [expr(arg, sources, params, query) | " IS NULL"]
+    [?(, expr(arg, sources, params, query) | " IS NULL)"]
   end
 
   defp expr({:not, _, [expr]}, sources, params, query) do
@@ -782,16 +772,31 @@ defmodule Ecto.Adapters.ClickHouse.Connection do
         _ -> {[], args}
       end
 
-    case handle_call(fun, length(args)) do
-      {:binary_op, op} ->
+    # TODO don't length(args)
+    case {fun, handle_call(fun, length(args))} do
+      {:or, {:binary_op, op}} ->
         [left, right] = args
 
         [
-          op_to_binary(left, sources, params, query),
-          op | op_to_binary(right, sources, params, query)
+          "((",
+          expr(left, sources, params, query),
+          ?),
+          op,
+          ?(,
+          expr(right, sources, params, query),
+          "))"
         ]
 
-      {:fun, fun} ->
+      {_, {:binary_op, op}} ->
+        [left, right] = args
+
+        [
+          expr(left, sources, params, query),
+          op,
+          expr(right, sources, params, query)
+        ]
+
+      {_, {:fun, fun}} ->
         [fun, ?(, modifier, intersperse_map(args, ?,, &expr(&1, sources, params, query)), ?)]
     end
   end
@@ -835,18 +840,6 @@ defmodule Ecto.Adapters.ClickHouse.Connection do
       message: "unsupported expression #{inspect(expr)}"
   end
 
-  defp op_to_binary({op, _, [_, _]} = expr, sources, params, query) when op in @binary_ops do
-    paren_expr(expr, sources, params, query)
-  end
-
-  defp op_to_binary({:is_nil, _, [_]} = expr, sources, params, query) do
-    paren_expr(expr, sources, params, query)
-  end
-
-  defp op_to_binary(expr, sources, params, query) do
-    expr(expr, sources, params, query)
-  end
-
   defp create_names(%{sources: sources}, as_prefix) do
     sources |> create_names(0, tuple_size(sources), as_prefix) |> List.to_tuple()
   end