Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add possibility to run arbitrary code in migration #61

Closed
yzh44yzh opened this issue Mar 3, 2023 · 4 comments
Closed

Add possibility to run arbitrary code in migration #61

yzh44yzh opened this issue Mar 3, 2023 · 4 comments

Comments

@yzh44yzh
Copy link

yzh44yzh commented Mar 3, 2023

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.

@sofakingworld
Copy link
Member

@yzh44yzh Hello!

First problem is in Multi-statement query, isn't it?

I've tried to create N tables from config, and got this error:

%MatchError{term: {:error, %Pillar.HttpClient.Response{body: "Code: 62. DB::Exception: Syntax error (Multi-statements are not allowed): failed at position 77 (end of query): ;CREATE TABLE IF NOT EXISTS example_1 (field FixedString(10)) ENGINE = Memory;CREATE TABLE IF NOT EXISTS example_2 (field FixedString(10)) ENGINE = Memory;CREAT. . (SYNTAX_ERROR) (version 22.1.3.7 (official build))\n", headers: [{"date", "Fri, 03 Mar 2023 14:02:19 GMT"}, {"connection", "Keep-Alive"}, {"content-type", "text/plain; charset=UTF-8"}, {"x-clickhouse-server-display-name", "da7091889375"}, {"transfer-encoding", "chunked"}, {"x-clickhouse-exception-code", "62"}, {"keep-alive", "timeout=3"}, {"x-clickhouse-summary", "{\"read_rows\":\"0\",\"read_bytes\":\"0\",\"written_rows\":\"0\",\"written_bytes\":\"0\",\"total_rows_to_read\":\"0\"}"}], status_code: 400}}}

(Multi-statements are not allowed) Error here

My migration looks like:

defmodule Pillar.Migrations.CreateMultipleTables do

  def up do
    (0..number_of_shards) |> Enum.map(fn i ->
      "CREATE TABLE IF NOT EXISTS example_#{i} (field FixedString(10)) ENGINE = Memory"
    end) |> Enum.join(";")
  end

  def number_of_shards do
    (System.get_env("SHARDS_COUNT") || "10") |> String.to_integer()
  end
end

For your case, I guess we can improve Pillar migrations,

for example if migration has function multiple_up, then migration should run list of SQL queries

Something like this:
image

What do you think? Could it help?

@yzh44yzh
Copy link
Author

yzh44yzh commented Mar 3, 2023

Good idea. I believe multi-statement migrations could be used more widely that just for sharded tables.

@sofakingworld
Copy link
Member

@yzh44yzh I found, that its already done

defmodule Pillar.Migrations.CreateMultipleTables do
  def up do
    Enum.map(0..10, fn i ->
      "CREATE TABLE IF NOT EXISTS transaction_table_shard_#{i} (field FixedString(10)) ENGINE = Memory"
    end)
  end
end

If in up function is list, it will execute each SQL in one migration

@yzh44yzh
Copy link
Author

yzh44yzh commented Mar 4, 2023

Great, thank you

@yzh44yzh yzh44yzh closed this as completed Mar 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants