refactor (#69)

lib/ecto/adapters/clickhouse/connection.ex

@@ -183,7 +183,7 @@ defmodule Ecto.Adapters.ClickHouse.Connection do
           [quote_name(field), " IS NULL"]
 
         {{field, value}, idx} ->
-          [quote_name(field), " = {$", Integer.to_string(idx), ?:, ch_typeof(value), ?}]
+          [quote_name(field), ?=, build_param(idx, param_type(value))]
       end)
 
     ["DELETE FROM ", quote_table(prefix, table), " WHERE ", filters]
@@ -515,18 +515,11 @@ defmodule Ecto.Adapters.ClickHouse.Connection do
   end
 
   defp expr({:^, [], [ix]}, _sources, params, _query) do
-    ["{$", Integer.to_string(ix), ?:, param_type_at(params, ix), ?}]
+    build_param(ix, param_type_at(params, ix))
   end
 
   defp expr({:^, [], [ix, len]}, _sources, params, _query) when len > 0 do
-    params =
-      ix..(ix + len)
-      |> Enum.take(len)
-      |> intersperse_map(?,, fn ix ->
-        ["{$", Integer.to_string(ix), ?:, param_type_at(params, ix), ?}]
-      end)
-
-    [?(, params, ?)]
+    [?(, build_params(ix, len, params), ?)]
   end
 
   # using an empty array literal since empty tuples are not allowed in ClickHouse
@@ -573,10 +566,6 @@ defmodule Ecto.Adapters.ClickHouse.Connection do
 
   defp expr({:in, _, [_, {:^, _, [_ix, 0]}]}, _sources, _params, _query), do: "0"
 
-  defp expr({:in, _, [left, {{:., _, _}, _, _} = right]}, sources, params, query) do
-    ["has(", expr(right, sources, params, query), ?,, expr(left, sources, params, query), ?)]
-  end
-
   defp expr({:in, _, [left, right]}, sources, params, query) do
     [expr(left, sources, params, query), " IN ", expr(right, sources, params, query)]
   end
@@ -787,6 +776,19 @@ defmodule Ecto.Adapters.ClickHouse.Connection do
 
   def intersperse_map([], _separator, _mapper), do: []
 
+  @compile inline: [build_param: 2]
+  defp build_param(ix, type) do
+    ["{$", Integer.to_string(ix), ?:, type, ?}]
+  end
+
+  @doc false
+  def build_params(ix, len, params) when len > 1 do
+    [build_param(ix, param_type_at(params, ix)), ?, | build_params(ix + 1, len - 1, params)]
+  end
+
+  def build_params(ix, _len = 1, params), do: build_param(ix, param_type_at(params, ix))
+  def build_params(_ix, _len = 0, _params), do: []
+
   @doc false
   def quote_name(name, quoter \\ ?")
   def quote_name(nil, _), do: []
@@ -864,90 +866,43 @@ defmodule Ecto.Adapters.ClickHouse.Connection do
     [expr(count, sources, params, query), " * ", interval(1, interval, sources, params, query)]
   end
 
-  defp ecto_to_db({:array, t}) do
-    ["Array(", ecto_to_db(t), ?)]
-  end
-
-  defp ecto_to_db(:id), do: "UInt64"
-  defp ecto_to_db(:uuid), do: "UUID"
-  defp ecto_to_db(s) when s in [:string, :binary, :binary_id], do: "String"
   # when ecto migrator queries for versions in schema_versions it uses type(version, :integer)
   # so we need :integer to be the same as :bigint which is used for schema_versions table definition
   # this is why :integer is Int64 and not Int32
-  defp ecto_to_db(i) when i in [:integer, :bigint], do: "Int64"
-  defp ecto_to_db(:float), do: "Float64"
+  defp ecto_to_db(:integer), do: "Int64"
+  defp ecto_to_db(:binary), do: "String"
+  defp ecto_to_db({:parameterized, Ch, type}), do: Ch.Types.encode(type)
+  defp ecto_to_db({:array, type}), do: ["Array(", ecto_to_db(type), ?)]
+  defp ecto_to_db(type) when type in [:uuid, :string, :date, :boolean], do: Ch.Types.encode(type)
 
