balance-platform / pillar Goto Github PK

The lib doesn't support read response as stream isn't it?

I'm currently getting this error:

{:error, %Pillar.HttpClient.TransportError{reason: :socket_closed_remotely}}

I believe the issue is with httpc.request and after looking around to similar issues I've found these 2 topics, that could possibly be relevant.

issue with default httpc adapter
issue with default profile in httpc

Hello. I think it would be good idea to allow ro run arbitrary code in migration. It would be useful for our case.

We have shard-tables like this:

transactions_1
transactions_2
transaction_N

The number of shards is defined by configuration. So we can't just run static SQL-query to migrate all those table.

In fact, we do migrations manually like this:

  @spec list_tables() :: [String.t()]
  def list_tables do
    {:ok, tables} = Bo.ClickhouseMaster.query("SHOW TABLES FORMAT JSON")
    Enum.map(tables, fn %{"name" => table_name} -> table_name end)
  end

  @spec migrate_many_tables(String.t(), String.t(), map()) :: :ok
  def migrate_many_tables(table_prefix, query_pattern, params \\ %{}) do
    list_tables()
    |> Enum.filter(fn table_name -> String.starts_with?(table_name, table_prefix) end)
    |> Enum.map(fn table_name -> String.replace(query_pattern, "{table_name}", table_name) end)
    |> Enum.map(fn query -> Bo.ClickhouseMaster.query(query, params) end)
    :ok
  end

  def migration_1 do
    query = """
    ALTER TABLE {table_name}
    ADD COLUMN IF NOT EXISTS 
    method_type String DEFAULT ''
    """

    migrate_many_tables("transactions", query)
  end

Do it manually is not good at all, so we are thinking to run it from Ecto migrations :) This idea is funny, but it will work. The other option is to improve Pillar migrations.

Elixir 1.12.3

got an error in phoenix app

"updated_at" => {:error, "feature needs elixir v1.11 minimum"}

iex

function_exported?(DateTime, :new, 3)
# => false
Code.ensure_loaded(DateTime)
# => {:module, DateTime}
function_exported?(DateTime, :new, 3)
# => true

Sometimes queries may execute long time.

Right now __using__ strategy generates one Singleton Worker, which can be busy at the moment, would be great to have multiple workers, aka Poolboy

Does have any plans to implement a Sandbox to be used in the test environment like Ecto does?

It looks like there is limited support for LowCardinality columns, specifically for LowCardinality(String). Any other type, UInt16, Float, etc) fails to convert.

However, there is full support for Nullable(X). It seems like that solution, modified for LowCardinality would solve this issue.

  def convert("Nullable" <> type, value) do
    case is_nil(value) do
      true -> nil
      false -> convert(type, value)
    end
  end

Possible solution would be:

  def convert("LowCardinality" <> type, value) do
    case is_nil(value) do
      true -> nil
      false -> convert(type, value)
    end
  end

INSERT queries don't support FORMAT option

At the moment, selecting IPv4 or IPv6 columns from CH fails in Pillar.TypeConvert.ToElixir.convert/2, since those two types are not checked. I'd be happy to send a PR, but I'd like to get an agreement, on what Elixir data type should be returned.

We could either return it as Erlang's :inet.ip_address() type (see https://www.erlang.org/doc/man/inet.html#type-ip_address ) or just as plain string.

What do you think?

Pillar is missing support for CH Map type. Now, this one is a little bit more tricky. Naturally, a Clickhouse Map can be converted to an Elixir map and vice versa. Handling the sub types correctly is the tricky part.

ToClickhouse needs to check if all keys have the same type and all values have the same type, e.g. %{"foo" => "bar", 1 => "xxx"} can't work, since 1 is not a string (or "foo" is not an integer). Optionally, we could cast all keys/values to the correct type. But this would require, that we provide a type for the query. I guess using maps as params in select queries should be quite rare (e.g. who does select ... where my_map == map('a', 'b') ?). It's probably more likely that people use maps directly in inserts or as result columns in selects.

Also, I'm not sure how easy it would be to handle sub types universally in a correct way in ToElixir. E.g. Map(String, UInt16) or even trickier Map(LowCardinality(String), DateTime)). Is this something you think is feasible? Or should we maybe start with a fixed set of defined (and common) types, like Map(String, String) ?

What do you think?

Again, happy to send a PR (or at least help), if you are interested.

From 22.6 (tested previous versions 22.2, 22.3, 22.4, 22.5 and they were fine), the following test errors appear as well:

  1) test #insert_to_table bad request with unexistable fields (PillarTest)
     test/pillar_test.exs:462
     match (=) failed
     code:  assert {:error, result} =
              Pillar.insert_to_table(conn, table_name, %{
                field0: 1.1,
                field1: "Hello",
                field2: 0,
                field3: [1, 2, 3],
                field4: "this field doesn't exists"
              })
     left:  {:error, result}
     right: {:ok, ""}
     stacktrace:
       test/pillar_test.exs:476: (test)

  2) test If bulk insert ends by error, handle function works on_errors option, dump_to_file -> saves data to file (Pillar.BulkInsertBufferTest)
     test/pillar/bulk_insert_buffer_test.exs:111
     ** (File.Error) could not read file "errors_from_bulk_insert_tests": no such file or directory
     code: assert File.read!("errors_from_bulk_insert_tests") == "[%{a: \"hello\", b: \"honey\"}]"
     stacktrace:
       (elixir 1.14.0) lib/file.ex:358: File.read!/1
       test/pillar/bulk_insert_buffer_test.exs:117: (test)

