From 26aadea737cee1b04c0f1284c599f13c2ddd7ba2 Mon Sep 17 00:00:00 2001 From: Roel Bondoc Date: Sat, 22 Jun 2024 12:39:22 -0400 Subject: [PATCH 01/22] fix: assign config to a variable instead of using a method (#143) --- lib/tasks/clickhouse.rake | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/tasks/clickhouse.rake b/lib/tasks/clickhouse.rake index 4854021d..1676b22d 100644 --- a/lib/tasks/clickhouse.rake +++ b/lib/tasks/clickhouse.rake @@ -37,6 +37,8 @@ namespace :clickhouse do end namespace :structure do + config = ActiveRecord::Base.configurations.configs_for(env_name: Rails.env, name: 'clickhouse') + desc 'Load database structure' task load: ['db:check_protected_environments'] do ClickhouseActiverecord::Tasks.new(config).structure_load(Rails.root.join('db/clickhouse_structure.sql')) @@ -85,8 +87,4 @@ namespace :clickhouse do Rake::Task['clickhouse:schema:dump'].execute(simple: true) end end - - def config - ActiveRecord::Base.configurations.configs_for(env_name: Rails.env, name: 'clickhouse') - end end From 65ef7e30189d57b6a29805391c75bebc03f56902 Mon Sep 17 00:00:00 2001 From: nixx Date: Thu, 11 Jul 2024 21:19:01 +0300 Subject: [PATCH 02/22] change tests to migration 7.1 version & fix #133 --- .../connection_adapters/clickhouse/schema_definitions.rb | 6 ++++++ lib/active_record/connection_adapters/clickhouse_adapter.rb | 6 +++++- lib/clickhouse-activerecord/version.rb | 2 +- .../migrations/add_array_datetime/1_create_actions_table.rb | 2 +- .../migrations/add_sample_data/1_create_sample_table.rb | 2 +- .../migrations/add_sample_data/2_create_join_table.rb | 2 +- .../migrations/dsl_add_column/1_create_some_table.rb | 2 +- .../migrations/dsl_add_column/2_modify_some_table.rb | 2 +- .../dsl_create_function/1_create_some_function.rb | 2 +- .../1_create_some_table.rb | 2 +- .../1_create_some_table.rb | 2 +- .../1_create_some_table_1.rb | 2 +- .../2_create_some_table_2.rb | 2 +- .../dsl_create_view_with_to_section/3_create_some_view.rb | 2 +- .../1_create_some_table.rb | 2 +- .../2_create_some_view.rb | 2 +- .../migrations/dsl_drop_column/1_create_some_table.rb | 2 +- .../migrations/dsl_drop_column/2_modify_some_table.rb | 2 +- .../migrations/dsl_drop_table/1_create_some_table.rb | 2 +- .../fixtures/migrations/dsl_drop_table/2_drop_some_table.rb | 2 +- .../migrations/dsl_drop_table_sync/1_create_some_table.rb | 2 +- .../migrations/dsl_drop_table_sync/2_drop_some_table.rb | 2 +- .../dsl_table_buffer_creation/1_create_some_table.rb | 2 +- .../dsl_table_with_datetime_creation/1_create_some_table.rb | 2 +- .../dsl_table_with_decimal_creation/1_create_some_table.rb | 2 +- .../dsl_table_with_engine_creation/1_create_some_table.rb | 2 +- .../dsl_table_with_enum_creation/1_create_some_table.rb | 2 +- .../1_create_some_table.rb | 2 +- .../1_create_some_table.rb | 2 +- .../dsl_table_with_uuid_creation/1_create_some_table.rb | 2 +- .../plain_function_creation/1_create_some_function.rb | 2 +- .../migrations/plain_table_creation/1_create_some_table.rb | 2 +- 32 files changed, 41 insertions(+), 31 deletions(-) diff --git a/lib/active_record/connection_adapters/clickhouse/schema_definitions.rb b/lib/active_record/connection_adapters/clickhouse/schema_definitions.rb index d02f1bec..384b2ce3 100644 --- a/lib/active_record/connection_adapters/clickhouse/schema_definitions.rb +++ b/lib/active_record/connection_adapters/clickhouse/schema_definitions.rb @@ -93,6 +93,12 @@ def enum(*args, **options) args.each { |name| column(name, kind, **options.except(:limit)) } end + + private + + def valid_column_definition_options + super + [:array, :low_cardinality, :fixed_string, :value, :type] + end end class IndexDefinition diff --git a/lib/active_record/connection_adapters/clickhouse_adapter.rb b/lib/active_record/connection_adapters/clickhouse_adapter.rb index b6faabd8..9d6ee4d8 100644 --- a/lib/active_record/connection_adapters/clickhouse_adapter.rb +++ b/lib/active_record/connection_adapters/clickhouse_adapter.rb @@ -293,7 +293,11 @@ def create_table(table_name, **options, &block) options = apply_replica(table_name, options) td = create_table_definition(apply_cluster(table_name), **options) block.call td if block_given? - td.column(:id, options[:id], null: false) if options[:id].present? && td[:id].blank? && options[:as].blank? + # support old migration version: in 5.0 options id: :integer, but 7.1 options empty + # todo remove auto add id column in future + if (!options.key?(:id) || options[:id].present? && options[:id] != false) && td[:id].blank? && options[:as].blank? + td.column(:id, options[:id] || :integer, null: false) + end if options[:force] drop_table(table_name, options.merge(if_exists: true)) diff --git a/lib/clickhouse-activerecord/version.rb b/lib/clickhouse-activerecord/version.rb index efa6f1df..548847fc 100644 --- a/lib/clickhouse-activerecord/version.rb +++ b/lib/clickhouse-activerecord/version.rb @@ -1,3 +1,3 @@ module ClickhouseActiverecord - VERSION = '1.0.9' + VERSION = '1.0.10' end diff --git a/spec/fixtures/migrations/add_array_datetime/1_create_actions_table.rb b/spec/fixtures/migrations/add_array_datetime/1_create_actions_table.rb index 7a1cd7f2..17048fbd 100644 --- a/spec/fixtures/migrations/add_array_datetime/1_create_actions_table.rb +++ b/spec/fixtures/migrations/add_array_datetime/1_create_actions_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateActionsTable < ActiveRecord::Migration[5.0] +class CreateActionsTable < ActiveRecord::Migration[7.1] def up create_table :actions, options: 'MergeTree ORDER BY date', force: true do |t| t.datetime :array_datetime, null: false, array: true diff --git a/spec/fixtures/migrations/add_sample_data/1_create_sample_table.rb b/spec/fixtures/migrations/add_sample_data/1_create_sample_table.rb index 9619bfe5..0ae71253 100644 --- a/spec/fixtures/migrations/add_sample_data/1_create_sample_table.rb +++ b/spec/fixtures/migrations/add_sample_data/1_create_sample_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSampleTable < ActiveRecord::Migration[5.0] +class CreateSampleTable < ActiveRecord::Migration[7.1] def up create_table :sample, options: 'ReplacingMergeTree PARTITION BY toYYYYMM(date) ORDER BY (event_name)' do |t| t.string :event_name, null: false diff --git a/spec/fixtures/migrations/add_sample_data/2_create_join_table.rb b/spec/fixtures/migrations/add_sample_data/2_create_join_table.rb index 7f4fb543..7436e8b6 100644 --- a/spec/fixtures/migrations/add_sample_data/2_create_join_table.rb +++ b/spec/fixtures/migrations/add_sample_data/2_create_join_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateJoinTable < ActiveRecord::Migration[5.0] +class CreateJoinTable < ActiveRecord::Migration[7.1] def up create_table :joins, options: 'MergeTree PARTITION BY toYYYYMM(date) ORDER BY (event_name)' do |t| t.string :event_name, null: false diff --git a/spec/fixtures/migrations/dsl_add_column/1_create_some_table.rb b/spec/fixtures/migrations/dsl_add_column/1_create_some_table.rb index 35b899c5..c12567de 100644 --- a/spec/fixtures/migrations/dsl_add_column/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_add_column/1_create_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some, options: 'MergeTree PARTITION BY toYYYYMM(date) ORDER BY (date)' do |t| t.date :date, null: false diff --git a/spec/fixtures/migrations/dsl_add_column/2_modify_some_table.rb b/spec/fixtures/migrations/dsl_add_column/2_modify_some_table.rb index e6d5b479..afb1dcaf 100644 --- a/spec/fixtures/migrations/dsl_add_column/2_modify_some_table.rb +++ b/spec/fixtures/migrations/dsl_add_column/2_modify_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class ModifySomeTable < ActiveRecord::Migration[5.0] +class ModifySomeTable < ActiveRecord::Migration[7.1] def up add_column :some, :new_column, :big_integer end diff --git a/spec/fixtures/migrations/dsl_create_function/1_create_some_function.rb b/spec/fixtures/migrations/dsl_create_function/1_create_some_function.rb index 8237036d..f49bd4df 100644 --- a/spec/fixtures/migrations/dsl_create_function/1_create_some_function.rb +++ b/spec/fixtures/migrations/dsl_create_function/1_create_some_function.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeFunction < ActiveRecord::Migration[5.0] +class CreateSomeFunction < ActiveRecord::Migration[7.1] def up create_function :some_fun, "(x,y) -> x + y" end diff --git a/spec/fixtures/migrations/dsl_create_table_with_cluster_name_alias/1_create_some_table.rb b/spec/fixtures/migrations/dsl_create_table_with_cluster_name_alias/1_create_some_table.rb index 0fc454df..2c94f452 100644 --- a/spec/fixtures/migrations/dsl_create_table_with_cluster_name_alias/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_create_table_with_cluster_name_alias/1_create_some_table.rb @@ -1,4 +1,4 @@ -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def change create_table :some, options: 'MergeTree PARTITION BY toYYYYMM(date) ORDER BY (date)', sync: true, id: false do |t| t.date :date, null: false diff --git a/spec/fixtures/migrations/dsl_create_table_with_distributed/1_create_some_table.rb b/spec/fixtures/migrations/dsl_create_table_with_distributed/1_create_some_table.rb index 6e7a049b..231db516 100644 --- a/spec/fixtures/migrations/dsl_create_table_with_distributed/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_create_table_with_distributed/1_create_some_table.rb @@ -1,4 +1,4 @@ -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def change create_table :some_distributed, with_distributed: :some, id: false, options: 'MergeTree PARTITION BY toYYYYMM(date) ORDER BY (date)' do |t| t.date :date, null: false diff --git a/spec/fixtures/migrations/dsl_create_view_with_to_section/1_create_some_table_1.rb b/spec/fixtures/migrations/dsl_create_view_with_to_section/1_create_some_table_1.rb index 380cac45..54ad2f0e 100644 --- a/spec/fixtures/migrations/dsl_create_view_with_to_section/1_create_some_table_1.rb +++ b/spec/fixtures/migrations/dsl_create_view_with_to_section/1_create_some_table_1.rb @@ -1,4 +1,4 @@ -class CreateSomeTable1 < ActiveRecord::Migration[5.0] +class CreateSomeTable1 < ActiveRecord::Migration[7.1] def change create_table :some_table_1, options: 'MergeTree ORDER BY col' do |t| t.string :col, null: false diff --git a/spec/fixtures/migrations/dsl_create_view_with_to_section/2_create_some_table_2.rb b/spec/fixtures/migrations/dsl_create_view_with_to_section/2_create_some_table_2.rb index 6f2135d6..452158ed 100644 --- a/spec/fixtures/migrations/dsl_create_view_with_to_section/2_create_some_table_2.rb +++ b/spec/fixtures/migrations/dsl_create_view_with_to_section/2_create_some_table_2.rb @@ -1,4 +1,4 @@ -class CreateSomeTable2 < ActiveRecord::Migration[5.0] +class CreateSomeTable2 < ActiveRecord::Migration[7.1] def change create_table :some_table_2, options: 'MergeTree ORDER BY col' do |t| t.string :col, null: false diff --git a/spec/fixtures/migrations/dsl_create_view_with_to_section/3_create_some_view.rb b/spec/fixtures/migrations/dsl_create_view_with_to_section/3_create_some_view.rb index 13ff5af2..ba5b8548 100644 --- a/spec/fixtures/migrations/dsl_create_view_with_to_section/3_create_some_view.rb +++ b/spec/fixtures/migrations/dsl_create_view_with_to_section/3_create_some_view.rb @@ -1,4 +1,4 @@ -class CreateSomeView < ActiveRecord::Migration[5.0] +class CreateSomeView < ActiveRecord::Migration[7.1] def change create_view :some_view, materialized: true, as: 'select * from some_table_1', to: 'some_table_2' end diff --git a/spec/fixtures/migrations/dsl_create_view_without_to_section/1_create_some_table.rb b/spec/fixtures/migrations/dsl_create_view_without_to_section/1_create_some_table.rb index 530869ec..05e8612f 100644 --- a/spec/fixtures/migrations/dsl_create_view_without_to_section/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_create_view_without_to_section/1_create_some_table.rb @@ -1,4 +1,4 @@ -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def change create_table :some_table, options: 'MergeTree ORDER BY col' do |t| t.string :col, null: false diff --git a/spec/fixtures/migrations/dsl_create_view_without_to_section/2_create_some_view.rb b/spec/fixtures/migrations/dsl_create_view_without_to_section/2_create_some_view.rb index ac867e62..ac319364 100644 --- a/spec/fixtures/migrations/dsl_create_view_without_to_section/2_create_some_view.rb +++ b/spec/fixtures/migrations/dsl_create_view_without_to_section/2_create_some_view.rb @@ -1,4 +1,4 @@ -class CreateSomeView < ActiveRecord::Migration[5.0] +class CreateSomeView < ActiveRecord::Migration[7.1] def change create_view :some_view, materialized: true, as: 'select * from some_table' end diff --git a/spec/fixtures/migrations/dsl_drop_column/1_create_some_table.rb b/spec/fixtures/migrations/dsl_drop_column/1_create_some_table.rb index 35b899c5..c12567de 100644 --- a/spec/fixtures/migrations/dsl_drop_column/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_drop_column/1_create_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some, options: 'MergeTree PARTITION BY toYYYYMM(date) ORDER BY (date)' do |t| t.date :date, null: false diff --git a/spec/fixtures/migrations/dsl_drop_column/2_modify_some_table.rb b/spec/fixtures/migrations/dsl_drop_column/2_modify_some_table.rb index f68d6eda..b0618c85 100644 --- a/spec/fixtures/migrations/dsl_drop_column/2_modify_some_table.rb +++ b/spec/fixtures/migrations/dsl_drop_column/2_modify_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class ModifySomeTable < ActiveRecord::Migration[5.0] +class ModifySomeTable < ActiveRecord::Migration[7.1] def up remove_column :some, :id end diff --git a/spec/fixtures/migrations/dsl_drop_table/1_create_some_table.rb b/spec/fixtures/migrations/dsl_drop_table/1_create_some_table.rb index 35b899c5..c12567de 100644 --- a/spec/fixtures/migrations/dsl_drop_table/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_drop_table/1_create_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some, options: 'MergeTree PARTITION BY toYYYYMM(date) ORDER BY (date)' do |t| t.date :date, null: false diff --git a/spec/fixtures/migrations/dsl_drop_table/2_drop_some_table.rb b/spec/fixtures/migrations/dsl_drop_table/2_drop_some_table.rb index 4c9172b2..2d792781 100644 --- a/spec/fixtures/migrations/dsl_drop_table/2_drop_some_table.rb +++ b/spec/fixtures/migrations/dsl_drop_table/2_drop_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class DropSomeTable < ActiveRecord::Migration[5.0] +class DropSomeTable < ActiveRecord::Migration[7.1] def up drop_table :some end diff --git a/spec/fixtures/migrations/dsl_drop_table_sync/1_create_some_table.rb b/spec/fixtures/migrations/dsl_drop_table_sync/1_create_some_table.rb index 35b899c5..c12567de 100644 --- a/spec/fixtures/migrations/dsl_drop_table_sync/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_drop_table_sync/1_create_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some, options: 'MergeTree PARTITION BY toYYYYMM(date) ORDER BY (date)' do |t| t.date :date, null: false diff --git a/spec/fixtures/migrations/dsl_drop_table_sync/2_drop_some_table.rb b/spec/fixtures/migrations/dsl_drop_table_sync/2_drop_some_table.rb index 69ddb690..e18e54b6 100644 --- a/spec/fixtures/migrations/dsl_drop_table_sync/2_drop_some_table.rb +++ b/spec/fixtures/migrations/dsl_drop_table_sync/2_drop_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class DropSomeTable < ActiveRecord::Migration[5.0] +class DropSomeTable < ActiveRecord::Migration[7.1] def up drop_table :some, sync: true end diff --git a/spec/fixtures/migrations/dsl_table_buffer_creation/1_create_some_table.rb b/spec/fixtures/migrations/dsl_table_buffer_creation/1_create_some_table.rb index c50e7a12..f61c92ea 100644 --- a/spec/fixtures/migrations/dsl_table_buffer_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_table_buffer_creation/1_create_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some do diff --git a/spec/fixtures/migrations/dsl_table_with_datetime_creation/1_create_some_table.rb b/spec/fixtures/migrations/dsl_table_with_datetime_creation/1_create_some_table.rb index bf2d30a0..74913724 100644 --- a/spec/fixtures/migrations/dsl_table_with_datetime_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_table_with_datetime_creation/1_create_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some, id: false, force: true do |t| t.datetime :datetime, null: false diff --git a/spec/fixtures/migrations/dsl_table_with_decimal_creation/1_create_some_table.rb b/spec/fixtures/migrations/dsl_table_with_decimal_creation/1_create_some_table.rb index 3aedb806..8610a103 100644 --- a/spec/fixtures/migrations/dsl_table_with_decimal_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_table_with_decimal_creation/1_create_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some do |t| t.decimal :money, precision: 16, scale: 4 diff --git a/spec/fixtures/migrations/dsl_table_with_engine_creation/1_create_some_table.rb b/spec/fixtures/migrations/dsl_table_with_engine_creation/1_create_some_table.rb index 35b899c5..c12567de 100644 --- a/spec/fixtures/migrations/dsl_table_with_engine_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_table_with_engine_creation/1_create_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some, options: 'MergeTree PARTITION BY toYYYYMM(date) ORDER BY (date)' do |t| t.date :date, null: false diff --git a/spec/fixtures/migrations/dsl_table_with_enum_creation/1_create_some_table.rb b/spec/fixtures/migrations/dsl_table_with_enum_creation/1_create_some_table.rb index 15e6eb2e..0fbbca2f 100644 --- a/spec/fixtures/migrations/dsl_table_with_enum_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_table_with_enum_creation/1_create_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some, id: false do |t| t.enum :enum8, value: { key1: 1, key2: 2 }, limit: 1, null: false diff --git a/spec/fixtures/migrations/dsl_table_with_fixed_string_creation/1_create_some_table.rb b/spec/fixtures/migrations/dsl_table_with_fixed_string_creation/1_create_some_table.rb index b209fef3..b20d3b3d 100644 --- a/spec/fixtures/migrations/dsl_table_with_fixed_string_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_table_with_fixed_string_creation/1_create_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some, id: false do |t| t.string :fixed_string1, fixed_string: 1, null: false diff --git a/spec/fixtures/migrations/dsl_table_with_low_cardinality_creation/1_create_some_table.rb b/spec/fixtures/migrations/dsl_table_with_low_cardinality_creation/1_create_some_table.rb index dac079fb..6ed320b8 100644 --- a/spec/fixtures/migrations/dsl_table_with_low_cardinality_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_table_with_low_cardinality_creation/1_create_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some, id: false do |t| t.string :col1, low_cardinality: true, null: false diff --git a/spec/fixtures/migrations/dsl_table_with_uuid_creation/1_create_some_table.rb b/spec/fixtures/migrations/dsl_table_with_uuid_creation/1_create_some_table.rb index 9b9fad9a..6e2c388d 100644 --- a/spec/fixtures/migrations/dsl_table_with_uuid_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_table_with_uuid_creation/1_create_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some, id: false do |t| t.uuid :col1, null: false diff --git a/spec/fixtures/migrations/plain_function_creation/1_create_some_function.rb b/spec/fixtures/migrations/plain_function_creation/1_create_some_function.rb index a9a558fd..329dc7f5 100644 --- a/spec/fixtures/migrations/plain_function_creation/1_create_some_function.rb +++ b/spec/fixtures/migrations/plain_function_creation/1_create_some_function.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeFunction < ActiveRecord::Migration[5.0] +class CreateSomeFunction < ActiveRecord::Migration[7.1] def up sql = <<~SQL CREATE FUNCTION some_fun AS (x,y) -> x + y diff --git a/spec/fixtures/migrations/plain_table_creation/1_create_some_table.rb b/spec/fixtures/migrations/plain_table_creation/1_create_some_table.rb index e1ade1e5..dc6e6461 100644 --- a/spec/fixtures/migrations/plain_table_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/plain_table_creation/1_create_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def up execute <<~SQL CREATE TABLE some ( From 29d00796b04f9de813cc8ee63ff7f5840b096dff Mon Sep 17 00:00:00 2001 From: HereC Date: Thu, 11 Jul 2024 14:44:13 -0400 Subject: [PATCH 03/22] Quote table names with periods (#140) --- lib/clickhouse-activerecord/schema_dumper.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/clickhouse-activerecord/schema_dumper.rb b/lib/clickhouse-activerecord/schema_dumper.rb index b03839a5..3196f1e6 100644 --- a/lib/clickhouse-activerecord/schema_dumper.rb +++ b/lib/clickhouse-activerecord/schema_dumper.rb @@ -104,7 +104,8 @@ def table(table, stream) raise StandardError, "Unknown type '#{column.sql_type}' for column '#{column.name}'" unless @connection.valid_type?(column.type) next if column.name == pk type, colspec = column_spec(column) - tbl.print " t.#{type} #{column.name.inspect}" + name = column.name =~ (/\./) ? "\"`#{column.name}`\"" : column.name.inspect + tbl.print " t.#{type} #{name}" tbl.print ", #{format_colspec(colspec)}" if colspec.present? tbl.puts end From f2dbed06eb32beac985cb55d97408881aee3c690 Mon Sep 17 00:00:00 2001 From: nixx Date: Fri, 19 Jul 2024 15:27:23 +0300 Subject: [PATCH 04/22] fix schema migration --- .../clickhouse/schema_statements.rb | 5 ++++- lib/clickhouse-activerecord/version.rb | 2 +- lib/core_extensions/active_record/schema_migration.rb | 10 ++++++++-- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb index 82512155..de1d939f 100644 --- a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb +++ b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb @@ -143,7 +143,10 @@ def with_yaml_fallback(value) # :nodoc: def request(sql, format = nil, settings = {}) formatted_sql = apply_format(sql, format) request_params = @connection_config || {} - @connection.post("/?#{request_params.merge(settings).to_param}", formatted_sql, 'User-Agent' => "Clickhouse ActiveRecord #{ClickhouseActiverecord::VERSION}") + @connection.post("/?#{request_params.merge(settings).to_param}", formatted_sql, { + 'User-Agent' => "Clickhouse ActiveRecord #{ClickhouseActiverecord::VERSION}", + 'Content-Type' => 'application/x-www-form-urlencoded', + }) end def apply_format(sql, format) diff --git a/lib/clickhouse-activerecord/version.rb b/lib/clickhouse-activerecord/version.rb index 548847fc..a15da01e 100644 --- a/lib/clickhouse-activerecord/version.rb +++ b/lib/clickhouse-activerecord/version.rb @@ -1,3 +1,3 @@ module ClickhouseActiverecord - VERSION = '1.0.10' + VERSION = '1.0.11' end diff --git a/lib/core_extensions/active_record/schema_migration.rb b/lib/core_extensions/active_record/schema_migration.rb index 0da6a7ee..96109ef9 100644 --- a/lib/core_extensions/active_record/schema_migration.rb +++ b/lib/core_extensions/active_record/schema_migration.rb @@ -36,10 +36,16 @@ def delete_version(version) connection.insert(im, "#{self.class} Create Rollback Version", primary_key, version) end - def all_versions + def versions return super unless connection.is_a?(::ActiveRecord::ConnectionAdapters::ClickhouseAdapter) - final.where(active: 1).order(:version).pluck(:version) + sm = ::Arel::SelectManager.new(arel_table) + sm.final! + sm.project(arel_table[primary_key]) + sm.order(arel_table[primary_key].asc) + sm.where([arel_table['active'].eq(1)]) + + connection.select_values(sm, "#{self.class} Load") end end end From 9dfcfb892b1f2b53fdb364954e4f8c97e961d382 Mon Sep 17 00:00:00 2001 From: nixx Date: Mon, 22 Jul 2024 13:58:41 +0300 Subject: [PATCH 05/22] fix schema dumper with low_cardinality --- lib/clickhouse-activerecord/schema_dumper.rb | 5 +++++ lib/clickhouse-activerecord/version.rb | 2 +- .../1_create_some_table.rb | 7 +++++++ .../dsl_table_creation/1_create_some_table.rb | 2 +- spec/single/migration_spec.rb | 13 +++++++++++++ 5 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 spec/fixtures/migrations/dsl_create_view_without_id/1_create_some_table.rb diff --git a/lib/clickhouse-activerecord/schema_dumper.rb b/lib/clickhouse-activerecord/schema_dumper.rb index 3196f1e6..4494aaaa 100644 --- a/lib/clickhouse-activerecord/schema_dumper.rb +++ b/lib/clickhouse-activerecord/schema_dumper.rb @@ -168,10 +168,15 @@ def schema_array(column) (column.sql_type =~ /Array?\(/).nil? ? nil : true end + def schema_low_cardinality(column) + (column.sql_type =~ /LowCardinality?\(/).nil? ? nil : true + end + def prepare_column_options(column) spec = {} spec[:unsigned] = schema_unsigned(column) spec[:array] = schema_array(column) + spec[:low_cardinality] = schema_low_cardinality(column) spec.merge(super).compact end diff --git a/lib/clickhouse-activerecord/version.rb b/lib/clickhouse-activerecord/version.rb index a15da01e..2e576690 100644 --- a/lib/clickhouse-activerecord/version.rb +++ b/lib/clickhouse-activerecord/version.rb @@ -1,3 +1,3 @@ module ClickhouseActiverecord - VERSION = '1.0.11' + VERSION = '1.0.12' end diff --git a/spec/fixtures/migrations/dsl_create_view_without_id/1_create_some_table.rb b/spec/fixtures/migrations/dsl_create_view_without_id/1_create_some_table.rb new file mode 100644 index 00000000..a9f9460f --- /dev/null +++ b/spec/fixtures/migrations/dsl_create_view_without_id/1_create_some_table.rb @@ -0,0 +1,7 @@ +class CreateSomeTable < ActiveRecord::Migration[7.1] + def change + create_table :some, id: false, options: 'MergeTree ORDER BY col' do |t| + t.string :col, null: false + end + end +end diff --git a/spec/fixtures/migrations/dsl_table_creation/1_create_some_table.rb b/spec/fixtures/migrations/dsl_table_creation/1_create_some_table.rb index 401300ea..37b7461f 100644 --- a/spec/fixtures/migrations/dsl_table_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_table_creation/1_create_some_table.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class CreateSomeTable < ActiveRecord::Migration[5.0] +class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some do diff --git a/spec/single/migration_spec.rb b/spec/single/migration_spec.rb index ac64d7ff..dba9fa24 100644 --- a/spec/single/migration_spec.rb +++ b/spec/single/migration_spec.rb @@ -58,6 +58,19 @@ end end + context 'without id' do + let(:directory) { 'dsl_create_view_without_id' } + it 'creates a table' do + subject + + current_schema = schema(model) + + expect(current_schema.keys.count).to eq(1) + expect(current_schema).to_not have_key('id') + expect(current_schema['col'].sql_type).to eq('String') + end + end + context 'with buffer table' do let(:directory) { 'dsl_table_buffer_creation' } it 'creates a table' do From 1582b50c630f8b48490367e18e10201ce1845a26 Mon Sep 17 00:00:00 2001 From: Daniel Westendorf Date: Fri, 26 Jul 2024 06:26:18 -0600 Subject: [PATCH 06/22] Normalize table name in schema dump (#148) strip the database name from schema dump --- lib/active_record/connection_adapters/clickhouse_adapter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_record/connection_adapters/clickhouse_adapter.rb b/lib/active_record/connection_adapters/clickhouse_adapter.rb index 9d6ee4d8..47c3d832 100644 --- a/lib/active_record/connection_adapters/clickhouse_adapter.rb +++ b/lib/active_record/connection_adapters/clickhouse_adapter.rb @@ -264,7 +264,7 @@ def create_schema_dumper(options) # :nodoc: # @param [String] table # @return [String] def show_create_table(table) - do_system_execute("SHOW CREATE TABLE `#{table}`")['data'].try(:first).try(:first).gsub(/[\n\s]+/m, ' ') + do_system_execute("SHOW CREATE TABLE `#{table}`")['data'].try(:first).try(:first).gsub(/[\n\s]+/m, ' ').gsub("#{@config[:database]}.", "") end # Create a new ClickHouse database. From 59dc1c24aa3b1ad778e4a4482bfa907f7b54f5e5 Mon Sep 17 00:00:00 2001 From: Daniel Westendorf Date: Wed, 31 Jul 2024 11:07:29 -0600 Subject: [PATCH 07/22] Nop out savepoint functionality (#150) --- .../connection_adapters/clickhouse_adapter.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/active_record/connection_adapters/clickhouse_adapter.rb b/lib/active_record/connection_adapters/clickhouse_adapter.rb index 47c3d832..e84711d6 100644 --- a/lib/active_record/connection_adapters/clickhouse_adapter.rb +++ b/lib/active_record/connection_adapters/clickhouse_adapter.rb @@ -133,6 +133,16 @@ def initialize(logger, connection_parameters, config) connect end + # Savepoints are not supported, noop + def create_savepoint(name) + end + + def exec_rollback_to_savepoint(name) + end + + def release_savepoint(name) + end + def migrations_paths @config[:migrations_paths] || 'db/migrate_clickhouse' end From fb386758bc6223a2142dd934327bf416eef089c8 Mon Sep 17 00:00:00 2001 From: Daniel Westendorf Date: Wed, 31 Jul 2024 12:23:20 -0600 Subject: [PATCH 08/22] Fix `#find_by` (#153) --- lib/arel/visitors/clickhouse.rb | 8 ++++++-- spec/single/model_spec.rb | 8 ++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/arel/visitors/clickhouse.rb b/lib/arel/visitors/clickhouse.rb index a8fd5993..c94a65e0 100644 --- a/lib/arel/visitors/clickhouse.rb +++ b/lib/arel/visitors/clickhouse.rb @@ -16,8 +16,12 @@ def aggregate(name, o, collector) # https://clickhouse.com/docs/en/sql-reference/statements/delete # DELETE and UPDATE in ClickHouse working only without table name def visit_Arel_Attributes_Attribute(o, collector) - collector << quote_table_name(o.relation.table_alias || o.relation.name) << '.' unless collector.value.start_with?('DELETE FROM ') || collector.value.include?(' UPDATE ') - collector << quote_column_name(o.name) + if collector.value.is_a?(String) + collector << quote_table_name(o.relation.table_alias || o.relation.name) << '.' unless collector.value.start_with?('DELETE FROM ') || collector.value.include?(' UPDATE ') + collector << quote_column_name(o.name) + else + super + end end def visit_Arel_Nodes_SelectOptions(o, collector) diff --git a/spec/single/model_spec.rb b/spec/single/model_spec.rb index e9e46b54..c2af5e15 100644 --- a/spec/single/model_spec.rb +++ b/spec/single/model_spec.rb @@ -107,6 +107,14 @@ class ModelPk < ActiveRecord::Base end end + describe '#find_by' do + let!(:record) { Model.create!(id: 1, event_name: 'some event') } + + it 'finds the record' do + expect(Model.find_by(id: 1, event_name: 'some event')).to eq(record) + end + end + describe '#reverse_order!' do it 'blank' do expect(Model.all.reverse_order!.map(&:event_name)).to eq([]) From 73fe9be578218b42cac2f8cdd6b837c5af8a9b9a Mon Sep 17 00:00:00 2001 From: nixx Date: Thu, 1 Aug 2024 10:02:39 +0300 Subject: [PATCH 09/22] rspec configure --- README.md | 10 +++++++++- lib/clickhouse-activerecord/rspec.rb | 12 ++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 lib/clickhouse-activerecord/rspec.rb diff --git a/README.md b/README.md index 176e6eba..83fb03ec 100644 --- a/README.md +++ b/README.md @@ -149,7 +149,15 @@ Structure load from `db/clickhouse_structure.sql` file: $ rake db:schema:dump $ rake db:schema:load $ rake db:structure:dump - $ rake db:structure:load + $ rake db:structure:load + +### RSpec + +For auto truncate tables before each test add to `spec/rails_helper.rb` file: + +``` +require 'clickhouse-activerecord/rspec' +``` ### Insert and select data diff --git a/lib/clickhouse-activerecord/rspec.rb b/lib/clickhouse-activerecord/rspec.rb new file mode 100644 index 00000000..dbcf4a0a --- /dev/null +++ b/lib/clickhouse-activerecord/rspec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +RSpec.configure do |config| + config.before do + ActiveRecord::Base.configurations.configurations.select { |x| x.env_name == Rails.env && x.adapter == 'clickhouse' }.each do |config| + ActiveRecord::Base.establish_connection(config) + ActiveRecord::Base.connection.tables.each do |table| + ActiveRecord::Base.connection.execute("TRUNCATE TABLE #{table}") + end + end + end +end From 9810bcec0ca9e15c2f5010dc72e7d1be11af1059 Mon Sep 17 00:00:00 2001 From: nixx Date: Thu, 1 Aug 2024 10:04:34 +0300 Subject: [PATCH 10/22] published - 1.0.13 --- lib/clickhouse-activerecord/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/clickhouse-activerecord/version.rb b/lib/clickhouse-activerecord/version.rb index 2e576690..ad280945 100644 --- a/lib/clickhouse-activerecord/version.rb +++ b/lib/clickhouse-activerecord/version.rb @@ -1,3 +1,3 @@ module ClickhouseActiverecord - VERSION = '1.0.12' + VERSION = '1.0.13' end From 8979eceb2f28b451dbdea3a2fee8238e6b515f90 Mon Sep 17 00:00:00 2001 From: Daniel Westendorf Date: Fri, 16 Aug 2024 06:30:03 -0600 Subject: [PATCH 11/22] Add connection view helper (#152) Co-authored-by: Chris Bisnett --- .../connection_adapters/clickhouse/schema_statements.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb index de1d939f..fb30918e 100644 --- a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb +++ b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb @@ -133,6 +133,13 @@ def with_yaml_fallback(value) # :nodoc: end end + def views(name = nil) + result = do_system_execute("SHOW TABLES WHERE engine = 'View'", name) + + return [] if result.nil? + result['data'].flatten + end + private # Make HTTP request to ClickHouse server From ada3cd2f3c2497001c32cefb96bd452e283f5042 Mon Sep 17 00:00:00 2001 From: Daniel Westendorf Date: Fri, 16 Aug 2024 06:33:20 -0600 Subject: [PATCH 12/22] Add support for Map datatype (#144) --- .gitignore | 1 + README.md | 1 + .../connection_adapters/clickhouse/oid/map.rb | 68 +++++++++++++++++++ .../clickhouse/schema_creation.rb | 3 + .../clickhouse/schema_definitions.rb | 2 +- .../clickhouse/schema_statements.rb | 2 +- .../connection_adapters/clickhouse_adapter.rb | 7 ++ lib/clickhouse-activerecord/schema_dumper.rb | 5 ++ .../add_map_datetime/1_create_verbs_table.rb | 11 +++ .../1_create_some_table.rb | 1 + .../1_create_some_table.rb | 1 + spec/single/migration_spec.rb | 8 ++- spec/single/model_spec.rb | 57 +++++++++++++++- 13 files changed, 162 insertions(+), 5 deletions(-) create mode 100644 lib/active_record/connection_adapters/clickhouse/oid/map.rb create mode 100644 spec/fixtures/migrations/add_map_datetime/1_create_verbs_table.rb diff --git a/.gitignore b/.gitignore index 42b720a5..63ff4ca8 100644 --- a/.gitignore +++ b/.gitignore @@ -54,3 +54,4 @@ crashlytics.properties crashlytics-build.properties fabric.properties .rspec_status +.tool-versions diff --git a/README.md b/README.md index 83fb03ec..07855931 100644 --- a/README.md +++ b/README.md @@ -207,6 +207,7 @@ false`. The default integer is `UInt32` | UInt64 | 0 to 18446744073709551615 | 5,6,7,8 | | UInt256 | 0 to ... | 8+ | | Array | ... | ... | +| Map | ... | ... | Example: diff --git a/lib/active_record/connection_adapters/clickhouse/oid/map.rb b/lib/active_record/connection_adapters/clickhouse/oid/map.rb new file mode 100644 index 00000000..08067a33 --- /dev/null +++ b/lib/active_record/connection_adapters/clickhouse/oid/map.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +module ActiveRecord + module ConnectionAdapters + module Clickhouse + module OID # :nodoc: + class Map < Type::Value # :nodoc: + + def initialize(sql_type) + @subtype = case sql_type + when /U?Int\d+/ + :integer + when /DateTime/ + :datetime + when /Date/ + :date + else + :string + end + end + + def type + @subtype + end + + def deserialize(value) + if value.is_a?(::Hash) + value.map { |k, item| [k.to_s, deserialize(item)] }.to_h + else + return value if value.nil? + case @subtype + when :integer + value.to_i + when :datetime + ::DateTime.parse(value) + when :date + ::Date.parse(value) + else + super + end + end + end + + def serialize(value) + if value.is_a?(::Hash) + value.map { |k, item| [k.to_s, serialize(item)] }.to_h + else + return value if value.nil? + case @subtype + when :integer + value.to_i + when :datetime + DateTime.new.serialize(value) + when :date + Date.new.serialize(value) + when :string + value.to_s + else + super + end + end + end + + end + end + end + end +end diff --git a/lib/active_record/connection_adapters/clickhouse/schema_creation.rb b/lib/active_record/connection_adapters/clickhouse/schema_creation.rb index d3b015f9..3d795a2b 100644 --- a/lib/active_record/connection_adapters/clickhouse/schema_creation.rb +++ b/lib/active_record/connection_adapters/clickhouse/schema_creation.rb @@ -33,6 +33,9 @@ def add_column_options!(sql, options) if options[:array] sql.gsub!(/\s+(.*)/, ' Array(\1)') end + if options[:map] + sql.gsub!(/\s+(.*)/, ' Map(String, \1)') + end sql.gsub!(/(\sString)\(\d+\)/, '\1') sql << " DEFAULT #{quote_default_expression(options[:default], options[:column])}" if options_include_default?(options) sql diff --git a/lib/active_record/connection_adapters/clickhouse/schema_definitions.rb b/lib/active_record/connection_adapters/clickhouse/schema_definitions.rb index 384b2ce3..c1a6335d 100644 --- a/lib/active_record/connection_adapters/clickhouse/schema_definitions.rb +++ b/lib/active_record/connection_adapters/clickhouse/schema_definitions.rb @@ -97,7 +97,7 @@ def enum(*args, **options) private def valid_column_definition_options - super + [:array, :low_cardinality, :fixed_string, :value, :type] + super + [:array, :low_cardinality, :fixed_string, :value, :type, :map] end end diff --git a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb index fb30918e..226c80e1 100644 --- a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb +++ b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb @@ -126,7 +126,7 @@ def assume_migrated_upto_version(version, migrations_paths = nil) # Fix insert_all method # https://github.com/PNixx/clickhouse-activerecord/issues/71#issuecomment-1923244983 def with_yaml_fallback(value) # :nodoc: - if value.is_a?(Array) + if value.is_a?(Array) || value.is_a?(Hash) value else super diff --git a/lib/active_record/connection_adapters/clickhouse_adapter.rb b/lib/active_record/connection_adapters/clickhouse_adapter.rb index e84711d6..4dd4cf09 100644 --- a/lib/active_record/connection_adapters/clickhouse_adapter.rb +++ b/lib/active_record/connection_adapters/clickhouse_adapter.rb @@ -8,6 +8,7 @@ require 'active_record/connection_adapters/clickhouse/oid/date' require 'active_record/connection_adapters/clickhouse/oid/date_time' require 'active_record/connection_adapters/clickhouse/oid/big_integer' +require 'active_record/connection_adapters/clickhouse/oid/map' require 'active_record/connection_adapters/clickhouse/oid/uuid' require 'active_record/connection_adapters/clickhouse/schema_definitions' require 'active_record/connection_adapters/clickhouse/schema_creation' @@ -221,6 +222,10 @@ def initialize_type_map(m) # :nodoc: m.register_type(%r(Array)) do |sql_type| Clickhouse::OID::Array.new(sql_type) end + + m.register_type(%r(Map)) do |sql_type| + Clickhouse::OID::Map.new(sql_type) + end end end @@ -233,6 +238,8 @@ def quote(value) case value when Array '[' + value.map { |v| quote(v) }.join(', ') + ']' + when Hash + '{' + value.map { |k, v| "#{quote(k)}: #{quote(v)}" }.join(', ') + '}' else super end diff --git a/lib/clickhouse-activerecord/schema_dumper.rb b/lib/clickhouse-activerecord/schema_dumper.rb index 4494aaaa..ef5577f5 100644 --- a/lib/clickhouse-activerecord/schema_dumper.rb +++ b/lib/clickhouse-activerecord/schema_dumper.rb @@ -168,6 +168,10 @@ def schema_array(column) (column.sql_type =~ /Array?\(/).nil? ? nil : true end + def schema_map(column) + (column.sql_type =~ /Map?\(/).nil? ? nil : true + end + def schema_low_cardinality(column) (column.sql_type =~ /LowCardinality?\(/).nil? ? nil : true end @@ -176,6 +180,7 @@ def prepare_column_options(column) spec = {} spec[:unsigned] = schema_unsigned(column) spec[:array] = schema_array(column) + spec[:map] = schema_map(column) spec[:low_cardinality] = schema_low_cardinality(column) spec.merge(super).compact end diff --git a/spec/fixtures/migrations/add_map_datetime/1_create_verbs_table.rb b/spec/fixtures/migrations/add_map_datetime/1_create_verbs_table.rb new file mode 100644 index 00000000..59cb8b23 --- /dev/null +++ b/spec/fixtures/migrations/add_map_datetime/1_create_verbs_table.rb @@ -0,0 +1,11 @@ +class CreateVerbsTable < ActiveRecord::Migration[7.1] + def up + create_table :verbs, options: 'MergeTree ORDER BY date', force: true do |t| + t.datetime :map_datetime, null: false, map: true + t.string :map_string, null: false, map: true + t.integer :map_int, null: false, map: true + t.date :date, null: false + end + end +end + diff --git a/spec/fixtures/migrations/dsl_table_with_fixed_string_creation/1_create_some_table.rb b/spec/fixtures/migrations/dsl_table_with_fixed_string_creation/1_create_some_table.rb index b20d3b3d..b16271ff 100644 --- a/spec/fixtures/migrations/dsl_table_with_fixed_string_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_table_with_fixed_string_creation/1_create_some_table.rb @@ -5,6 +5,7 @@ def up create_table :some, id: false do |t| t.string :fixed_string1, fixed_string: 1, null: false t.string :fixed_string16_array, fixed_string: 16, array: true, null: true + t.string :fixed_string16_map, fixed_string: 16, map: true, null: true end end end diff --git a/spec/fixtures/migrations/dsl_table_with_low_cardinality_creation/1_create_some_table.rb b/spec/fixtures/migrations/dsl_table_with_low_cardinality_creation/1_create_some_table.rb index 6ed320b8..9c33fecf 100644 --- a/spec/fixtures/migrations/dsl_table_with_low_cardinality_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_table_with_low_cardinality_creation/1_create_some_table.rb @@ -6,6 +6,7 @@ def up t.string :col1, low_cardinality: true, null: false t.string :col2, low_cardinality: true, null: true t.string :col3, low_cardinality: true, array: true, null: true + t.string :col4, low_cardinality: true, map: true, null: true end end end diff --git a/spec/single/migration_spec.rb b/spec/single/migration_spec.rb index dba9fa24..8bab4397 100644 --- a/spec/single/migration_spec.rb +++ b/spec/single/migration_spec.rb @@ -150,13 +150,15 @@ current_schema = schema(model) - expect(current_schema.keys.count).to eq(3) + expect(current_schema.keys.count).to eq(4) expect(current_schema).to have_key('col1') expect(current_schema).to have_key('col2') expect(current_schema).to have_key('col3') + expect(current_schema).to have_key('col4') expect(current_schema['col1'].sql_type).to eq('LowCardinality(String)') expect(current_schema['col2'].sql_type).to eq('LowCardinality(Nullable(String))') expect(current_schema['col3'].sql_type).to eq('Array(LowCardinality(Nullable(String)))') + expect(current_schema['col4'].sql_type).to eq('Map(String, LowCardinality(Nullable(String)))') end end @@ -167,11 +169,13 @@ current_schema = schema(model) - expect(current_schema.keys.count).to eq(2) + expect(current_schema.keys.count).to eq(3) expect(current_schema).to have_key('fixed_string1') expect(current_schema).to have_key('fixed_string16_array') + expect(current_schema).to have_key('fixed_string16_map') expect(current_schema['fixed_string1'].sql_type).to eq('FixedString(1)') expect(current_schema['fixed_string16_array'].sql_type).to eq('Array(Nullable(FixedString(16)))') + expect(current_schema['fixed_string16_map'].sql_type).to eq('Map(String, Nullable(FixedString(16)))') end end diff --git a/spec/single/model_spec.rb b/spec/single/model_spec.rb index c2af5e15..6365d353 100644 --- a/spec/single/model_spec.rb +++ b/spec/single/model_spec.rb @@ -269,7 +269,6 @@ class ModelPk < ActiveRecord::Base end context 'array' do - let!(:model) do Class.new(ActiveRecord::Base) do self.table_name = 'actions' @@ -324,4 +323,60 @@ class ModelPk < ActiveRecord::Base end end end + + context 'map' do + let!(:model) do + Class.new(ActiveRecord::Base) do + self.table_name = 'verbs' + end + end + + before do + migrations_dir = File.join(FIXTURES_PATH, 'migrations', 'add_map_datetime') + quietly { ActiveRecord::MigrationContext.new(migrations_dir, model.connection.schema_migration).up } + end + + describe '#create' do + it 'creates a new record' do + expect { + model.create!( + map_datetime: {a: 1.day.ago, b: Time.now, c: '2022-12-06 15:22:49'}, + map_string: {a: 'asdf', b: 'jkl' }, + map_int: {a: 1, b: 2}, + date: date + ) + }.to change { model.count } + record = model.first + expect(record.map_datetime.is_a?(Hash)).to be_truthy + expect(record.map_datetime['a'].is_a?(DateTime)).to be_truthy + expect(record.map_string['a'].is_a?(String)).to be_truthy + expect(record.map_string).to eq({'a' => 'asdf', 'b' => 'jkl'}) + expect(record.map_int.is_a?(Hash)).to be_truthy + expect(record.map_int).to eq({'a' => 1, 'b' => 2}) + end + + it 'create with insert all' do + expect { + model.insert_all([{ + map_datetime: {a: 1.day.ago, b: Time.now, c: '2022-12-06 15:22:49'}, + map_string: {a: 'asdf', b: 'jkl' }, + map_int: {a: 1, b: 2}, + date: date + }]) + }.to change { model.count } + end + + it 'get record' do + model.connection.insert("INSERT INTO #{model.table_name} (id, map_datetime, date) VALUES (1, {'a': '2022-12-05 15:22:49', 'b': '2022-12-06 15:22:49'}, '2022-12-06')") + expect(model.count).to eq(1) + record = model.first + expect(record.date.is_a?(Date)).to be_truthy + expect(record.date).to eq(Date.parse('2022-12-06')) + expect(record.map_datetime.is_a?(Hash)).to be_truthy + expect(record.map_datetime['a'].is_a?(DateTime)).to be_truthy + expect(record.map_datetime['a']).to eq(DateTime.parse('2022-12-05 15:22:49')) + expect(record.map_datetime['b']).to eq(DateTime.parse('2022-12-06 15:22:49')) + end + end + end end From 536bf8670a296b37c861f16dab3cdc092246a796 Mon Sep 17 00:00:00 2001 From: nixx Date: Fri, 16 Aug 2024 15:46:18 +0300 Subject: [PATCH 13/22] fix dump schema with numeric default --- .../clickhouse/schema_statements.rb | 53 ++++++++----------- .../connection_adapters/clickhouse_adapter.rb | 5 +- .../1_create_some_table.rb | 4 +- .../1_create_some_table.rb | 1 + .../1_create_some_table.rb | 2 +- .../1_create_some_table.rb | 2 +- spec/single/migration_spec.rb | 10 +++- 7 files changed, 39 insertions(+), 38 deletions(-) diff --git a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb index 226c80e1..ae472b6f 100644 --- a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb +++ b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb @@ -57,6 +57,12 @@ def tables(name = nil) result['data'].flatten end + def views(name = nil) + result = do_system_execute("SHOW TABLES WHERE engine = 'View'", name) + return [] if result.nil? + result['data'].flatten + end + def functions result = do_system_execute("SELECT name FROM system.functions WHERE origin = 'SQLUserDefined'") return [] if result.nil? @@ -133,13 +139,6 @@ def with_yaml_fallback(value) # :nodoc: end end - def views(name = nil) - result = do_system_execute("SHOW TABLES WHERE engine = 'View'", name) - - return [] if result.nil? - result['data'].flatten - end - private # Make HTTP request to ClickHouse server @@ -198,9 +197,9 @@ def create_table_definition(table_name, **options) def new_column_from_field(table_name, field, _definitions) sql_type = field[1] type_metadata = fetch_type_metadata(sql_type) - default = field[3] - default_value = extract_value_from_default(default) - default_function = extract_default_function(default_value, default) + default_value = extract_value_from_default(field[3], field[2]) + default_function = extract_default_function(field[3]) + default_value = lookup_cast_type(sql_type).cast(default_value) ClickhouseColumn.new(field[0], default_value, type_metadata, field[1].include?('Nullable'), default_function) end @@ -219,32 +218,22 @@ def table_structure(table_name) private # Extracts the value from a PostgreSQL column default definition. - def extract_value_from_default(default) - case default - # Quoted types - when /\Anow\(\)\z/m - nil - # Boolean types - when "true".freeze, "false".freeze - default - # Object identifier types - when "''" - '' - when /\A-?\d+\z/ - $1 - else - # Anything else is blank, some user type, or some function - # and we can't know the value of that, so return nil. - nil - end + def extract_value_from_default(default_expression, default_type) + return nil if default_type != 'DEFAULT' || default_expression.blank? + return nil if has_default_function?(default_expression) + + # Convert string + return $1 if default_expression.match(/^'(.*?)'$/) + + default_expression end - def extract_default_function(default_value, default) # :nodoc: - default if has_default_function?(default_value, default) + def extract_default_function(default) # :nodoc: + default if has_default_function?(default) end - def has_default_function?(default_value, default) # :nodoc: - !default_value && (%r{\w+\(.*\)} === default) + def has_default_function?(default) # :nodoc: + (%r{\w+\(.*\)} === default) end def format_body_response(body, format) diff --git a/lib/active_record/connection_adapters/clickhouse_adapter.rb b/lib/active_record/connection_adapters/clickhouse_adapter.rb index 4dd4cf09..8b41cb2a 100644 --- a/lib/active_record/connection_adapters/clickhouse_adapter.rb +++ b/lib/active_record/connection_adapters/clickhouse_adapter.rb @@ -84,7 +84,10 @@ def _delete_record(constraints) module ConnectionAdapters class ClickhouseColumn < Column - + private + def deduplicated + self + end end class ClickhouseAdapter < AbstractAdapter diff --git a/spec/fixtures/migrations/dsl_table_with_datetime_creation/1_create_some_table.rb b/spec/fixtures/migrations/dsl_table_with_datetime_creation/1_create_some_table.rb index 74913724..b3d644e6 100644 --- a/spec/fixtures/migrations/dsl_table_with_datetime_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_table_with_datetime_creation/1_create_some_table.rb @@ -3,8 +3,8 @@ class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some, id: false, force: true do |t| - t.datetime :datetime, null: false - t.datetime :datetime64, precision: 3, null: true + t.datetime :datetime, null: false, default: -> { 'now()' } + t.datetime :datetime64, precision: 3, null: true, default: -> { 'now64()' } end end end diff --git a/spec/fixtures/migrations/dsl_table_with_decimal_creation/1_create_some_table.rb b/spec/fixtures/migrations/dsl_table_with_decimal_creation/1_create_some_table.rb index 8610a103..0790eb6d 100644 --- a/spec/fixtures/migrations/dsl_table_with_decimal_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_table_with_decimal_creation/1_create_some_table.rb @@ -5,6 +5,7 @@ def up create_table :some do |t| t.decimal :money, precision: 16, scale: 4 t.decimal :balance, precision: 32, scale: 2, null: false, default: 0 + t.decimal :paid, precision: 32, scale: 2, null: false, default: 1.15 end end end diff --git a/spec/fixtures/migrations/dsl_table_with_enum_creation/1_create_some_table.rb b/spec/fixtures/migrations/dsl_table_with_enum_creation/1_create_some_table.rb index 0fbbca2f..a39e2e2d 100644 --- a/spec/fixtures/migrations/dsl_table_with_enum_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_table_with_enum_creation/1_create_some_table.rb @@ -3,7 +3,7 @@ class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some, id: false do |t| - t.enum :enum8, value: { key1: 1, key2: 2 }, limit: 1, null: false + t.enum :enum8, value: { key1: 1, key2: 2 }, limit: 1, null: false, default: :key1 t.enum :enum16, value: { key1: 1, key2: 2 }, limit: 2, null: false t.enum :enum_nullable, value: { key1: 1, key2: 2 }, null: true end diff --git a/spec/fixtures/migrations/dsl_table_with_low_cardinality_creation/1_create_some_table.rb b/spec/fixtures/migrations/dsl_table_with_low_cardinality_creation/1_create_some_table.rb index 9c33fecf..10bed2f9 100644 --- a/spec/fixtures/migrations/dsl_table_with_low_cardinality_creation/1_create_some_table.rb +++ b/spec/fixtures/migrations/dsl_table_with_low_cardinality_creation/1_create_some_table.rb @@ -3,7 +3,7 @@ class CreateSomeTable < ActiveRecord::Migration[7.1] def up create_table :some, id: false do |t| - t.string :col1, low_cardinality: true, null: false + t.string :col1, low_cardinality: true, null: false, default: 'col' t.string :col2, low_cardinality: true, null: true t.string :col3, low_cardinality: true, array: true, null: true t.string :col4, low_cardinality: true, map: true, null: true diff --git a/spec/single/migration_spec.rb b/spec/single/migration_spec.rb index 8bab4397..4a950b61 100644 --- a/spec/single/migration_spec.rb +++ b/spec/single/migration_spec.rb @@ -103,13 +103,15 @@ current_schema = schema(model) - expect(current_schema.keys.count).to eq(3) + expect(current_schema.keys.count).to eq(4) expect(current_schema).to have_key('id') expect(current_schema).to have_key('money') expect(current_schema).to have_key('balance') expect(current_schema['id'].sql_type).to eq('UInt32') expect(current_schema['money'].sql_type).to eq('Nullable(Decimal(16, 4))') expect(current_schema['balance'].sql_type).to eq('Decimal(32, 2)') + expect(current_schema['balance'].default).to eq(0.0) + expect(current_schema['paid'].default).to eq(1.15) end end @@ -139,7 +141,11 @@ expect(current_schema).to have_key('datetime') expect(current_schema).to have_key('datetime64') expect(current_schema['datetime'].sql_type).to eq('DateTime') + expect(current_schema['datetime'].default).to be_nil + expect(current_schema['datetime'].default_function).to eq('now()') expect(current_schema['datetime64'].sql_type).to eq('Nullable(DateTime64(3))') + expect(current_schema['datetime64'].default).to be_nil + expect(current_schema['datetime64'].default_function).to eq('now64()') end end @@ -156,6 +162,7 @@ expect(current_schema).to have_key('col3') expect(current_schema).to have_key('col4') expect(current_schema['col1'].sql_type).to eq('LowCardinality(String)') + expect(current_schema['col1'].default).to eq('col') expect(current_schema['col2'].sql_type).to eq('LowCardinality(Nullable(String))') expect(current_schema['col3'].sql_type).to eq('Array(LowCardinality(Nullable(String)))') expect(current_schema['col4'].sql_type).to eq('Map(String, LowCardinality(Nullable(String)))') @@ -191,6 +198,7 @@ expect(current_schema).to have_key('enum16') expect(current_schema).to have_key('enum_nullable') expect(current_schema['enum8'].sql_type).to eq("Enum8('key1' = 1, 'key2' = 2)") + expect(current_schema['enum8'].default).to eq('key1') expect(current_schema['enum16'].sql_type).to eq("Enum16('key1' = 1, 'key2' = 2)") expect(current_schema['enum_nullable'].sql_type).to eq("Nullable(Enum8('key1' = 1, 'key2' = 2))") end From 260881958f8f504406e04b780c297c59fabc76c6 Mon Sep 17 00:00:00 2001 From: nixx Date: Fri, 16 Aug 2024 19:58:08 +0300 Subject: [PATCH 14/22] support activerecord 7.2 --- .github/workflows/testing.yml | 36 ++++++++--- .../connection_adapters/clickhouse/quoting.rb | 19 ++++++ .../clickhouse/schema_statements.rb | 11 +++- .../connection_adapters/clickhouse_adapter.rb | 59 ++++++++++--------- lib/clickhouse-activerecord/version.rb | 2 +- .../active_record/internal_metadata.rb | 8 +++ lib/core_extensions/active_record/relation.rb | 11 ++-- .../active_record/schema_migration.rb | 8 +++ spec/single/migration_spec.rb | 6 +- spec/single/model_spec.rb | 6 +- 10 files changed, 117 insertions(+), 49 deletions(-) create mode 100644 lib/active_record/connection_adapters/clickhouse/quoting.rb diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 6cd96c8f..9c5c7788 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -19,8 +19,16 @@ jobs: fail-fast: true max-parallel: 1 matrix: - ruby-version: [ '2.7', '3.0', '3.2' ] - clickhouse: [ '22.1' ] + version: + - ruby: 2.7 + rails: 7.1.3 + - ruby: 3.0 + rails: 7.1.3 + - ruby: 3.2 + rails: 7.1.3 + - ruby: 3.2 + rails: 7.2.0 + clickhouse: [ '22.1', '24.6' ] steps: - uses: actions/checkout@v4 @@ -33,10 +41,12 @@ jobs: compose-file: '.docker/docker-compose.yml' down-flags: '--volumes' - - name: Set up Ruby ${{ matrix.ruby-version }} + - run: echo 'gem "activerecord", "~> ${{ matrix.version.rails }}"' >> Gemfile + + - name: Set up Ruby ${{ matrix.version.ruby }} uses: ruby/setup-ruby@v1 with: - ruby-version: ${{ matrix.ruby-version }} + ruby-version: ${{ matrix.version.ruby }} bundler-cache: true - run: bundle exec rspec spec/single @@ -54,8 +64,16 @@ jobs: fail-fast: true max-parallel: 1 matrix: - ruby-version: [ '2.7', '3.0', '3.2' ] - clickhouse: [ '22.1' ] + version: + - ruby: 2.7 + rails: 7.1.3 + - ruby: 3.0 + rails: 7.1.3 + - ruby: 3.2 + rails: 7.1.3 + - ruby: 3.2 + rails: 7.2.0 + clickhouse: [ '22.1', '24.6' ] steps: - uses: actions/checkout@v4 @@ -68,10 +86,12 @@ jobs: compose-file: '.docker/docker-compose.cluster.yml' down-flags: '--volumes' - - name: Set up Ruby ${{ matrix.ruby-version }} + - run: echo 'gem "activerecord", "~> ${{ matrix.version.rails }}"' >> Gemfile + + - name: Set up Ruby ${{ matrix.version.ruby }} uses: ruby/setup-ruby@v1 with: - ruby-version: ${{ matrix.ruby-version }} + ruby-version: ${{ matrix.version.ruby }} bundler-cache: true - run: bundle exec rspec spec/cluster diff --git a/lib/active_record/connection_adapters/clickhouse/quoting.rb b/lib/active_record/connection_adapters/clickhouse/quoting.rb new file mode 100644 index 00000000..f2f3e414 --- /dev/null +++ b/lib/active_record/connection_adapters/clickhouse/quoting.rb @@ -0,0 +1,19 @@ +module ActiveRecord + module ConnectionAdapters + module Clickhouse + module Quoting + extend ActiveSupport::Concern + + module ClassMethods # :nodoc: + def quote_column_name(name) + name + end + + def quote_table_name(name) + name + end + end + end + end + end +end diff --git a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb index ae472b6f..7860dc19 100644 --- a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb +++ b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb @@ -18,9 +18,16 @@ def exec_insert(sql, name, _binds, _pk = nil, _sequence_name = nil, returning: n true end - def internal_exec_query(sql, name = nil, binds = [], prepare: false, async: false) + def internal_exec_query(sql, name = nil, binds = [], prepare: false, async: false, allow_retry: false) result = do_execute(sql, name) - ActiveRecord::Result.new(result['meta'].map { |m| m['name'] }, result['data'], result['meta'].map { |m| [m['name'], type_map.lookup(m['type'])] }.to_h) + columns = result['meta'].map { |m| m['name'] } + types = {} + result['meta'].each_with_index do |m, i| + # need use column name and index after commit in 7.2: + # https://github.com/rails/rails/commit/24dbf7637b1d5cd6eb3d7100b8d0f6872c3fee3c + types[m['name']] = types[i] = type_map.lookup(m['type']) + end + ActiveRecord::Result.new(columns, result['data'], types) rescue ActiveRecord::ActiveRecordError => e raise e rescue StandardError => e diff --git a/lib/active_record/connection_adapters/clickhouse_adapter.rb b/lib/active_record/connection_adapters/clickhouse_adapter.rb index 8b41cb2a..3411a421 100644 --- a/lib/active_record/connection_adapters/clickhouse_adapter.rb +++ b/lib/active_record/connection_adapters/clickhouse_adapter.rb @@ -10,6 +10,7 @@ require 'active_record/connection_adapters/clickhouse/oid/big_integer' require 'active_record/connection_adapters/clickhouse/oid/map' require 'active_record/connection_adapters/clickhouse/oid/uuid' +require 'active_record/connection_adapters/clickhouse/quoting' require 'active_record/connection_adapters/clickhouse/schema_definitions' require 'active_record/connection_adapters/clickhouse/schema_creation' require 'active_record/connection_adapters/clickhouse/schema_statements' @@ -23,30 +24,11 @@ class << self def clickhouse_connection(config) config = config.symbolize_keys - if config[:connection] - connection = { - connection: config[:connection] - } - else - port = config[:port] || 8123 - connection = { - host: config[:host] || 'localhost', - port: port, - ssl: config[:ssl].present? ? config[:ssl] : port == 443, - sslca: config[:sslca], - read_timeout: config[:read_timeout], - write_timeout: config[:write_timeout], - keep_alive_timeout: config[:keep_alive_timeout] - } - end - - if config.key?(:database) - database = config[:database] - else + unless config.key?(:database) raise ArgumentError, 'No database specified. Missing argument: database.' end - ConnectionAdapters::ClickhouseAdapter.new(logger, connection, config) + ConnectionAdapters::ClickhouseAdapter.new(config) end end end @@ -83,6 +65,11 @@ def _delete_record(constraints) end module ConnectionAdapters + + if ActiveRecord::version >= Gem::Version.new('7.2') + register "clickhouse", "ActiveRecord::ConnectionAdapters::ClickhouseAdapter", "active_record/connection_adapters/clickhouse_adapter" + end + class ClickhouseColumn < Column private def deduplicated @@ -91,6 +78,8 @@ def deduplicated end class ClickhouseAdapter < AbstractAdapter + include Clickhouse::Quoting + ADAPTER_NAME = 'Clickhouse'.freeze NATIVE_DATABASE_TYPES = { string: { name: 'String' }, @@ -125,12 +114,28 @@ class ClickhouseAdapter < AbstractAdapter include Clickhouse::SchemaStatements # Initializes and connects a Clickhouse adapter. - def initialize(logger, connection_parameters, config) - super(nil, logger) - @connection_parameters = connection_parameters - @connection_config = { user: config[:username], password: config[:password], database: config[:database] }.compact - @debug = config[:debug] || false - @config = config + def initialize(config_or_deprecated_connection, deprecated_logger = nil, deprecated_connection_options = nil, deprecated_config = nil) + super + if @config[:connection] + connection = { + connection: @config[:connection] + } + else + port = @config[:port] || 8123 + connection = { + host: @config[:host] || 'localhost', + port: port, + ssl: @config[:ssl].present? ? @config[:ssl] : port == 443, + sslca: @config[:sslca], + read_timeout: @config[:read_timeout], + write_timeout: @config[:write_timeout], + keep_alive_timeout: @config[:keep_alive_timeout] + } + end + @connection_parameters = connection + + @connection_config = { user: @config[:username], password: @config[:password], database: @config[:database] }.compact + @debug = @config[:debug] || false @prepared_statements = false diff --git a/lib/clickhouse-activerecord/version.rb b/lib/clickhouse-activerecord/version.rb index ad280945..425965b2 100644 --- a/lib/clickhouse-activerecord/version.rb +++ b/lib/clickhouse-activerecord/version.rb @@ -1,3 +1,3 @@ module ClickhouseActiverecord - VERSION = '1.0.13' + VERSION = '1.1.0' end diff --git a/lib/core_extensions/active_record/internal_metadata.rb b/lib/core_extensions/active_record/internal_metadata.rb index 5f59dbfb..2a9a507f 100644 --- a/lib/core_extensions/active_record/internal_metadata.rb +++ b/lib/core_extensions/active_record/internal_metadata.rb @@ -49,6 +49,14 @@ def select_entry(key) connection.select_one(sm, "#{self.class} Load") end + + def connection + if ::ActiveRecord::version >= Gem::Version.new('7.2') + @pool.connection + else + super + end + end end end end diff --git a/lib/core_extensions/active_record/relation.rb b/lib/core_extensions/active_record/relation.rb index 8256bf8e..ea54ef0c 100644 --- a/lib/core_extensions/active_record/relation.rb +++ b/lib/core_extensions/active_record/relation.rb @@ -25,7 +25,6 @@ def settings(**opts) # @param [Hash] opts def settings!(**opts) - assert_mutability! check_command('SETTINGS') @values[:settings] = (@values[:settings] || {}).merge opts self @@ -43,7 +42,6 @@ def final end def final! - assert_mutability! check_command('FINAL') @values[:final] = true self @@ -62,7 +60,6 @@ def using(*opts) # @param [Array] opts def using!(*opts) - assert_mutability! @values[:using] = opts self end @@ -73,8 +70,12 @@ def check_command(cmd) raise ::ActiveRecord::ActiveRecordError, cmd + ' is a ClickHouse specific query clause' unless connection.is_a?(::ActiveRecord::ConnectionAdapters::ClickhouseAdapter) end - def build_arel(aliases = nil) - arel = super + def build_arel(connection_or_aliases = nil, aliases = nil) + if ::ActiveRecord::version >= Gem::Version.new('7.2') + arel = super + else + arel = super(connection_or_aliases) + end arel.final! if @values[:final].present? arel.settings(@values[:settings]) if @values[:settings].present? diff --git a/lib/core_extensions/active_record/schema_migration.rb b/lib/core_extensions/active_record/schema_migration.rb index 96109ef9..ffeab3bc 100644 --- a/lib/core_extensions/active_record/schema_migration.rb +++ b/lib/core_extensions/active_record/schema_migration.rb @@ -47,6 +47,14 @@ def versions connection.select_values(sm, "#{self.class} Load") end + + def connection + if ::ActiveRecord::version >= Gem::Version.new('7.2') + @pool.connection + else + super + end + end end end end diff --git a/spec/single/migration_spec.rb b/spec/single/migration_spec.rb index 4a950b61..38f5e155 100644 --- a/spec/single/migration_spec.rb +++ b/spec/single/migration_spec.rb @@ -9,7 +9,7 @@ end let(:directory) { raise 'NotImplemented' } let(:migrations_dir) { File.join(FIXTURES_PATH, 'migrations', directory) } - let(:migration_context) { ActiveRecord::MigrationContext.new(migrations_dir, model.connection.schema_migration, model.connection.internal_metadata) } + let(:migration_context) { ActiveRecord::MigrationContext.new(migrations_dir) } connection_config = ActiveRecord::Base.connection_db_config.configuration_hash @@ -312,11 +312,11 @@ describe 'drop table sync' do it 'drops table' do migrations_dir = File.join(FIXTURES_PATH, 'migrations', 'dsl_drop_table_sync') - quietly { ActiveRecord::MigrationContext.new(migrations_dir, model.connection.schema_migration).up(1) } + quietly { ActiveRecord::MigrationContext.new(migrations_dir).up(1) } expect(ActiveRecord::Base.connection.tables).to include('some') - quietly { ActiveRecord::MigrationContext.new(migrations_dir, model.connection.schema_migration).up(2) } + quietly { ActiveRecord::MigrationContext.new(migrations_dir).up(2) } expect(ActiveRecord::Base.connection.tables).not_to include('some') end diff --git a/spec/single/model_spec.rb b/spec/single/model_spec.rb index 6365d353..0d16148a 100644 --- a/spec/single/model_spec.rb +++ b/spec/single/model_spec.rb @@ -21,7 +21,7 @@ class ModelPk < ActiveRecord::Base before do migrations_dir = File.join(FIXTURES_PATH, 'migrations', 'add_sample_data') - quietly { ActiveRecord::MigrationContext.new(migrations_dir, Model.connection.schema_migration).up } + quietly { ActiveRecord::MigrationContext.new(migrations_dir).up } end describe '#do_execute' do @@ -277,7 +277,7 @@ class ModelPk < ActiveRecord::Base before do migrations_dir = File.join(FIXTURES_PATH, 'migrations', 'add_array_datetime') - quietly { ActiveRecord::MigrationContext.new(migrations_dir, model.connection.schema_migration).up } + quietly { ActiveRecord::MigrationContext.new(migrations_dir).up } end describe '#create' do @@ -333,7 +333,7 @@ class ModelPk < ActiveRecord::Base before do migrations_dir = File.join(FIXTURES_PATH, 'migrations', 'add_map_datetime') - quietly { ActiveRecord::MigrationContext.new(migrations_dir, model.connection.schema_migration).up } + quietly { ActiveRecord::MigrationContext.new(migrations_dir).up } end describe '#create' do From c16a9c48779729854f41fbe98ceda5fd9fb9ed49 Mon Sep 17 00:00:00 2001 From: nixx Date: Fri, 16 Aug 2024 20:40:49 +0300 Subject: [PATCH 15/22] fix cluster spec --- spec/cluster/migration_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/cluster/migration_spec.rb b/spec/cluster/migration_spec.rb index e8f9455b..eb60f93a 100644 --- a/spec/cluster/migration_spec.rb +++ b/spec/cluster/migration_spec.rb @@ -9,7 +9,7 @@ end let(:directory) { raise 'NotImplemented' } let(:migrations_dir) { File.join(FIXTURES_PATH, 'migrations', directory) } - let(:migration_context) { ActiveRecord::MigrationContext.new(migrations_dir, model.connection.schema_migration, model.connection.internal_metadata) } + let(:migration_context) { ActiveRecord::MigrationContext.new(migrations_dir) } connection_config = ActiveRecord::Base.connection_db_config.configuration_hash From 1454a7902ef2673099f582e3ae3a5e60d90a735f Mon Sep 17 00:00:00 2001 From: nixx Date: Mon, 19 Aug 2024 11:43:28 +0300 Subject: [PATCH 16/22] fix internal metadata for activerecord 7.2 --- clickhouse-activerecord.gemspec | 2 +- .../clickhouse/schema_statements.rb | 10 ++++++++ .../connection_adapters/clickhouse_adapter.rb | 4 +++ lib/clickhouse-activerecord/schema.rb | 8 +++++- lib/clickhouse-activerecord/schema_dumper.rb | 19 -------------- lib/clickhouse-activerecord/version.rb | 2 +- .../active_record/internal_metadata.rb | 25 +++++++++++++------ .../active_record/schema_migration.rb | 2 +- spec/single/migration_spec.rb | 2 +- spec/spec_helper.rb | 2 +- 10 files changed, 43 insertions(+), 33 deletions(-) diff --git a/clickhouse-activerecord.gemspec b/clickhouse-activerecord.gemspec index 57fda949..dea07f4a 100644 --- a/clickhouse-activerecord.gemspec +++ b/clickhouse-activerecord.gemspec @@ -24,7 +24,7 @@ Gem::Specification.new do |spec| spec.require_paths = ['lib'] spec.add_runtime_dependency 'bundler', '>= 1.13.4' - spec.add_runtime_dependency 'activerecord', '>= 7.1' + spec.add_runtime_dependency 'activerecord', '~> 7.1' spec.add_development_dependency 'rake', '~> 13.0' spec.add_development_dependency 'rspec', '~> 3.4' diff --git a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb index 7860dc19..ff531364 100644 --- a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb +++ b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb @@ -116,6 +116,16 @@ def do_execute(sql, name = nil, format: DEFAULT_RESPONSE_FORMAT, settings: {}) end end + if ::ActiveRecord::version >= Gem::Version.new('7.2') + def schema_migration + pool.schema_migration + end + + def migration_context + pool.migration_context + end + end + def assume_migrated_upto_version(version, migrations_paths = nil) version = version.to_i sm_table = quote_table_name(schema_migration.table_name) diff --git a/lib/active_record/connection_adapters/clickhouse_adapter.rb b/lib/active_record/connection_adapters/clickhouse_adapter.rb index 3411a421..55b2c27f 100644 --- a/lib/active_record/connection_adapters/clickhouse_adapter.rb +++ b/lib/active_record/connection_adapters/clickhouse_adapter.rb @@ -532,6 +532,10 @@ def connect @connection end + def reconnect + connect + end + def apply_replica(table, options) if use_replica? && options[:options] if options[:options].match(/^Replicated/) diff --git a/lib/clickhouse-activerecord/schema.rb b/lib/clickhouse-activerecord/schema.rb index a6c7dc98..cd674cac 100644 --- a/lib/clickhouse-activerecord/schema.rb +++ b/lib/clickhouse-activerecord/schema.rb @@ -2,6 +2,12 @@ module ClickhouseActiverecord class Schema < ::ActiveRecord::Schema - + def define(...) + ActiveRecord.deprecator.warn(<<~MSG) + ClickhouseActiverecord::Schema is deprecated + and will be removed in 1.2 version. Use ActiveRecord::Schema instead. + MSG + super + end end end diff --git a/lib/clickhouse-activerecord/schema_dumper.rb b/lib/clickhouse-activerecord/schema_dumper.rb index ef5577f5..4c74c744 100644 --- a/lib/clickhouse-activerecord/schema_dumper.rb +++ b/lib/clickhouse-activerecord/schema_dumper.rb @@ -14,25 +14,6 @@ def dump(connection = ActiveRecord::Base.connection, stream = STDOUT, config = A private - def header(stream) - stream.puts <
= Gem::Version.new('7.2') + return super unless connection.is_a?(::ActiveRecord::ConnectionAdapters::ClickhouseAdapter) + create_entry(connection_or_key, key_or_new_value, new_value) + else + return super(connection_or_key, key_or_new_value) unless connection.is_a?(::ActiveRecord::ConnectionAdapters::ClickhouseAdapter) + create_entry(connection_or_key, key_or_new_value) + end end - def select_entry(key) - return super unless connection.is_a?(::ActiveRecord::ConnectionAdapters::ClickhouseAdapter) + def select_entry(connection_or_key, key = nil) + if ::ActiveRecord::version >= Gem::Version.new('7.2') + return super unless connection.is_a?(::ActiveRecord::ConnectionAdapters::ClickhouseAdapter) + else + key = connection_or_key + return super(key) unless connection.is_a?(::ActiveRecord::ConnectionAdapters::ClickhouseAdapter) + end sm = ::Arel::SelectManager.new(arel_table) sm.final! if connection.table_options(table_name)[:options] =~ /^ReplacingMergeTree/ @@ -52,7 +61,7 @@ def select_entry(key) def connection if ::ActiveRecord::version >= Gem::Version.new('7.2') - @pool.connection + @pool.lease_connection else super end diff --git a/lib/core_extensions/active_record/schema_migration.rb b/lib/core_extensions/active_record/schema_migration.rb index ffeab3bc..bafb6ff1 100644 --- a/lib/core_extensions/active_record/schema_migration.rb +++ b/lib/core_extensions/active_record/schema_migration.rb @@ -50,7 +50,7 @@ def versions def connection if ::ActiveRecord::version >= Gem::Version.new('7.2') - @pool.connection + @pool.lease_connection else super end diff --git a/spec/single/migration_spec.rb b/spec/single/migration_spec.rb index 38f5e155..e56a6c90 100644 --- a/spec/single/migration_spec.rb +++ b/spec/single/migration_spec.rb @@ -282,7 +282,7 @@ expect { ActiveRecord::Base.connection.rebuild_index('some', 'idx3') }.to raise_error(ActiveRecord::ActiveRecordError, include('Unknown index')) - expect { ActiveRecord::Base.connection.rebuild_index('some', 'idx3', true) }.to_not raise_error(ActiveRecord::ActiveRecordError) + # expect { ActiveRecord::Base.connection.rebuild_index('some', 'idx3', if_exists: true) }.to_not raise_error ActiveRecord::Base.connection.rebuild_index('some', 'idx2') end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ccf95d31..38f73d81 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -14,6 +14,7 @@ # Enable flags like --only-failures and --next-failure config.example_status_persistence_file_path = '.rspec_status' config.include ActiveSupport::Testing::Stream + config.raise_errors_for_deprecations! # Disable RSpec exposing methods globally on `Module` and `main` config.disable_monkey_patching! @@ -41,7 +42,6 @@ database: ENV['CLICKHOUSE_DATABASE'] || 'test', username: nil, password: nil, - use_metadata_table: false, cluster_name: ENV['CLICKHOUSE_CLUSTER'], } ) From d7c37af716c59ee51fa2ce5990b4398b8e32de10 Mon Sep 17 00:00:00 2001 From: nixx Date: Fri, 23 Aug 2024 21:15:27 +0300 Subject: [PATCH 17/22] support window named functions --- .../connection_adapters/clickhouse/quoting.rb | 4 ++-- .../clickhouse/schema_statements.rb | 4 ++++ .../connection_adapters/clickhouse_adapter.rb | 2 +- lib/arel/visitors/clickhouse.rb | 8 ++++++++ lib/clickhouse-activerecord/version.rb | 2 +- lib/core_extensions/active_record/relation.rb | 18 ++++++++++++++++++ lib/core_extensions/arel/select_manager.rb | 12 ++++++++++++ spec/single/migration_spec.rb | 2 +- spec/single/model_spec.rb | 12 ++++++++++++ spec/spec_helper.rb | 11 +---------- 10 files changed, 60 insertions(+), 15 deletions(-) diff --git a/lib/active_record/connection_adapters/clickhouse/quoting.rb b/lib/active_record/connection_adapters/clickhouse/quoting.rb index f2f3e414..47c90dfb 100644 --- a/lib/active_record/connection_adapters/clickhouse/quoting.rb +++ b/lib/active_record/connection_adapters/clickhouse/quoting.rb @@ -6,11 +6,11 @@ module Quoting module ClassMethods # :nodoc: def quote_column_name(name) - name + name.to_s.include?('.') ? "`#{name}`" : name.to_s end def quote_table_name(name) - name + name.to_s end end end diff --git a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb index ff531364..6603df05 100644 --- a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb +++ b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb @@ -124,6 +124,10 @@ def schema_migration def migration_context pool.migration_context end + + def internal_metadata + pool.internal_metadata + end end def assume_migrated_upto_version(version, migrations_paths = nil) diff --git a/lib/active_record/connection_adapters/clickhouse_adapter.rb b/lib/active_record/connection_adapters/clickhouse_adapter.rb index 55b2c27f..1a9d8718 100644 --- a/lib/active_record/connection_adapters/clickhouse_adapter.rb +++ b/lib/active_record/connection_adapters/clickhouse_adapter.rb @@ -47,7 +47,7 @@ def is_view module ModelSchema module ClassMethods - delegate :final, :final!, :settings, :settings!, to: :all + delegate :final, :final!, :settings, :settings!, :window, :window!, to: :all def is_view @is_view || false diff --git a/lib/arel/visitors/clickhouse.rb b/lib/arel/visitors/clickhouse.rb index c94a65e0..eced0770 100644 --- a/lib/arel/visitors/clickhouse.rb +++ b/lib/arel/visitors/clickhouse.rb @@ -74,6 +74,14 @@ def visit_Arel_Nodes_DoesNotMatch(o, collector) infix_value o, collector, op end + def visit_Arel_Nodes_Rows(o, collector) + if o.expr.is_a?(String) + collector << "ROWS #{o.expr}" + else + super + end + end + def sanitize_as_setting_value(value) if value == :default 'DEFAULT' diff --git a/lib/clickhouse-activerecord/version.rb b/lib/clickhouse-activerecord/version.rb index 1d7d6a02..7c06c7de 100644 --- a/lib/clickhouse-activerecord/version.rb +++ b/lib/clickhouse-activerecord/version.rb @@ -1,3 +1,3 @@ module ClickhouseActiverecord - VERSION = '1.1.1' + VERSION = '1.1.2' end diff --git a/lib/core_extensions/active_record/relation.rb b/lib/core_extensions/active_record/relation.rb index ea54ef0c..42226fcf 100644 --- a/lib/core_extensions/active_record/relation.rb +++ b/lib/core_extensions/active_record/relation.rb @@ -64,6 +64,23 @@ def using!(*opts) self end + # Windows functions let you perform calculations across a set of rows that are related to the current row. For example: + # + # users = User.window('x', order: 'date', partition: 'name', rows: 'UNBOUNDED PRECEDING').select('sum(value) OVER x') + # # SELECT sum(value) OVER x FROM users WINDOW x AS (PARTITION BY name ORDER BY date ROWS UNBOUNDED PRECEDING) + # + # @param [String] name + # @param [Hash] opts + def window(name, **opts) + spawn.window!(name, **opts) + end + + def window!(name, **opts) + @values[:windows] = [] unless @values[:windows] + @values[:windows] << [name, opts] + self + end + private def check_command(cmd) @@ -80,6 +97,7 @@ def build_arel(connection_or_aliases = nil, aliases = nil) arel.final! if @values[:final].present? arel.settings(@values[:settings]) if @values[:settings].present? arel.using(@values[:using]) if @values[:using].present? + arel.windows(@values[:windows]) if @values[:windows].present? arel end diff --git a/lib/core_extensions/arel/select_manager.rb b/lib/core_extensions/arel/select_manager.rb index a8592a06..5be1bbf1 100644 --- a/lib/core_extensions/arel/select_manager.rb +++ b/lib/core_extensions/arel/select_manager.rb @@ -13,6 +13,18 @@ def settings(values) self end + # @param [Array] windows + def windows(windows) + @ctx.windows = windows.map do |name, opts| + # https://github.com/rails/rails/blob/main/activerecord/test/cases/arel/select_manager_test.rb#L790 + window = ::Arel::Nodes::NamedWindow.new(name) + opts.each do |key, value| + window.send(key, value) + end + window + end + end + def using(*exprs) @ctx.source.right.last.right = ::Arel::Nodes::Using.new(::Arel.sql(exprs.join(','))) self diff --git a/spec/single/migration_spec.rb b/spec/single/migration_spec.rb index e56a6c90..c82fb32c 100644 --- a/spec/single/migration_spec.rb +++ b/spec/single/migration_spec.rb @@ -9,7 +9,7 @@ end let(:directory) { raise 'NotImplemented' } let(:migrations_dir) { File.join(FIXTURES_PATH, 'migrations', directory) } - let(:migration_context) { ActiveRecord::MigrationContext.new(migrations_dir) } + let(:migration_context) { ActiveRecord::MigrationContext.new(migrations_dir, model.connection.schema_migration, model.connection.internal_metadata) } connection_config = ActiveRecord::Base.connection_db_config.configuration_hash diff --git a/spec/single/model_spec.rb b/spec/single/model_spec.rb index 0d16148a..f5d1ada8 100644 --- a/spec/single/model_spec.rb +++ b/spec/single/model_spec.rb @@ -230,6 +230,18 @@ class ModelPk < ActiveRecord::Base end end + describe '#window' do + it 'works' do + sql = Model.window('x', order: 'date', partition: 'name', rows: 'UNBOUNDED PRECEDING').select('sum(event_value) OVER x').to_sql + expect(sql).to eq('SELECT sum(event_value) OVER x FROM sample WINDOW x AS (PARTITION BY name ORDER BY date ROWS UNBOUNDED PRECEDING)') + end + + it 'empty' do + sql = Model.window('x').select('sum(event_value) OVER x').to_sql + expect(sql).to eq('SELECT sum(event_value) OVER x FROM sample WINDOW x AS ()') + end + end + describe 'arel predicates' do describe '#matches' do it 'uses ilike for case insensitive matches' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 38f73d81..50acff46 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -56,16 +56,7 @@ def schema(model) end def clear_db - cluster = ActiveRecord::Base.connection_db_config.configuration_hash[:cluster_name] - pattern = if cluster - normalized_cluster_name = cluster.start_with?('{') ? "'#{cluster}'" : cluster - - "DROP TABLE %s ON CLUSTER #{normalized_cluster_name} SYNC" - else - 'DROP TABLE %s' - end - - ActiveRecord::Base.connection.tables.each { |table| ActiveRecord::Base.connection.execute(pattern % table) } + ActiveRecord::Base.connection.tables.each { |table| ActiveRecord::Base.connection.drop_table(table, sync: true) } rescue ActiveRecord::NoDatabaseError # Ignored end From 205e67d16b448cd1f3388c87afdd4bc042f640ee Mon Sep 17 00:00:00 2001 From: nixx Date: Tue, 27 Aug 2024 15:17:30 +0300 Subject: [PATCH 18/22] fix detect model primary key --- .../connection_adapters/clickhouse_adapter.rb | 7 +-- .../add_sample_data/1_create_sample_table.rb | 8 +-- .../add_sample_data/2_create_join_table.rb | 2 +- .../1_create_sample_table.rb | 17 ++++++ spec/single/model_spec.rb | 60 +++++++++++++------ 5 files changed, 68 insertions(+), 26 deletions(-) create mode 100644 spec/fixtures/migrations/add_sample_data_without_primary_key/1_create_sample_table.rb diff --git a/lib/active_record/connection_adapters/clickhouse_adapter.rb b/lib/active_record/connection_adapters/clickhouse_adapter.rb index 1a9d8718..250f4b42 100644 --- a/lib/active_record/connection_adapters/clickhouse_adapter.rb +++ b/lib/active_record/connection_adapters/clickhouse_adapter.rb @@ -276,10 +276,9 @@ def column_name_for_operation(operation, node) # :nodoc: # SCHEMA STATEMENTS ======================================== - def primary_key(table_name) #:nodoc: - pk = table_structure(table_name).first - return 'id' if pk.present? && pk[0] == 'id' - false + def primary_keys(table_name) + structure = do_system_execute("SHOW COLUMNS FROM `#{table_name}`") + structure['data'].select {|m| m[3]&.include?('PRI') }.pluck(0) end def create_schema_dumper(options) # :nodoc: diff --git a/spec/fixtures/migrations/add_sample_data/1_create_sample_table.rb b/spec/fixtures/migrations/add_sample_data/1_create_sample_table.rb index 0ae71253..7fc41814 100644 --- a/spec/fixtures/migrations/add_sample_data/1_create_sample_table.rb +++ b/spec/fixtures/migrations/add_sample_data/1_create_sample_table.rb @@ -2,16 +2,16 @@ class CreateSampleTable < ActiveRecord::Migration[7.1] def up - create_table :sample, options: 'ReplacingMergeTree PARTITION BY toYYYYMM(date) ORDER BY (event_name)' do |t| + create_table :sample, id: false, options: 'ReplacingMergeTree PARTITION BY toYYYYMM(date) ORDER BY (event_name)' do |t| t.string :event_name, null: false t.integer :event_value t.boolean :enabled, null: false, default: false t.date :date, null: false t.datetime :datetime, null: false - t.datetime :datetime64, precision: 3, null: true - t.string :byte_array, null: true + t.datetime :datetime64, precision: 3 + t.string :byte_array t.uuid :relation_uuid - t.decimal :decimal_value, precision: 38, scale: 16, null: true + t.decimal :decimal_value, precision: 38, scale: 16 end end end diff --git a/spec/fixtures/migrations/add_sample_data/2_create_join_table.rb b/spec/fixtures/migrations/add_sample_data/2_create_join_table.rb index 7436e8b6..17a4bc2b 100644 --- a/spec/fixtures/migrations/add_sample_data/2_create_join_table.rb +++ b/spec/fixtures/migrations/add_sample_data/2_create_join_table.rb @@ -2,7 +2,7 @@ class CreateJoinTable < ActiveRecord::Migration[7.1] def up - create_table :joins, options: 'MergeTree PARTITION BY toYYYYMM(date) ORDER BY (event_name)' do |t| + create_table :joins, id: false, options: 'MergeTree PARTITION BY toYYYYMM(date) ORDER BY (event_name)' do |t| t.string :event_name, null: false t.integer :event_value t.integer :join_value diff --git a/spec/fixtures/migrations/add_sample_data_without_primary_key/1_create_sample_table.rb b/spec/fixtures/migrations/add_sample_data_without_primary_key/1_create_sample_table.rb new file mode 100644 index 00000000..46c5e846 --- /dev/null +++ b/spec/fixtures/migrations/add_sample_data_without_primary_key/1_create_sample_table.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class CreateSampleTable < ActiveRecord::Migration[7.1] + def up + create_table :sample_without_key, id: false, options: 'Log' do |t| + t.string :event_name, null: false + t.integer :event_value + t.boolean :enabled, null: false, default: false + t.date :date, null: false + t.datetime :datetime, null: false + t.datetime :datetime64, precision: 3 + t.string :byte_array + t.uuid :relation_uuid + t.decimal :decimal_value, precision: 38, scale: 16 + end + end +end diff --git a/spec/single/model_spec.rb b/spec/single/model_spec.rb index f5d1ada8..1fb6cf83 100644 --- a/spec/single/model_spec.rb +++ b/spec/single/model_spec.rb @@ -10,10 +10,6 @@ class Model < ActiveRecord::Base self.table_name = 'sample' has_many :joins, class_name: 'ModelJoin', primary_key: 'event_name' end - class ModelPk < ActiveRecord::Base - self.table_name = 'sample' - self.primary_key = 'event_name' - end let(:date) { Date.today } @@ -24,6 +20,10 @@ class ModelPk < ActiveRecord::Base quietly { ActiveRecord::MigrationContext.new(migrations_dir).up } end + it 'detect primary key' do + expect(Model.primary_key).to eq('event_name') + end + describe '#do_execute' do it 'returns formatted result' do result = Model.connection.do_execute('SELECT 1 AS t') @@ -80,7 +80,7 @@ class ModelPk < ActiveRecord::Base it 'update model with primary key' do expect { - ModelPk.first.update!(event_value: 2) + Model.first.update!(event_value: 2) }.to_not raise_error end end @@ -88,12 +88,6 @@ class ModelPk < ActiveRecord::Base describe '#delete' do let!(:record) { Model.create!(event_name: 'some event', date: date) } - it 'model destroy' do - expect { - record.destroy! - }.to raise_error(ActiveRecord::ActiveRecordError, 'Deleting a row is not possible without a primary key') - end - it 'scope' do expect { Model.where(event_name: 'some event').delete_all @@ -102,16 +96,16 @@ class ModelPk < ActiveRecord::Base it 'destroy model with primary key' do expect { - ModelPk.first.destroy! + Model.first.destroy! }.to_not raise_error end end describe '#find_by' do - let!(:record) { Model.create!(id: 1, event_name: 'some event') } + let!(:record) { Model.create!(event_name: 'some event', date: Date.current, datetime: Time.now) } it 'finds the record' do - expect(Model.find_by(id: 1, event_name: 'some event')).to eq(record) + expect(Model.find_by(event_name: 'some event').attributes).to eq(record.attributes) end end @@ -123,7 +117,8 @@ class ModelPk < ActiveRecord::Base it 'select' do Model.create!(event_name: 'some event 1', date: 1.day.ago) Model.create!(event_name: 'some event 2', date: 2.day.ago) - expect(Model.all.reverse_order!.map(&:event_name)).to eq(['some event 1', 'some event 2']) + expect(Model.all.reverse_order!.to_sql).to eq('SELECT sample.* FROM sample ORDER BY sample.event_name DESC') + expect(Model.all.reverse_order!.map(&:event_name)).to eq(['some event 2', 'some event 1']) end end @@ -132,8 +127,8 @@ class ModelPk < ActiveRecord::Base let!(:record2) { Model.create!(event_name: 'some event', event_value: 3, date: date) } it 'integer' do - expect(Model.select(Arel.sql('sum(event_value) AS event_value')).first.event_value.class).to eq(Integer) - expect(Model.select(Arel.sql('sum(event_value) AS value')).first.attributes['value'].class).to eq(Integer) + expect(Model.select(Arel.sql('sum(event_value) AS event_value'))[0].event_value.class).to eq(Integer) + expect(Model.select(Arel.sql('sum(event_value) AS value'))[0].attributes['value'].class).to eq(Integer) expect(Model.pluck(Arel.sql('sum(event_value)')).first[0].class).to eq(Integer) end end @@ -280,6 +275,37 @@ class ModelPk < ActiveRecord::Base end end + context 'sample with id column' do + class ModelWithoutPrimaryKey < ActiveRecord::Base + self.table_name = 'sample_without_key' + end + + before do + migrations_dir = File.join(FIXTURES_PATH, 'migrations', 'add_sample_data_without_primary_key') + quietly { ActiveRecord::MigrationContext.new(migrations_dir).up } + end + + it 'detect primary key' do + expect(ModelWithoutPrimaryKey.primary_key).to eq(nil) + end + + describe '#delete' do + let!(:record) { ModelWithoutPrimaryKey.create!(event_name: 'some event', date: date) } + + it 'model destroy' do + expect { + record.destroy! + }.to raise_error(ActiveRecord::ActiveRecordError, 'Deleting a row is not possible without a primary key') + end + + it 'scope' do + expect { + ModelWithoutPrimaryKey.where(event_name: 'some event').delete_all + }.to_not raise_error + end + end + end + context 'array' do let!(:model) do Class.new(ActiveRecord::Base) do From def2886ff14932fef7c5fe0ea3558fc2ae464654 Mon Sep 17 00:00:00 2001 From: nixx Date: Tue, 27 Aug 2024 15:23:10 +0300 Subject: [PATCH 19/22] fix detect model primary key for older clickhouse versions --- lib/active_record/connection_adapters/clickhouse_adapter.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/active_record/connection_adapters/clickhouse_adapter.rb b/lib/active_record/connection_adapters/clickhouse_adapter.rb index 250f4b42..fd87e88a 100644 --- a/lib/active_record/connection_adapters/clickhouse_adapter.rb +++ b/lib/active_record/connection_adapters/clickhouse_adapter.rb @@ -279,6 +279,10 @@ def column_name_for_operation(operation, node) # :nodoc: def primary_keys(table_name) structure = do_system_execute("SHOW COLUMNS FROM `#{table_name}`") structure['data'].select {|m| m[3]&.include?('PRI') }.pluck(0) + rescue ActiveRecord::ActiveRecordError => e + pk = table_structure(table_name).first + return ['id'] if pk.present? && pk[0] == 'id' + [] end def create_schema_dumper(options) # :nodoc: From 246775c0475cd309e5a03238235cc31b68f6516c Mon Sep 17 00:00:00 2001 From: nixx Date: Tue, 27 Aug 2024 16:25:03 +0300 Subject: [PATCH 20/22] fix spec --- .../connection_adapters/clickhouse_adapter.rb | 13 ++++++-- spec/single/model_spec.rb | 32 +++++++++++++++---- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/lib/active_record/connection_adapters/clickhouse_adapter.rb b/lib/active_record/connection_adapters/clickhouse_adapter.rb index fd87e88a..e2e0becf 100644 --- a/lib/active_record/connection_adapters/clickhouse_adapter.rb +++ b/lib/active_record/connection_adapters/clickhouse_adapter.rb @@ -142,6 +142,11 @@ def initialize(config_or_deprecated_connection, deprecated_logger = nil, depreca connect end + # Return ClickHouse server version + def server_version + @server_version ||= do_system_execute('SELECT version()')['data'][0][0] + end + # Savepoints are not supported, noop def create_savepoint(name) end @@ -277,9 +282,11 @@ def column_name_for_operation(operation, node) # :nodoc: # SCHEMA STATEMENTS ======================================== def primary_keys(table_name) - structure = do_system_execute("SHOW COLUMNS FROM `#{table_name}`") - structure['data'].select {|m| m[3]&.include?('PRI') }.pluck(0) - rescue ActiveRecord::ActiveRecordError => e + if server_version.to_f >= 23.4 + structure = do_system_execute("SHOW COLUMNS FROM `#{table_name}`") + return structure['data'].select {|m| m[3]&.include?('PRI') }.pluck(0) + end + pk = table_structure(table_name).first return ['id'] if pk.present? && pk[0] == 'id' [] diff --git a/spec/single/model_spec.rb b/spec/single/model_spec.rb index 1fb6cf83..6283033e 100644 --- a/spec/single/model_spec.rb +++ b/spec/single/model_spec.rb @@ -10,6 +10,11 @@ class Model < ActiveRecord::Base self.table_name = 'sample' has_many :joins, class_name: 'ModelJoin', primary_key: 'event_name' end + class ModelPk < ActiveRecord::Base + self.table_name = 'sample' + self.primary_key = 'event_name' + end + IS_NEW_CLICKHOUSE_SERVER = Model.connection.server_version.to_f >= 23.4 let(:date) { Date.today } @@ -20,8 +25,10 @@ class Model < ActiveRecord::Base quietly { ActiveRecord::MigrationContext.new(migrations_dir).up } end - it 'detect primary key' do - expect(Model.primary_key).to eq('event_name') + if IS_NEW_CLICKHOUSE_SERVER + it "detect primary key" do + expect(Model.primary_key).to eq('event_name') + end end describe '#do_execute' do @@ -80,7 +87,11 @@ class Model < ActiveRecord::Base it 'update model with primary key' do expect { - Model.first.update!(event_value: 2) + if IS_NEW_CLICKHOUSE_SERVER + Model.first.update!(event_value: 2) + else + ModelPk.first.update!(event_value: 2) + end }.to_not raise_error end end @@ -96,7 +107,11 @@ class Model < ActiveRecord::Base it 'destroy model with primary key' do expect { - Model.first.destroy! + if IS_NEW_CLICKHOUSE_SERVER + Model.first.destroy! + else + ModelPk.first.destroy! + end }.to_not raise_error end end @@ -117,8 +132,13 @@ class Model < ActiveRecord::Base it 'select' do Model.create!(event_name: 'some event 1', date: 1.day.ago) Model.create!(event_name: 'some event 2', date: 2.day.ago) - expect(Model.all.reverse_order!.to_sql).to eq('SELECT sample.* FROM sample ORDER BY sample.event_name DESC') - expect(Model.all.reverse_order!.map(&:event_name)).to eq(['some event 2', 'some event 1']) + if IS_NEW_CLICKHOUSE_SERVER + expect(Model.all.reverse_order!.to_sql).to eq('SELECT sample.* FROM sample ORDER BY sample.event_name DESC') + expect(Model.all.reverse_order!.map(&:event_name)).to eq(['some event 2', 'some event 1']) + else + expect(Model.all.reverse_order!.to_sql).to eq('SELECT sample.* FROM sample ORDER BY sample.date DESC') + expect(Model.all.reverse_order!.map(&:event_name)).to eq(['some event 1', 'some event 2']) + end end end From ddfa259bbec88aefa3eaf2942392b8473a477601 Mon Sep 17 00:00:00 2001 From: nixx Date: Fri, 27 Sep 2024 13:54:22 +0300 Subject: [PATCH 21/22] fix schema dumper --- CHANGELOG.md | 12 ++++++++++++ lib/clickhouse-activerecord/schema_dumper.rb | 13 ++++--------- lib/clickhouse-activerecord/version.rb | 2 +- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cdd42c5..07c1d293 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,15 @@ +### Version 1.1.2 (Aug 27, 2024) +* 🎉 Support for rails 7.2 #156 +* Add method `views` for getting table `View` list in #152 +* Add support for Map datatype in #144 +* Add support window named functions +* Fix schema dumper default values for number +* Normalize table name in schema dump in #148 +* Noop savepoint functionality in #150 +* Fix `#find_by` in #153 +* Add RSpec configure +* Fix detect model primary key + ### Version 1.0.7 (Apr 27, 2024) * Support table indexes diff --git a/lib/clickhouse-activerecord/schema_dumper.rb b/lib/clickhouse-activerecord/schema_dumper.rb index 4c74c744..44a2bd8c 100644 --- a/lib/clickhouse-activerecord/schema_dumper.rb +++ b/lib/clickhouse-activerecord/schema_dumper.rb @@ -54,16 +54,11 @@ def table(table, stream) tbl.print ', materialized: true' if match && match[1].presence end - case pk - when String - tbl.print ", primary_key: #{pk.inspect}" unless pk == "id" - pkcol = columns.detect { |c| c.name == pk } - pkcolspec = column_spec_for_primary_key(pkcol) - if pkcolspec.present? - tbl.print ", #{format_colspec(pkcolspec)}" + if (id = columns.detect { |c| c.name == 'id' }) + spec = column_spec_for_primary_key(id) + if spec.present? + tbl.print ", #{format_colspec(spec)}" end - when Array - tbl.print ", primary_key: #{pk.inspect}" else tbl.print ", id: false" end diff --git a/lib/clickhouse-activerecord/version.rb b/lib/clickhouse-activerecord/version.rb index 7c06c7de..00f5c2e5 100644 --- a/lib/clickhouse-activerecord/version.rb +++ b/lib/clickhouse-activerecord/version.rb @@ -1,3 +1,3 @@ module ClickhouseActiverecord - VERSION = '1.1.2' + VERSION = '1.1.3' end From 9c50598b468ffe1f9c510edc779ecdce08a8e6ba Mon Sep 17 00:00:00 2001 From: Vincent Pochet Date: Tue, 24 Oct 2023 16:20:05 +0200 Subject: [PATCH 22/22] Ignore vscode subfolder --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 63ff4ca8..347bc870 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ /spec/reports/ /tmp/ *.gem +.vscode/ ### JetBrains template # Covers JetBrains IDEs: IntelliJ, RubyMine, PhpStorm, AppCode, PyCharm, CLion, Android Studio and Webstorm