plausible / ecto_ch

ClickHouse adapter for Ecto
MIT License
79 stars 11 forks source link

DateTime64 does not using precision when building queries #178

Closed macumbr closed 5 months ago

macumbr commented 5 months ago

I'm using the ecto_ch to connect on clickhouse.

When doing this query

query: #Ecto.Query<from i0 in TrioService.Clickhouse.TrioSpi.In, where: i0.timestamp >= ^"2024-06-28 20:02:17.382780", where: i0.company_id == ^"01903762-1022-510e-2337-c6b01a315401">

it generates this query on clickhouse. I simplified the query for an easier understanding.

SELECT * FROM trio_service_log.in_documents AS i0 WHERE (i0.timestamp >= _CAST('1719604937.382', 'DateTime64')) AND (i0.company_id = '01903762-1022-510e-2337-c6b01a315401') LIMIT _CAST(4, 'Int64')

After a little bit of testing, I run the query below directly on clickhouse passing microseconds

SELECT * FROM trio_service_log.in_documents AS i0 WHERE (i0.timestamp >= _CAST('1719604937.382780', 'DateTime64(6)')) AND (i0.company_id = '01903762-1022-510e-2337-c6b01a315401') LIMIT _CAST(4, 'Int64')

It will use all the microseconds digits in the query and return the registry that I was expecting.

To fix this problem, I suggest to change the file. lib/ecto/adapters/clickhouse/connection.ex:1108 from

"DateTime64"

to

"DateTime64(#{precision})"

The whole method with the fix is


defp param_type(%DateTime{microsecond: microsecond}) do
    case microsecond do
      {_val, precision} when precision > 0 ->
        "DateTime64(#{precision})"

      _ ->
        "DateTime"
    end
  end

Passing the precision, fixed my issue. I tried to open a branch and commit the fix in order to create a PR to run all the tests, but I don't have access.

Please, have a look on this little issue and let me know if you need any further details.

Thanks in advance,

Murilo

ruslandoga commented 5 months ago

👋 @macumbr

Thank you very much for a very detailed explanation!

I tried to open a branch and commit the fix in order to create a PR to run all the tests, but I don't have access.

The usual approach is to fork the repo and then open a PR from the forked repo into the original one.

ruslandoga commented 5 months ago

I've published the fix as v0.3.8

Thank you again for reporting the issue!