From 205e67d16b448cd1f3388c87afdd4bc042f640ee Mon Sep 17 00:00:00 2001 From: nixx Date: Tue, 27 Aug 2024 15:17:30 +0300 Subject: [PATCH] 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