........................................
Finished in 14.5 seconds (0.00s async, 14.5s sync)
86 tests, 2 failures

Looks like a valid result is now returned?

Hi, can you provide an example how to use pool of workers with runtime config, to hide connection strings in env variables?

defmodule ClickhouseMaster do
  use Pillar,
    connection_strings: [
      "http://user:password@host-master-1:8123/database",
      "http://user:password@host-master-2:8123/database"
    ],
    name: __MODULE__,
    pool_size: 15
end

ClickhouseMaster.start_link()

in this scenario variables are being read while compilation time, if we try to pass Application.fetch_env or similar, in Application children = [...] I get a compile time error

• Allow to set default timeout in __using__ stategy
• Allow override timeout on query methods via 4 argument (options)

for example, a value with incorrect type is being added to a clickhouse column, we get 500 error from clickhouse and entire bulk insert gets rejected /lost. is there any way this 500 error can be caught in application so a retry logic can be added?

:httpc adds an empty te request header, which renders all requests to Clickhouse bad. It is unknown to me how to change this behavior.

Source: http://erlang.org/pipermail/erlang-questions/2010-April/050464.html

How to replicate:

• ClickhouseMaster.query("SELECT * from events")

Request:

POST /?database=<omitted>&password=<omitted>&user=<omitted> HTTP/1.1
content-type: application/json
content-length: 20
te: 
host: localhost:8123
connection: keep-alive

SELECT * from events

Response:

HTTP/1.1 400 Bad Request
Connection: Close

Would it make sense to add elixir to clickhouse tuple support?
I was able to get it working locally by adding

  def convert(param) when is_tuple(param) do
    values = param
    |> Tuple.to_list()
    |> Enum.map_join(",", &convert/1)

    "(#{values})"
  end

Hello. I found out that pillars incorrectly converts boolean parameters.

Example:

iex(4)> ClickhouseMaster.query("select 1 where 1 == 1");
{:ok, 1}

iex(5)> ClickhouseMaster.query("select 1 where 1 == {bool_value}", %{bool_value: true})
{:error,
 %Pillar.HttpClient.Response{
   body: <<67, 111, 100, 101, 58, 32, 54, 50, 46, 32, 68, 66, 58, 58, 69, 120,
     99, 101, 112, 116, 105, 111, 110, 58, 32, 83, 121, 110, 116, 97, 120, 32,
     101, 114, 114, 111, 114, 58, 32, 102, 97, 105, 108, 101, 100, 32, 97,
     ...>>,
   headers: [

The problem is here: https://github.com/balance-platform/pillar/blob/master/lib/pillar/query_builder.ex#L15-L17

iex(7)> String.replace("where 1 == {bool_value}", ["{bool_value}"], fn _ -> 1 end)
<<119, 104, 101, 114, 101, 32, 49, 32, 61, 61, 32, 1>>

The fix is that
https://github.com/balance-platform/pillar/blob/master/lib/pillar/type_convert/to_clickhouse.ex#L26
should return string, not int, as it is done here:
https://github.com/balance-platform/pillar/blob/master/lib/pillar/type_convert/to_clickhouse.ex#L22

By the way, convert could be implemented like this:

def convert(true), do: "1"
def convert(false), do: "0"

but that is not a big deal.

Has there been any thoughts about converting an Ecto.Query to valid clickhouse sql?

That way users could write queries using Ecto.Query and then convert the ecto queries to valid clickhouse SQL?

We could start with just some simple queries and support the basics.

Thoughts?

Pillar puts the Clickhouse password in the URL when connecting. This means it can end up in log files and is more easily leaked.

https://clickhouse.com/docs/en/interfaces/http/#default-database says:

The username and password can be indicated in one of three ways:
[..]
2. In the ‘user’ and ‘password’ URL parameters (We do not recommend using this method as the parameter might be logged by web proxy and cached in the browser).

The documentation recommends using either a HTTP header, or HTTP basic auth.

Greetings!

We encountered unexpected behavior. The data type DateTime64 allows milliseconds, but Pillar rounds it to seconds.

For example input data:

created_1 = "2023-11-30T13:45:00.500000Z"
created_2 = "2023-11-30T13:45:00.750000Z"
created_3 = "2023-11-30T13:45:00.850000Z"
created_4 = "2023-11-30T13:45:01.200000Z"

In ClickHouse it saved like:

There are may be a reason for this behavior. Can you explain why it works this way?

We have problem to build pagination based on date when we insert more than 1 records per second.