-  defp ecto_to_db(:decimal) do
-    raise ArgumentError,
-          "cast to :decimal is not supported, please use `Ch, type: \"Decimal(P, S)\"` instead"
-  end
-
-  defp ecto_to_db({:parameterized, Ch, type}) do
-    ecto_to_db(type)
-  end
-
-  defp ecto_to_db(:boolean), do: "Bool"
-  defp ecto_to_db(:date), do: "Date"
-  defp ecto_to_db(:date32), do: "Date32"
-  defp ecto_to_db(dt) when dt in [:datetime, :utc_datetime, :naive_datetime], do: "DateTime"
-  defp ecto_to_db(:u8), do: "UInt8"
-  defp ecto_to_db(:u16), do: "UInt16"
-  defp ecto_to_db(:u32), do: "UInt32"
-  defp ecto_to_db(:u64), do: "UInt64"
-  defp ecto_to_db(:u128), do: "UInt128"
-  defp ecto_to_db(:u256), do: "UInt256"
-  defp ecto_to_db(:i8), do: "Int8"
-  defp ecto_to_db(:i16), do: "Int16"
-  defp ecto_to_db(:i32), do: "Int32"
-  defp ecto_to_db(:i64), do: "Int64"
-  defp ecto_to_db(:i128), do: "Int128"
-  defp ecto_to_db(:i256), do: "Int256"
-  defp ecto_to_db(:f32), do: "Float32"
-  defp ecto_to_db(:f64), do: "Float64"
-
-  for size <- [32, 64, 128, 256] do
-    defp ecto_to_db({unquote(:"decimal#{size}"), scale}) do
-      [unquote("Decimal#{size}("), Integer.to_string(scale), ?)]
-    end
-  end
-
-  defp ecto_to_db({:decimal, precision, scale}) do
-    ["Decimal(", Integer.to_string(precision), ", ", Integer.to_string(scale), ?)]
-  end
-
-  defp ecto_to_db({:string, size}) do
-    ["FixedString(", Integer.to_string(size), ?)]
-  end
-
-  defp ecto_to_db({:nullable, type}) do
-    ["Nullable(", ecto_to_db(type), ?)]
-  end
-
-  defp ecto_to_db(other) when is_atom(other) do
-    Atom.to_string(other)
+  defp ecto_to_db(type) do
+    raise ArgumentError, "unknown or ambiguous ClickHouse type: #{inspect(type)}"
   end
 
   defp param_type_at(params, ix) do
     value = Enum.at(params, ix)
-    ch_typeof(value)
+    param_type(value)
   end
 
-  defp ch_typeof(s) when is_binary(s), do: "String"
-  defp ch_typeof(i) when is_integer(i) and i > 0x7FFFFFFFFFFFFFFF, do: "UInt64"
-  defp ch_typeof(i) when is_integer(i), do: "Int64"
-  defp ch_typeof(f) when is_float(f), do: "Float64"
-  defp ch_typeof(b) when is_boolean(b), do: "Bool"
-  defp ch_typeof(%DateTime{}), do: "DateTime"
-  defp ch_typeof(%Date{}), do: "Date"
-  defp ch_typeof(%NaiveDateTime{}), do: "DateTime"
+  defp param_type(s) when is_binary(s), do: "String"
+  defp param_type(i) when is_integer(i) and i > 0x7FFFFFFFFFFFFFFF, do: "UInt64"
+  defp param_type(i) when is_integer(i), do: "Int64"
+  defp param_type(f) when is_float(f), do: "Float64"
+  defp param_type(b) when is_boolean(b), do: "Bool"
+
+  # TODO DateTime64 and Date32
+  defp param_type(%NaiveDateTime{}), do: "DateTime"
+  defp param_type(%DateTime{}), do: "DateTime"
+  defp param_type(%Date{}), do: "Date"
 
-  defp ch_typeof(%Decimal{exp: exp}) do
+  defp param_type(%Decimal{exp: exp}) do
     # TODO use sizes 128 and 256 as well if needed
     scale = if exp < 0, do: abs(exp), else: 0
     ["Decimal64(", Integer.to_string(scale), ?)]
   end
 
