plausible / ecto_ch

Ecto ClickHouse adapter
MIT License
72 stars 10 forks source link

Issue with Map with bool values #128

Closed hkrutzer closed 1 year ago

hkrutzer commented 1 year ago

I am having an issue where I have a field like this:

    field(:bools, Ch,
      type: "SimpleAggregateFunction(groupArrayArray, Array(Map(String, UInt8)))"
    )

and I insert data like this: [%{"foo"=> true}, %{"bar" => false}]. This causes an error:

{"is invalid",
              [
                type: {:parameterized, Ch,
                 {:simple_aggregate_function, "groupArrayArray",
                  {:array, {:map, :string, :u8}}}},
                validation: :cast
              ]}

true is of course not a UInt8 so I suspect that is what is causing the error.

But when I make it a Bool, I get

** (Ch.Error) Code: 117. DB::Exception: Type of 'bools' must be SimpleAggregateFunction(groupArrayArray, Array(Map(String, UInt8))), not SimpleAggregateFunction(groupArrayArray, Array(Map(String, Bool))): While executing BinaryRowInputFormat. (INCORRECT_DATA) (version 23.7.4.5 (official build))

Perhaps https://github.com/plausible/ecto_ch/pull/126 happens to address this? But I thought I'd make an issue anyway.

ruslandoga commented 1 year ago

No, #126 wouldn't change the internals of Ch Ecto type, it's mostly about queries that use type/2 macro.

Can you please share more of your code? Ideally as a failing test case.

ruslandoga commented 1 year ago

I think I understand.

You have defined a column with type SimpleAggregateFunction(groupArrayArray, Array(Map(String, UInt8))) but from Elixir you want to inserts booleans there. You can do that with a custom Ecto type, ecto_ch doesn't handle type mismatches automatically (mostly because basic Ecto types like booleans don't do it automatically either).

There is a similar type mismatch in Plausible:

https://github.com/plausible/analytics/blob/a5bac43981daa6b2106bd4d68fcf10678ee6feb9/lib/plausible/clickhouse_session_v2.ex#L7-L32

But yours is more complicated :)

hkrutzer commented 1 year ago

Thanks, I also found a mistake on my end so it doesn't seem to be an issue with ecto_ch, or I can fix it with a custom type 🙂