From ddadfbb661c257720e0119ca21da69bf3481287f Mon Sep 17 00:00:00 2001 From: jdenquin Date: Thu, 10 Oct 2024 17:49:59 +0200 Subject: [PATCH] fix specs and improve store --- app/models/clickhouse/events_enriched.rb | 8 +- app/models/clickhouse/events_raw.rb | 10 +- .../events/stores/clickhouse_store.rb | 110 +++++++++++------- .../factories/clickhouse/events_count_aggs.rb | 24 ---- spec/factories/clickhouse/events_enriched.rb | 10 +- spec/factories/clickhouse/events_max_aggs.rb | 24 ---- spec/factories/clickhouse/events_sum_aggs.rb | 24 ---- .../events/stores/clickhouse_store_spec.rb | 16 ++- 8 files changed, 92 insertions(+), 134 deletions(-) delete mode 100644 spec/factories/clickhouse/events_count_aggs.rb delete mode 100644 spec/factories/clickhouse/events_max_aggs.rb delete mode 100644 spec/factories/clickhouse/events_sum_aggs.rb diff --git a/app/models/clickhouse/events_enriched.rb b/app/models/clickhouse/events_enriched.rb index d9d0939984e..e81e6bd40da 100644 --- a/app/models/clickhouse/events_enriched.rb +++ b/app/models/clickhouse/events_enriched.rb @@ -10,15 +10,15 @@ class EventsEnriched < BaseRecord # # Table name: events_enriched # -# code :string not null +# code :string not null, primary key # decimal_value :decimal(26, ) # enriched_at :datetime not null # precise_total_amount_cents :decimal(40, 15) # properties :string not null # sorted_properties :string not null -# timestamp :datetime not null +# timestamp :datetime not null, primary key # value :string -# external_subscription_id :string not null -# organization_id :string not null +# external_subscription_id :string not null, primary key +# organization_id :string not null, primary key # transaction_id :string not null # diff --git a/app/models/clickhouse/events_raw.rb b/app/models/clickhouse/events_raw.rb index 6243bb0a2b5..e0c9753b174 100644 --- a/app/models/clickhouse/events_raw.rb +++ b/app/models/clickhouse/events_raw.rb @@ -10,12 +10,12 @@ class EventsRaw < BaseRecord # # Table name: events_raw # -# code :string not null +# code :string not null, primary key # precise_total_amount_cents :decimal(40, 15) # properties :string not null -# timestamp :datetime not null +# timestamp :datetime not null, primary key # external_customer_id :string not null -# external_subscription_id :string not null -# organization_id :string not null -# transaction_id :string not null +# external_subscription_id :string not null, primary key +# organization_id :string not null, primary key +# transaction_id :string not null, primary key # diff --git a/app/services/events/stores/clickhouse_store.rb b/app/services/events/stores/clickhouse_store.rb index e076e9063ca..3c8d4aaea93 100644 --- a/app/services/events/stores/clickhouse_store.rb +++ b/app/services/events/stores/clickhouse_store.rb @@ -14,7 +14,7 @@ def events(force_from: false, ordered: false) scope = scope.where("events_enriched.timestamp >= ?", from_datetime) if force_from || use_from_boundary scope = scope.where("events_enriched.timestamp <= ?", to_datetime) if to_datetime - scope = scope.limit_by(1, :transaction_id) + scope = scope.limit_by(1, 'events_enriched.transaction_id') scope = with_grouped_by_values(scope) if grouped_by_values? filters_scope(scope) @@ -45,13 +45,13 @@ def grouped_last_event .to_sql sql = <<-SQL - with events as (#{cte_sql}) + WITH events AS (#{cte_sql}) - select + SELECT DISTINCT ON (#{group_names}) #{group_names}, events.timestamp, property - from events + FROM events ORDER BY #{group_names}, events.timestamp DESC SQL @@ -65,7 +65,14 @@ def prorated_events_values(total_duration) end def count - events.count.to_i + sql = <<-SQL + WITH events AS (#{events.to_sql}) + + SELECT count() + FROM events + SQL + + ::Clickhouse::EventsEnriched.connection.select_value(sql).to_i end def grouped_count @@ -78,13 +85,13 @@ def grouped_count .select((groups + ["events_enriched.transaction_id"]).join(", ")) sql = <<-SQL - with events as (#{cte_sql.to_sql}) + WITH events AS (#{cte_sql.to_sql}) - select + SELECT #{group_names.join(", ")}, toDecimal128(count(), #{DECIMAL_SCALE}) - from events - group by #{group_names.join(",")} + FROM events + GROUP BY #{group_names.join(",")} SQL prepare_grouped_result(::Clickhouse::EventsEnriched.connection.select_all(sql).rows) @@ -198,7 +205,14 @@ def grouped_prorated_unique_count end def max - events.maximum("events_enriched.decimal_value") + sql = <<-SQL + WITH events AS (#{events.to_sql}) + + SELECT max(events.decimal_value) + FROM events + SQL + + ::Clickhouse::EventsEnriched.connection.select_value(sql) end def grouped_max @@ -213,13 +227,13 @@ def grouped_max .to_sql sql = <<-SQL - with events as (#{cte_sql}) + WITH events AS (#{cte_sql}) - select + SELECT #{group_names}, MAX(property) - from events - group by #{group_names} + FROM events + GROUP BY #{group_names} SQL prepare_grouped_result(::Clickhouse::EventsEnriched.connection.select_all(sql).rows) @@ -244,12 +258,12 @@ def grouped_last .to_sql sql = <<-SQL - with events as (#{cte_sql}) + WITH events AS (#{cte_sql}) - select + SELECT DISTINCT ON (#{group_names}) #{group_names}, property - from events + FROM events ORDER BY #{group_names}, events.timestamp DESC SQL @@ -257,7 +271,14 @@ def grouped_last end def sum_precise_total_amount_cents - events.sum("events_enriched.precise_total_amount_cents") + sql = <<-SQL + WITH events AS (#{events.to_sql}) + + SELECT SUM(events.precise_total_amount_cents) + FROM events + SQL + + ::Clickhouse::EventsEnriched.connection.select_value(sql) end def grouped_sum_precise_total_amount_cents @@ -270,20 +291,27 @@ def grouped_sum_precise_total_amount_cents .select((groups + [Arel.sql("precise_total_amount_cents as property")]).join(", ")) sql = <<-SQL - with events as (#{cte_sql.to_sql}) + WITH events AS (#{cte_sql.to_sql}) - select + SELECT #{group_names}, sum(events.property) - from events - group by #{group_names} + FROM events + GROUP BY #{group_names} SQL prepare_grouped_result(::Clickhouse::EventsEnriched.connection.select_all(sql).rows) end def sum - events.sum("events_enriched.decimal_value") + sql = <<-SQL + WITH events AS (#{events.to_sql}) + + SELECT sum(events.decimal_value) + FROM events + SQL + + ::Clickhouse::EventsEnriched.connection.select_value(sql) end def grouped_sum @@ -296,13 +324,13 @@ def grouped_sum .select((groups + [Arel.sql("events_enriched.decimal_value AS property")]).join(", ")) sql = <<-SQL - with events as (#{cte_sql.to_sql}) + WITH events AS (#{cte_sql.to_sql}) - select + SELECT #{group_names}, sum(events.property) - from events - group by #{group_names} + FROM events + GROUP BY #{group_names} SQL prepare_grouped_result(::Clickhouse::EventsEnriched.connection.select_all(sql).rows) @@ -320,10 +348,10 @@ def prorated_sum(period_duration:, persisted_duration: nil) .to_sql sql = <<-SQL - with events as (#{cte_sql}) + WITH events AS (#{cte_sql}) - select sum(events.prorated_value) - from events + SELECT sum(events.prorated_value) + FROM events SQL ::Clickhouse::EventsEnriched.connection.select_value(sql) @@ -346,13 +374,13 @@ def grouped_prorated_sum(period_duration:, persisted_duration: nil) .to_sql sql = <<-SQL - with events as (#{cte_sql}) + WITH events AS (#{cte_sql}) - select + SELECT #{group_names}, sum(events.prorated_value) - from events - group by #{group_names} + FROM events + GROUP BY #{group_names} SQL prepare_grouped_result(::Clickhouse::EventsEnriched.connection.select_all(sql).rows) @@ -362,18 +390,18 @@ def sum_date_breakdown date_field = date_in_customer_timezone_sql("events_enriched.timestamp") cte_sql = events - .select("toDate(#{date_field}) as day, events_enriched.decimal_value as property") + .select("toDate(#{date_field}) as day, events_enriched.decimal_value AS property") .to_sql sql = <<-SQL - with events as (#{cte_sql}) + WITH events AS (#{cte_sql}) - select + SELECT events.day, - sum(events.property) as day_sum - from events - group by events.day - order by events.day asc + sum(events.property) AS day_sum + FROM events + GROUP BY events.day + ORDER BY events.day asc SQL ::Clickhouse::EventsEnriched.connection.select_all(Arel.sql(sql)).rows.map do |row| diff --git a/spec/factories/clickhouse/events_count_aggs.rb b/spec/factories/clickhouse/events_count_aggs.rb deleted file mode 100644 index dad28553a78..00000000000 --- a/spec/factories/clickhouse/events_count_aggs.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -FactoryBot.define do - factory :clickhouse_events_count_agg, class: 'Clickhouse::EventsCountAgg' do - transient do - subscription { create(:subscription, customer:) } - customer { create(:customer) } - organization { customer.organization } - billable_metric { create(:billable_metric, organization:) } - plan { create(:plan, organization:) } - charge { create(:standard_charge, billable_metric:, plan:) } - end - - organization_id { organization.id } - external_subscription_id { subscription.external_id } - code { billable_metric.code } - timestamp { Time.current.beginning_of_hour } - value { 1.0 } - charge_id { charge.id } - - filters { {} } - grouped_by { {} } - end -end diff --git a/spec/factories/clickhouse/events_enriched.rb b/spec/factories/clickhouse/events_enriched.rb index 5aeb83c683a..3f462fd53bc 100644 --- a/spec/factories/clickhouse/events_enriched.rb +++ b/spec/factories/clickhouse/events_enriched.rb @@ -6,9 +6,6 @@ subscription { create(:subscription, customer:) } customer { create(:customer) } organization { customer.organization } - billable_metric { create(:billable_metric, organization:) } - plan { create(:plan, organization:) } - charge { create(:standard_charge, billable_metric:, plan:) } end organization_id { organization.id } @@ -17,10 +14,7 @@ timestamp { Time.current } transaction_id { "tr_#{SecureRandom.hex}" } properties { {} } - value { 21.0 } - charge_id { charge.id } - aggregation_type { billable_metric.aggregation_type } - filters { {} } - grouped_by { {} } + value { "21.0" } + decimal_value { 21.0 } end end diff --git a/spec/factories/clickhouse/events_max_aggs.rb b/spec/factories/clickhouse/events_max_aggs.rb deleted file mode 100644 index 3100662226f..00000000000 --- a/spec/factories/clickhouse/events_max_aggs.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -FactoryBot.define do - factory :clickhouse_events_max_agg, class: 'Clickhouse::EventsMaxAgg' do - transient do - subscription { create(:subscription, customer:) } - customer { create(:customer) } - organization { customer.organization } - billable_metric { create(:billable_metric, organization:) } - plan { create(:plan, organization:) } - charge { create(:standard_charge, billable_metric:, plan:) } - end - - organization_id { organization.id } - external_subscription_id { subscription.external_id } - code { billable_metric.code } - timestamp { Time.current.beginning_of_hour } - value { 4.0 } - charge_id { charge.id } - - filters { {} } - grouped_by { {} } - end -end diff --git a/spec/factories/clickhouse/events_sum_aggs.rb b/spec/factories/clickhouse/events_sum_aggs.rb deleted file mode 100644 index 228af7e0e6d..00000000000 --- a/spec/factories/clickhouse/events_sum_aggs.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -FactoryBot.define do - factory :clickhouse_events_sum_agg, class: 'Clickhouse::EventsSumAgg' do - transient do - subscription { create(:subscription, customer:) } - customer { create(:customer) } - organization { customer.organization } - billable_metric { create(:billable_metric, organization:) } - plan { create(:plan, organization:) } - charge { create(:standard_charge, billable_metric:, plan:) } - end - - organization_id { organization.id } - external_subscription_id { subscription.external_id } - code { billable_metric.code } - timestamp { Time.current.beginning_of_hour } - value { 12.0 } - charge_id { charge.id } - - filters { {} } - grouped_by { {} } - end -end diff --git a/spec/services/events/stores/clickhouse_store_spec.rb b/spec/services/events/stores/clickhouse_store_spec.rb index 445e8f5130b..72c3ee159ce 100644 --- a/spec/services/events/stores/clickhouse_store_spec.rb +++ b/spec/services/events/stores/clickhouse_store_spec.rb @@ -93,21 +93,21 @@ describe '.events' do it 'returns a list of events' do - expect(event_store.events.count).to eq(5) + expect(event_store.count).to eq(5) end context 'with grouped_by_values' do let(:grouped_by_values) { {'region' => 'europe'} } it 'returns a list of events' do - expect(event_store.events.count).to eq(3) + expect(event_store.count).to eq(3) end context 'when grouped_by_values value is nil' do let(:grouped_by_values) { {'region' => nil} } it 'returns a list of events' do - expect(event_store.events.count).to eq(5) + expect(event_store.count).to eq(5) end end end @@ -117,7 +117,7 @@ let(:ignored_filters) { [{'city' => ['paris']}, {'city' => ['londons'], 'country' => ['united kingdom']}] } it 'returns a list of events' do - expect(event_store.events.count).to eq(2) # 1st event is ignored + expect(event_store.count).to eq(2) # 1st event is ignored end end end @@ -368,6 +368,7 @@ organization_id: organization.id, external_subscription_id: subscription.external_id, code:, + transaction_id: SecureRandom.uuid, timestamp: boundaries[:from_datetime] + 1.hour, properties: { billable_metric.field_name => 2, @@ -380,6 +381,7 @@ organization_id: organization.id, external_subscription_id: subscription.external_id, code:, + transaction_id: SecureRandom.uuid, timestamp: boundaries[:from_datetime] + 1.day, properties: { billable_metric.field_name => 2, @@ -392,6 +394,7 @@ organization_id: organization.id, external_subscription_id: subscription.external_id, code:, + transaction_id: SecureRandom.uuid, timestamp: boundaries[:from_datetime] + 2.days, properties: { billable_metric.field_name => 2, @@ -405,6 +408,7 @@ organization_id: organization.id, external_subscription_id: subscription.external_id, code:, + transaction_id: SecureRandom.uuid, timestamp: boundaries[:from_datetime] + 2.days, properties: {billable_metric.field_name => 2}, value: "2", @@ -449,6 +453,7 @@ organization_id: organization.id, external_subscription_id: subscription.external_id, code:, + transaction_id: SecureRandom.uuid, timestamp: boundaries[:from_datetime] + 1.day, properties: { billable_metric.field_name => 2, @@ -461,6 +466,7 @@ organization_id: organization.id, external_subscription_id: subscription.external_id, code:, + transaction_id: SecureRandom.uuid, timestamp: boundaries[:from_datetime] + 1.day, properties: { billable_metric.field_name => 2, @@ -473,6 +479,7 @@ organization_id: organization.id, external_subscription_id: subscription.external_id, code:, + transaction_id: SecureRandom.uuid, timestamp: (boundaries[:from_datetime] + 1.day).end_of_day, properties: { billable_metric.field_name => 2, @@ -486,6 +493,7 @@ organization_id: organization.id, external_subscription_id: subscription.external_id, code:, + transaction_id: SecureRandom.uuid, timestamp: boundaries[:from_datetime] + 2.days, properties: {billable_metric.field_name => 2}, value: "2",