-  defp ch_typeof([]), do: "Array(Nothing)"
+  defp param_type([]), do: "Array(Nothing)"
+
   # TODO check whole list
-  defp ch_typeof([v | _]), do: ["Array(", ch_typeof(v), ?)]
+  defp param_type([v | _]), do: ["Array(", param_type(v), ?)]
 end

lib/ecto/adapters/clickhouse/schema.ex

@@ -126,14 +126,18 @@ defmodule Ecto.Adapters.ClickHouse.Schema do
           field :#{field}, Ch, type: "#{ch_type}"
         end
 
+    You can also try using `ecto.ch.schema` to generate a schema:
+
+        mix ecto.ch.schema <database>.#{schema.__schema__(:source)}
+
     """
   end
 
   # https://hexdocs.pm/ecto/Ecto.Schema.html#module-primitive-types
   defp ch_type_hint(:id), do: "Int64"
   defp ch_type_hint(:integer), do: "Int64"
   defp ch_type_hint(:float), do: "Float32"
-  defp ch_type_hint({:array, type}), do: "Array(#{ch_type_hint(type)})"
+  defp ch_type_hint({:array, type}), do: "{:array, #{ch_type_hint(type)}}"
   defp ch_type_hint(:map), do: "Map(String, Int64)"
   defp ch_type_hint({:map, type}), do: "Map(String, #{ch_type_hint(type)})"
   defp ch_type_hint(:decimal), do: "Decimal32(2)"

test/ch/type_test.exs

@@ -876,6 +876,14 @@ defmodule Ch.TypeTest do
                %{t1: {""}, t2: {"", 0}, t3: {"", [], 0}},
                %{t1: {"hello"}, t2: {"hello", 42}, t3: {"hello", ["world"], 42}}
              ]
+
+      # TODO
+      # assert Tuples |> where([t], "hell" in t.t1) |> all() |> unstruct() == []
+      # assert Tuples |> where([t], ^"hell" in t.t1) |> all() |> unstruct() == []
+      # assert Tuples |> where([t], 42 in t.t2) |> all() |> unstruct() == []
+      # assert Tuples |> where([t], ^42 in t.t2) |> all() |> unstruct() == []
+      # assert Tuples |> where([t], ["world"] in t.t3) |> all() |> unstruct() == []
+      # assert Tuples |> where([t], ^["world"] in t.t3) |> all() |> unstruct() == []
     end
   end
 

test/ecto/adapters/clickhouse/connection_test.exs

@@ -644,22 +644,13 @@ defmodule Ecto.Adapters.ClickHouse.ConnectionTest do
 
   @decimal64_2 Ecto.ParameterizedType.init(Ch, type: "Decimal64(2)")
   test "order_by and types" do
-    query =
-      "schema3"
-      |> order_by([e], type(fragment("?", e.binary), :decimal))
-      |> select(true)
-
-    assert_raise ArgumentError, ~r/cast to :decimal is not supported/, fn ->
-      all(query)
-    end
-
     query =
       "schema3"
       |> order_by([e], type(fragment("?", e.binary), ^@decimal64_2))
       |> select(true)
 
     assert all(query) ==
-             ~s[SELECT true FROM "schema3" AS s0 ORDER BY CAST(s0."binary" AS Decimal64(2))]
+             ~s[SELECT true FROM "schema3" AS s0 ORDER BY CAST(s0."binary" AS Decimal(18, 2))]
   end
 
   test "fragments" do
@@ -1720,13 +1711,13 @@ defmodule Ecto.Adapters.ClickHouse.ConnectionTest do
 
   test "delete" do
     query = delete(nil, "schema", [x: 1, y: 2], [])
-    assert query == ~s[DELETE FROM "schema" WHERE "x" = {$0:Int64} AND "y" = {$1:Int64}]
+    assert query == ~s[DELETE FROM "schema" WHERE "x"={$0:Int64} AND "y"={$1:Int64}]
 
     query = delete("prefix", "schema", [x: 1, y: 2], [])
-    assert query == ~s[DELETE FROM "prefix"."schema" WHERE "x" = {$0:Int64} AND "y" = {$1:Int64}]
+    assert query == ~s[DELETE FROM "prefix"."schema" WHERE "x"={$0:Int64} AND "y"={$1:Int64}]
 
     query = delete(nil, "schema", [x: nil, y: 1], [])
-    assert query == ~s[DELETE FROM "schema" WHERE "x" IS NULL AND "y" = {$1:Int64}]
+    assert query == ~s[DELETE FROM "schema" WHERE "x" IS NULL AND "y"={$1:Int64}]
   end
 
   test "executing a string during migration" do
@@ -2440,4 +2431,39 @@ defmodule Ecto.Adapters.ClickHouse.ConnectionTest do
              ~s/CREATE TABLE "posts"("id" Int32,PRIMARY KEY ("id")) ENGINE=MergeTree/
            ]
   end
+
+  test "build_params/3" do
+    params = [1, "a", true, Date.utc_today(), DateTime.utc_now()]
+
+    assert to_string(Connection.build_params(_ix = 0, _len = 0, params)) == ""
+    assert to_string(Connection.build_params(_ix = 1, _len = 0, params)) == ""
+    assert to_string(Connection.build_params(_ix = 2, _len = 0, params)) == ""
+
+    assert to_string(Connection.build_params(_ix = 0, _len = 1, params)) ==
+             "{$0:Int64}"
+
+    assert to_string(Connection.build_params(_ix = 0, _len = 2, params)) ==
+             "{$0:Int64},{$1:String}"
+
+    assert to_string(Connection.build_params(_ix = 1, _len = 1, params)) ==
+             "{$1:String}"
+
+    assert to_string(Connection.build_params(_ix = 1, _len = 2, params)) ==
+             "{$1:String},{$2:Bool}"
+
+    assert to_string(Connection.build_params(_ix = 2, _len = 1, params)) ==
+             "{$2:Bool}"
+
+    assert to_string(Connection.build_params(_ix = 2, _len = 2, params)) ==
+             "{$2:Bool},{$3:Date}"
+
+    assert to_string(Connection.build_params(_ix = 2, _len = 3, params)) ==
+             "{$2:Bool},{$3:Date},{$4:DateTime}"
+
+    assert to_string(Connection.build_params(_ix = 1, _len = 4, params)) ==
+             "{$1:String},{$2:Bool},{$3:Date},{$4:DateTime}"
+
+    assert to_string(Connection.build_params(_ix = 0, _len = 5, params)) ==
+             "{$0:Int64},{$1:String},{$2:Bool},{$3:Date},{$4:DateTime}"
+  end
 end

test/ecto/integration/type_test.exs

@@ -197,13 +197,14 @@ defmodule Ecto.Integration.TypeTest do
     assert [^blob] = TestRepo.all(query)
   end
 
+  @float64 Ecto.ParameterizedType.init(Ch, type: "Float64")
   test "tagged types" do
     %{id: post_id} = TestRepo.insert!(%Post{id: 1, visits: 12})
     TestRepo.insert!(%Comment{text: "#{post_id}", post_id: post_id})
 
     # Numbers
     assert [1] = TestRepo.all(from Post, select: type(^"1", :integer))
-    assert [1.0] = TestRepo.all(from Post, select: type(^1.0, :float))
+    assert [1.0] = TestRepo.all(from Post, select: type(^1.0, ^@float64))
     assert [1] = TestRepo.all(from p in Post, select: type(^"1", p.visits))
     assert [1.0] = TestRepo.all(from p in Post, select: type(^"1", p.intensity))
 
@@ -220,18 +221,20 @@ defmodule Ecto.Integration.TypeTest do
 
     # Math operations
     assert [4] = TestRepo.all(from Post, select: type(2 + ^"2", :integer))
-    assert [4.0] = TestRepo.all(from Post, select: type(2.0 + ^"2", :float))
+    # assert [4.0] = TestRepo.all(from Post, select: type(2.0 + ^"2", ^@float64))
     assert [4] = TestRepo.all(from p in Post, select: type(2 + ^"2", p.visits))
     assert [4.0] = TestRepo.all(from p in Post, select: type(2.0 + ^"2", p.intensity))
 
     # Comparison expression
     assert [12] = TestRepo.all(from p in Post, select: type(coalesce(p.visits, 0), :integer))
-    assert [0.0] = TestRepo.all(from p in Post, select: type(coalesce(p.intensity, 1.0), :float))
+
+    assert [0.0] =
+             TestRepo.all(from p in Post, select: type(coalesce(p.intensity, 1.0), ^@float64))
 
     assert [1.0] =
              TestRepo.all(
                from p in Post,
-                 select: type(coalesce(fragment("nullIf(?, 0)", p.intensity), 1.0), :float)
+                 select: type(coalesce(fragment("nullIf(?, 0)", p.intensity), 1.0), ^@float64)
              )
 
     # parent_as/1
@@ -284,11 +287,41 @@ defmodule Ecto.Integration.TypeTest do
 
     # Querying
     assert TestRepo.all(from t in Tag, where: t.ints == [1, 2, 3], select: t.ints) == [ints]
-    assert TestRepo.all(from t in Tag, where: 0 in t.ints, select: t.ints) == []
-    assert TestRepo.all(from t in Tag, where: 1 in t.ints, select: t.ints) == [ints]
     assert TestRepo.all(from t in "tags", where: t.ints == [1, 2, 3], select: t.ints) == [ints]
-    assert TestRepo.all(from t in "tags", where: 0 in t.ints, select: t.ints) == []
-    assert TestRepo.all(from t in "tags", where: 1 in t.ints, select: t.ints) == [ints]
+
+    # ClickHouse doesn't support IN operator on array columns
+    # works: select 1 in [1,2,3]
+    # fails: select * from tags t where 0 in t.ints
+
+    assert_raise Ch.Error, ~r/UNKNOWN_TABLE/, fn ->
+      TestRepo.all(from t in Tag, where: 0 in t.ints, select: t.ints)
+    end
+
+    assert_raise Ch.Error, ~r/UNKNOWN_TABLE/, fn ->
+      TestRepo.all(from t in Tag, where: 1 in t.ints, select: t.ints)
+    end
+
+    assert_raise Ch.Error, ~r/UNKNOWN_TABLE/, fn ->
+      TestRepo.all(from t in Tag, where: ^0 in t.ints, select: t.ints)
+    end
+
+    assert_raise Ch.Error, ~r/UNKNOWN_TABLE/, fn ->
+      TestRepo.all(from t in Tag, where: ^1 in t.ints, select: t.ints)
+    end
+
+    # has(arr, el) can be used instead
+
+    assert TestRepo.all(from t in Tag, where: fragment("has(?, ?)", t.ints, 0), select: t.ints) ==
+             []
+
+    assert TestRepo.all(from t in Tag, where: fragment("has(?, ?)", t.ints, 1), select: t.ints) ==
+             [ints]
+
+    assert TestRepo.all(from t in Tag, where: fragment("has(?, ?)", t.ints, ^0), select: t.ints) ==
+             []
+
+    assert TestRepo.all(from t in Tag, where: fragment("has(?, ?)", t.ints, ^1), select: t.ints) ==
+             [ints]
 
     # # Update
     # tag = TestRepo.update!(Ecto.Changeset.change tag, ints: nil)
@@ -378,13 +411,14 @@ defmodule Ecto.Integration.TypeTest do
     assert Decimal.equal?(Decimal.new("0.0"), cost)
   end
 
+  @float32 Ecto.ParameterizedType.init(Ch, type: "Float32")
   @decimal64_2 Ecto.ParameterizedType.init(Ch, type: "Decimal64(2)")
   test "decimal typed aggregations" do
     decimal = Decimal.new("1.0")
     TestRepo.insert!(%Post{cost: decimal})
 
     assert [1] = TestRepo.all(from p in Post, select: type(sum(p.cost), :integer))
-    assert [1.0] = TestRepo.all(from p in Post, select: type(sum(p.cost), :float))
+    assert [1.0] = TestRepo.all(from p in Post, select: type(sum(p.cost), ^@float32))
     [cost] = TestRepo.all(from p in Post, select: type(sum(p.cost), ^@decimal64_2))
     assert Decimal.equal?(decimal, cost)
   end