From 7fb1a7f12d64ac6c038e19729b4de147f2d96c78 Mon Sep 17 00:00:00 2001 From: Ryan Kerr Date: Thu, 21 Nov 2024 19:29:25 -0500 Subject: [PATCH] Integrate updates into rebase. * Restore support for table/database creation with request settings. * Move improved "DB::Exception" handling into ResponseProcessor. --- .../clickhouse/schema_statements.rb | 5 +++-- .../statement/response_processor.rb | 22 ++++++++++--------- .../connection_adapters/clickhouse_adapter.rb | 5 ++--- spec/cluster/migration_spec.rb | 2 +- .../1_create_some_function.rb | 2 +- spec/single/migration_spec.rb | 2 +- 6 files changed, 20 insertions(+), 18 deletions(-) diff --git a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb index f8d140c5..229e8568 100644 --- a/lib/active_record/connection_adapters/clickhouse/schema_statements.rb +++ b/lib/active_record/connection_adapters/clickhouse/schema_statements.rb @@ -6,7 +6,6 @@ module ActiveRecord module ConnectionAdapters module Clickhouse module SchemaStatements - DB_EXCEPTION_REGEXP = /\ACode:\s+\d+\.\s+DB::Exception:/.freeze def with_settings(**settings) @block_settings ||= {} @@ -129,7 +128,9 @@ def functions end def show_create_function(function) - execute("SELECT create_query FROM system.functions WHERE origin = 'SQLUserDefined' AND name = '#{function}'") + result = do_system_execute("SELECT create_query FROM system.functions WHERE origin = 'SQLUserDefined' AND name = '#{function}'") + return if result.nil? + result['data'].flatten.first end def table_options(table) diff --git a/lib/active_record/connection_adapters/clickhouse/statement/response_processor.rb b/lib/active_record/connection_adapters/clickhouse/statement/response_processor.rb index 89ea4a0c..5288965e 100644 --- a/lib/active_record/connection_adapters/clickhouse/statement/response_processor.rb +++ b/lib/active_record/connection_adapters/clickhouse/statement/response_processor.rb @@ -6,8 +6,11 @@ module Clickhouse class Statement class ResponseProcessor + DB_EXCEPTION_REGEXP = /\ACode:\s+\d+\.\s+DB::Exception:/.freeze + def initialize(raw_response, format) @raw_response = raw_response + @body = raw_response.body @format = format end @@ -18,7 +21,7 @@ def process raise_database_error! end rescue JSON::ParserError - @raw_response.body + @body end private @@ -28,29 +31,28 @@ def success? end def process_successful_response - raise_generic! if @raw_response.body.to_s.include?('DB::Exception') + raise_generic! if @body.include?('DB::Exception') && @body.match?(DB_EXCEPTION_REGEXP) format_body_response end def raise_generic! - raise ActiveRecord::ActiveRecordError, "Response code: #{@raw_response.code}:\n#{@raw_response.body}" + raise ActiveRecord::ActiveRecordError, "Response code: #{@raw_response.code}:\n#{@body}" end def format_body_response - body = @raw_response.body - return body if body.blank? + return @body if @body.blank? case @format when 'JSONCompact' - format_from_json_compact(body) + format_from_json_compact(@body) when 'JSONCompactEachRowWithNamesAndTypes' - format_from_json_compact_each_row_with_names_and_types(body) + format_from_json_compact_each_row_with_names_and_types(@body) else - body + @body end rescue JSON::ParserError - @raw_response.body + @body end def format_from_json_compact(body) @@ -79,7 +81,7 @@ def parse_json_payload(payload) end def raise_database_error! - case @raw_response.body + case @body when /DB::Exception:.*\(UNKNOWN_DATABASE\)/ raise ActiveRecord::NoDatabaseError when /DB::Exception:.*\(DATABASE_ALREADY_EXISTS\)/ diff --git a/lib/active_record/connection_adapters/clickhouse_adapter.rb b/lib/active_record/connection_adapters/clickhouse_adapter.rb index 4a12a88f..144a1881 100644 --- a/lib/active_record/connection_adapters/clickhouse_adapter.rb +++ b/lib/active_record/connection_adapters/clickhouse_adapter.rb @@ -13,7 +13,6 @@ require 'active_record/connection_adapters/clickhouse/oid/map' require 'active_record/connection_adapters/clickhouse/oid/uuid' require 'active_record/connection_adapters/clickhouse/column' -require 'active_record/connection_adapters/clickhouse/format_manager' require 'active_record/connection_adapters/clickhouse/quoting' require 'active_record/connection_adapters/clickhouse/schema_creation' require 'active_record/connection_adapters/clickhouse/schema_statements' @@ -326,7 +325,7 @@ def create_view(table_name, request_settings: {}, **options) drop_table(table_name, options.merge(if_exists: true)) end - execute(schema_creation.accept(td)) + execute(schema_creation.accept(td), settings: request_settings) end def create_table(table_name, request_settings: {}, **options, &block) @@ -343,7 +342,7 @@ def create_table(table_name, request_settings: {}, **options, &block) drop_table(table_name, options.merge(if_exists: true)) end - execute(schema_creation.accept(td)) + execute(schema_creation.accept(td), settings: request_settings) if options[:with_distributed] distributed_table_name = options.delete(:with_distributed) diff --git a/spec/cluster/migration_spec.rb b/spec/cluster/migration_spec.rb index c1c72b6d..76cb2cf2 100644 --- a/spec/cluster/migration_spec.rb +++ b/spec/cluster/migration_spec.rb @@ -70,7 +70,7 @@ let(:directory) { 'dsl_create_function' } it 'creates a function' do - ActiveRecord::Base.connection.do_execute('CREATE FUNCTION forced_fun AS (x, k, b) -> k*x + b', format: nil) + ActiveRecord::Base.connection.execute('CREATE FUNCTION forced_fun AS (x, k, b) -> k*x + b') subject 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 bd5c2686..e5002738 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 @@ -5,7 +5,7 @@ def up sql = <<~SQL CREATE FUNCTION multFun AS (x,y) -> x * y SQL - do_execute(sql, format: nil) + execute(sql) sql = <<~SQL CREATE FUNCTION addFun AS (x,y) -> x + y diff --git a/spec/single/migration_spec.rb b/spec/single/migration_spec.rb index 9b1c574f..aca41e2b 100644 --- a/spec/single/migration_spec.rb +++ b/spec/single/migration_spec.rb @@ -388,7 +388,7 @@ context 'dsl' do let(:directory) { 'dsl_create_function' } it 'creates a function' do - ActiveRecord::Base.connection.do_execute('CREATE FUNCTION forced_fun AS (x, k, b) -> k*x + b', format: nil) + ActiveRecord::Base.connection.execute('CREATE FUNCTION forced_fun AS (x, k, b) -> k*x + b') subject