diff --git a/app/services/ci/collect_aggregate_pipeline_analytics_service.rb b/app/services/ci/collect_aggregate_pipeline_analytics_service.rb
index 39e2f2d110c73d48b14d26bb5339dbee02bcf5b8..32db944382087cbc80b8be3c87e0ce3e23f838b9 100644
--- a/app/services/ci/collect_aggregate_pipeline_analytics_service.rb
+++ b/app/services/ci/collect_aggregate_pipeline_analytics_service.rb
@@ -28,7 +28,7 @@ def calculate_aggregate_count(query, result)
return if status_groups.exclude?(:any)
all_query = query.select(query.count_pipelines_function.as('all'))
- result[:any] = ::ClickHouse::Client.select(all_query.to_sql, :main).first['all']
+ result[:any] = ::ClickHouse::Client.select(all_query, :main).first['all']
end
def calculate_aggregate_status_group_counts(query, result)
@@ -39,7 +39,7 @@ def calculate_aggregate_status_group_counts(query, result)
.by_status(selected_statuses)
.group_by_status
- result_by_status = ::ClickHouse::Client.select(query.to_sql, :main).map(&:values).to_h
+ result_by_status = ::ClickHouse::Client.select(query, :main).map(&:values).to_h
result_by_status.each_pair { |status, count| result[STATUS_TO_STATUS_GROUP[status]] += count }
end
@@ -47,7 +47,7 @@ def calculate_aggregate_duration_percentiles(query, result)
return if duration_percentiles.empty?
duration_query = query.select(*duration_percentiles.map { |p| query.duration_quantile_function(p) })
- duration_result = ::ClickHouse::Client.select(duration_query.to_sql, :main)
+ duration_result = ::ClickHouse::Client.select(duration_query, :main)
result[:duration_statistics] = duration_result.first.symbolize_keys.transform_values do |interval|
interval.to_f.round(3).seconds
end
diff --git a/app/services/ci/collect_time_series_pipeline_analytics_service.rb b/app/services/ci/collect_time_series_pipeline_analytics_service.rb
index 3ee25619be1d91679e16d31f689a07a0a6c2aae2..206d56e0ad44792c3d08854a0591790d455e03b8 100644
--- a/app/services/ci/collect_time_series_pipeline_analytics_service.rb
+++ b/app/services/ci/collect_time_series_pipeline_analytics_service.rb
@@ -173,7 +173,7 @@ def group_and_sum_counts(counts_by_status_and_time)
end
def execute_select_query(query)
- ::ClickHouse::Client.select(query.to_sql, :main).map(&:symbolize_keys)
+ ::ClickHouse::Client.select(query, :main).map(&:symbolize_keys)
end
def collect_metrics
diff --git a/gems/click_house-client/lib/click_house/client/query_like.rb b/gems/click_house-client/lib/click_house/client/query_like.rb
index 9e9ee46a3388cb23126630e4201619b84a3d21de..b33c45ee0f80d5a6cb0df643fba99f66be4525c6 100644
--- a/gems/click_house-client/lib/click_house/client/query_like.rb
+++ b/gems/click_house-client/lib/click_house/client/query_like.rb
@@ -14,6 +14,11 @@ def to_sql
def to_redacted_sql(bind_index_manager = BindIndexManager.new)
raise NotImplementedError
end
+
+ # Override when placeholders should be supported
+ def placeholders
+ {}
+ end
end
end
end
diff --git a/gems/click_house-client/spec/click_house/client/query_like_spec.rb b/gems/click_house-client/spec/click_house/client/query_like_spec.rb
index 8b8426bd5fd8f30054f3ba4cbec30cdf17be84cc..b2c99fc81890de3f8b60143ee00afa6d2d165978 100644
--- a/gems/click_house-client/spec/click_house/client/query_like_spec.rb
+++ b/gems/click_house-client/spec/click_house/client/query_like_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe ClickHouse::Client::QueryLike do
+RSpec.describe ClickHouse::Client::QueryLike, feature_category: :database do
subject(:query) { described_class.new }
describe '#to_sql' do
@@ -12,4 +12,8 @@
describe '#to_redacted_sql' do
it { expect { query.to_redacted_sql }.to raise_error(NotImplementedError) }
end
+
+ describe '#placeholders' do
+ it { expect(query.placeholders).to eq({}) }
+ end
end
diff --git a/lib/click_house/models/base_model.rb b/lib/click_house/models/base_model.rb
index ec453bb638bd99d64180ffdf8fc62eab60ed6493..044c434701d51147b36129652c2d60758fe42b64 100644
--- a/lib/click_house/models/base_model.rb
+++ b/lib/click_house/models/base_model.rb
@@ -3,10 +3,10 @@
# rubocop: disable CodeReuse/ActiveRecord
module ClickHouse
module Models
- class BaseModel
+ class BaseModel < ClickHouse::Client::QueryLike
extend Forwardable
- def_delegators :@query_builder, :to_sql
+ def_delegators :@query_builder, :to_sql, :to_redacted_sql
def initialize(query_builder = ClickHouse::QueryBuilder.new(self.class.table_name))
@query_builder = query_builder
diff --git a/lib/click_house/redactor.rb b/lib/click_house/redactor.rb
index 6ca7c46e747a93b7aa22ce0c683378fc78705b4e..32e442d4b6f718cfcb6a95ec6abc05cff039cb25 100644
--- a/lib/click_house/redactor.rb
+++ b/lib/click_house/redactor.rb
@@ -43,10 +43,26 @@ def self.redact_condition(condition, bind_manager)
condition.left.gt(Arel.sql(bind_manager.next_bind_str))
when Arel::Nodes::GreaterThanOrEqual
condition.left.gteq(Arel.sql(bind_manager.next_bind_str))
+ when Arel::Nodes::NamedFunction
+ redact_named_function(condition, bind_manager)
else
raise ArgumentError, "Unsupported Arel node type for Redactor: #{condition.class}"
end
end
+
+ def self.redact_named_function(condition, bind_manager)
+ redacted_condition =
+ Arel::Nodes::NamedFunction.new(condition.name, condition.expressions.dup)
+
+ case redacted_condition.name
+ when 'startsWith'
+ redacted_condition.expressions[1] = Arel.sql(bind_manager.next_bind_str)
+ else
+ redacted_condition.expressions = redacted_condition.expressions.map { Arel.sql(bind_manager.next_bind_str) }
+ end
+
+ redacted_condition
+ end
end
end
# rubocop:enable CodeReuse/ActiveRecord
diff --git a/spec/lib/click_house/models/base_model_spec.rb b/spec/lib/click_house/models/base_model_spec.rb
index fcee40d48e3fa655c7e1efd8a49fba1d44af4b77..e6e22522f80b64cf3076cbf889fff5691b438ddc 100644
--- a/spec/lib/click_house/models/base_model_spec.rb
+++ b/spec/lib/click_house/models/base_model_spec.rb
@@ -15,16 +15,26 @@ def self.table_name
end
end
- describe '#to_sql' do
- it 'delegates to the query builder' do
- expect(query_builder).to receive(:to_sql).and_return("SELECT * FROM dummy_table")
+ it { expect(described_class).to be < ClickHouse::Client::QueryLike }
+
+ shared_examples 'method delegated to query builder' do |method_name|
+ it "delegates ##{method_name} to @query_builder" do
+ expect(query_builder).to receive(method_name).and_return("SELECT * FROM dummy_table")
dummy_instance = dummy_class.new(query_builder)
- expect(dummy_instance.to_sql).to eq("SELECT * FROM dummy_table")
+ expect(dummy_instance.public_send(method_name)).to eq("SELECT * FROM dummy_table")
end
end
+ describe '#to_sql' do
+ it_behaves_like 'method delegated to query builder', :to_sql
+ end
+
+ describe '#to_redacted_sql' do
+ it_behaves_like 'method delegated to query builder', :to_redacted_sql
+ end
+
describe '#where' do
it 'returns a new instance with refined query' do
dummy_instance = dummy_class.new(query_builder)
diff --git a/spec/lib/click_house/redactor_spec.rb b/spec/lib/click_house/redactor_spec.rb
index d8354b6cbb9fad7b905053c94e53b4f022ddf75d..3b16908f24f40caaa0ce3a468b6d4eacbea49bb5 100644
--- a/spec/lib/click_house/redactor_spec.rb
+++ b/spec/lib/click_house/redactor_spec.rb
@@ -107,6 +107,42 @@
expect(redacted_query).to eq(expected_redacted_sql)
end
+
+ context 'when condition is a named function' do
+ let(:function_name) { 'randomFunction' }
+ let(:expressions) { [Arel.sql("'traversal_path'"), Arel.sql('42')] }
+ let(:function_node) do
+ Arel::Nodes::NamedFunction.new(
+ function_name,
+ expressions
+ )
+ end
+
+ subject(:redacted_query) do
+ new_builder = builder.where(function_node)
+ described_class.redact(new_builder)
+ end
+
+ it 'redacts all condition expressions' do
+ expected_redacted_sql = <<~SQL.lines(chomp: true).join(' ')
+ SELECT * FROM "test_table" WHERE randomFunction($1, $2)
+ SQL
+
+ expect(redacted_query).to eq(expected_redacted_sql)
+ end
+
+ context 'when using startsWith function' do
+ let(:function_name) { 'startsWith' }
+
+ it 'redacts only value argument' do
+ expected_redacted_sql = <<~SQL.lines(chomp: true).join(' ')
+ SELECT * FROM "test_table" WHERE startsWith('traversal_path', $1)
+ SQL
+
+ expect(redacted_query).to eq(expected_redacted_sql)
+ end
+ end
+ end
end
context 'with unsupported arel nodes' do
diff --git a/spec/support/shared_examples/click_house/ci/collect_pipeline_analytics_shared_examples.rb b/spec/support/shared_examples/click_house/ci/collect_pipeline_analytics_shared_examples.rb
index 31c6036768e98da3e0de83784401c70e526925a4..580423710d75ff350bddb2692e222a112d4d245f 100644
--- a/spec/support/shared_examples/click_house/ci/collect_pipeline_analytics_shared_examples.rb
+++ b/spec/support/shared_examples/click_house/ci/collect_pipeline_analytics_shared_examples.rb
@@ -1,6 +1,20 @@
# frozen_string_literal: true
RSpec.shared_examples_for 'a pipeline analytics service' do
+ it 'does not execute raw sql queries' do
+ allow(::ClickHouse::Client).to receive(:select).and_call_original
+
+ expect(::ClickHouse::Client)
+ .not_to receive(:select)
+ .with(instance_of(String), instance_of(Symbol))
+
+ expect(::ClickHouse::Client)
+ .to receive(:select)
+ .with(kind_of(ClickHouse::Client::QueryLike), instance_of(Symbol))
+
+ result
+ end
+
context 'when ClickHouse database is not configured' do
before do
allow(::Gitlab::ClickHouse).to receive(:configured?).and_return(false)