Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add date_trunc function to mysql #17

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ ActiveReporting::Configuration.setting = value

`metric_lookup_class` - The name of a constant used to lookup prebuilt `Reporting::Metric` objects by name. The constant should define a class method called `#lookup` which can take a string or symbol of the metric name. (Default: `::Metric`)

`db_adapter` - Each database has a diferent way of doing the same thing on SQL. This is an abstraction layer on each database. For example `PostgreSQL` have a native `date_trunc` while `MySQL` doesn't so we do the same in another way. Make sure to choose your database's adapter (Default: `:sqlite3`). If you want MySQL put `:mysql2` and if you want PostGreSQL put `:postgresql`.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a note here explaining that while it is encouraged to set this value, it is not required if the user does not with to implement database-specific features such as the date functions.



## ActiveReporting::FactModel

Expand Down Expand Up @@ -219,7 +221,7 @@ class PhoneFactModel < ActiveReporting::FactModel
end
```

### Implicit hierarchies with datetime columns (PostgreSQL support only)
### Implicit hierarchies with datetime columns (PostgreSQL and MySQL support only)

The fastest approach to group by certain date metrics is to create so-called "date dimensions". For
those Postgres users that are restricted from organizing their data in this way, Postgres provides
Expand All @@ -235,7 +237,6 @@ end

When creating a metric, ActiveReporting will recognize implicit hierarchies for this dimension. The hierarchies correspond to the [values](https://www.postgresql.org/docs/8.1/static/functions-datetime.html#FUNCTIONS-DATETIME-TRUNC) supported by PostgreSQL. (See example under the metric section, below.)

*NOTE*: PRs welcomed to support this functionality in other databases.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess PRs are still welcome to support other databases 😉

Copy link
Contributor Author

@andresgutgon andresgutgon Oct 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL, true but the rest of databases are SQLitle which I guess nobody use in production. I doubt someone will do that


## Configuring Dimension Filters

Expand Down
5 changes: 5 additions & 0 deletions lib/active_reporting.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
# frozen_string_literal: true

require 'active_record'
require 'active_reporting/database_adapters/base'
require 'active_reporting/database_adapters/sqlite_adapter'
require 'active_reporting/database_adapters/mysql_adapter'
require 'active_reporting/database_adapters/postgresql_adapter'
require 'active_reporting/database_adapters/factory'
require 'active_reporting/active_record_adaptor'
require 'active_reporting/configuration'
require 'active_reporting/dimension'
Expand Down
17 changes: 17 additions & 0 deletions lib/active_reporting/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,22 @@ def self.config
yield self
end

# Set database adapter. There are some SQL functions that are only
# available in some databases. You need to specify adapter
#
# Default db_adapter: SqliteAdapter
def self.db_adapter
@db_adapter ||= DatabaseAdapters::Factory.for_database(:sqlite3)
end

# Set database adapter. There are some SQL functions that are only
# available in some databases. You need to specify adapter
#
# @param [Symbol] adapter_name
def self.db_adapter=(adapter_name)
@db_adapter = DatabaseAdapters::Factory.for_database(adapter_name)
end

# The default label used by all dimensions if not set otherwise
#
# Default value is `:name`
Expand Down Expand Up @@ -57,6 +73,7 @@ def self.ransack_fallback
# @return [Boolean]
def self.ransack_fallback=(fallback)
raise RansackNotAvailable unless ransack_available

@ransack_fallback = fallback
end

Expand Down
33 changes: 33 additions & 0 deletions lib/active_reporting/database_adapters/base.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# frozen_string_literal: true

module ActiveReporting
module DatabaseAdapters
DatabaseNotSupported = Class.new(StandardError)
MethodNotImplemented = Class.new(StandardError)
# Database adapters are here to solve SQL problems in the
# most idiomatic way in each database
class Base
attr_reader :name

def initialize(name)
@name = name
end

# @param [Symbol] interval
# @return [Boolean]
def allowed_datetime_hierarchy?(interval)
datetime_hierarchies.include?(interval.try(:to_sym))
end

protected

# Allowed datetime hierchies in each adapter
# By default (:sqlite) there is none
#
# @return [Array<Symbol>]
def datetime_hierarchies
@datetime_hierarchies ||= []
end
end
end
end
27 changes: 27 additions & 0 deletions lib/active_reporting/database_adapters/factory.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# frozen_string_literal: true

module ActiveReporting
module DatabaseAdapters
class Factory
class << self
ADAPTERS = {
sqlite3: SqliteAdapter,
mysql2: MysqlAdapter,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go with just sqlite and mysql here. The use of "3" and "2" in various Rails projects references the adaptor and version of the gem they use. In the case of this gem, we shouldn't care.

postgresql: PostgresqlAdapter,
postgis: PostgresqlAdapter
}.freeze
# @param [Symbol]
def for_database(adapter_name)
adapter = ADAPTERS[adapter_name]

return adapter.new(adapter_name) unless adapter.nil?

raise(
DatabaseNotSupported,
"Database with this #{adapter_name} is not supported by ActiveReporting"
)
end
end
end
end
end
56 changes: 56 additions & 0 deletions lib/active_reporting/database_adapters/mysql_adapter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# frozen_string_literal: true

module ActiveReporting
module DatabaseAdapters
# Database adapters are here to solve SQL problems in the
# most idiomatic way in each database
class MysqlAdapter < Base
# Generate SQL snippet with DATE_TRUNC
# @param [String] interval
# @param [String] field
# @return [String]
def date_trunc(interval, field)
clean_sql(
<<-SQL
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "squiggly heredoc" has been in Ruby since 2.3, I recommend using that instead.
https://ruby-doc.org/core-2.3.0/doc/syntax/literals_rdoc.html#label-Here+Documents

Also, I would say I personally am not concerned by multi-line SQL being generated, so I don't think #clean_sql is really necessary.

DATE_ADD(
"#{super_old_base_date}",
INTERVAL TIMESTAMPDIFF(
#{interval.upcase},
"#{super_old_base_date}",
#{field}
) #{interval.upcase}
)
SQL
)
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been playing around with different variants of replicating date_trunc, including this one. This approach is about 2x faster according to my primitive metrics, but it falls short in truly replicating date_trunc, mostly patently shown by the limit on supported datetime_hierarchies.

Take a look at this code and consider if it would be useful. It is ugly but may provide a better parallel in functionality.


protected

# Remove spaces and put all in one line
def clean_sql(sql)
sql
.strip
.gsub(/\n+/, ' ')
.gsub(/\s+/, ' ')
.gsub(/\(\s+\)/, '(')
.gsub(/\)\s+\)/, ')')
end

def datetime_hierarchies
@datetime_hierarchies ||= %i[
year
month
week
]
end

# Used to generate a diff when implementing
# datetime truncation
#
# @return [String]
def super_old_base_date
'1900-01-01'
end
end
end
end
38 changes: 38 additions & 0 deletions lib/active_reporting/database_adapters/postgresql_adapter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# frozen_string_literal: true

module ActiveReporting
module DatabaseAdapters
class PostgresqlAdapter < Base
# Values for the Postgres `date_trunc` method.
# See https://www.postgresql.org/docs/10/static/functions-datetime.html#FUNCTIONS-DATETIME-TRUNC

# Generate SQL snippet with DATE_TRUNC
# @param [String] interval
# @param [String] field
# @return [String]
def date_trunc(interval, field)
"DATE_TRUNC('#{interval}', #{field})"
end

protected

def datetime_hierarchies
@datetime_hierarchies ||= %i[
microseconds
milliseconds
second
minute
hour
day
week
month
quarter
year
decade
century
millennium
]
end
end
end
end
8 changes: 8 additions & 0 deletions lib/active_reporting/database_adapters/sqlite_adapter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# frozen_string_literal: true

module ActiveReporting
module DatabaseAdapters
class SqliteAdapter < Base
end
end
end
29 changes: 12 additions & 17 deletions lib/active_reporting/reporting_dimension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@
module ActiveReporting
class ReportingDimension
extend Forwardable
SUPPORTED_DBS = %w[PostgreSQL PostGIS].freeze
# Values for the Postgres `date_trunc` method.
# See https://www.postgresql.org/docs/10/static/functions-datetime.html#FUNCTIONS-DATETIME-TRUNC
DATETIME_HIERARCHIES = %i[microseconds milliseconds second minute hour day week month quarter year decade
century millennium].freeze
def_delegators :@dimension, :name, :type, :klass, :association, :model, :hierarchical?, :datetime?

def self.build_from_dimensions(fact_model, dimensions)
Expand All @@ -31,7 +26,7 @@ def self.label_config(label)
return { label: label } unless label.is_a?(Hash)

{
label: label[:field],
label: label[:label],
label_name: label[:name]
}
end
Expand Down Expand Up @@ -108,7 +103,6 @@ def determine_label_name(label_name)

def validate_hierarchical_label(hierarchical_label)
if datetime?
validate_supported_database_for_datetime_hierarchies
validate_against_datetime_hierarchies(hierarchical_label)
else
validate_dimension_is_hierachical(hierarchical_label)
Expand All @@ -119,34 +113,35 @@ def validate_hierarchical_label(hierarchical_label)

def validate_dimension_is_hierachical(hierarchical_label)
return if hierarchical?
raise InvalidDimensionLabel, "#{name} must be hierarchical to use label #{hierarchical_label}"
end

def validate_supported_database_for_datetime_hierarchies
return if SUPPORTED_DBS.include?(model.connection.adapter_name)
raise InvalidDimensionLabel,
"Cannot utilize datetime grouping for #{name}; " \
"database #{model.connection.adapter_name} is not supported"
raise InvalidDimensionLabel, "#{name} must be hierarchical to use label #{hierarchical_label}"
end

def validate_against_datetime_hierarchies(hierarchical_label)
return if DATETIME_HIERARCHIES.include?(hierarchical_label.to_sym)
return if Configuration.db_adapter.allowed_datetime_hierarchy?(hierarchical_label)

raise InvalidDimensionLabel, "#{hierarchical_label} is not a valid datetime grouping label in #{name}"
end

def validate_against_fact_model_properties(hierarchical_label)
return if dimension_fact_model.hierarchical_levels.include?(hierarchical_label.to_sym)

raise InvalidDimensionLabel, "#{hierarchical_label} is not a hierarchical label in #{name}"
end

def degenerate_fragment
return "#{name}_#{@label}" if datetime?

"#{model.quoted_table_name}.#{name}"
end

def degenerate_select_fragment
return "DATE_TRUNC('#{@label}', #{model.quoted_table_name}.#{name}) AS #{name}_#{@label}" if datetime?
"#{model.quoted_table_name}.#{name}"
return "#{model.quoted_table_name}.#{name}" unless datetime?

date_trunc = Configuration.db_adapter.date_trunc(
@label, "#{model.quoted_table_name}.#{name}"
)
"#{date_trunc} AS #{name}_#{@label}"
end

def identifier_fragment
Expand Down
20 changes: 18 additions & 2 deletions test/active_reporting/report_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,23 @@ def test_report_runs_with_an_aggregate_other_than_count
assert data.all? { |r| r.key?('a_metric') }
end

def test_report_runs_with_a_date_grouping
if ENV['DB'] == 'pg'
def test_report_runs_with_week_date_grouping
if valid_db_adapter?
metric = ActiveReporting::Metric.new(:a_metric, fact_model: SaleFactModel, dimensions: [{created_at: :week}])
report = ActiveReporting::Report.new(metric)
data = report.run
assert data.all? { |r| r.key?('created_at_week') }
assert data.size == 4
else
assert_raises ActiveReporting::InvalidDimensionLabel do
metric = ActiveReporting::Metric.new(:a_metric, fact_model: SaleFactModel, dimensions: [{created_at: :week}])
report = ActiveReporting::Report.new(metric)
end
end
end
Copy link
Contributor

@niborg niborg Oct 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would propose that you might want to add some more comprehensive tests for the MySQL date_trunc adaptation. You can run into a lot of edge cases. See here for some examples you could include.


def test_report_runs_with_month_date_grouping
if valid_db_adapter?
metric = ActiveReporting::Metric.new(:a_metric, fact_model: UserFactModel, dimensions: [{created_at: :month}])
report = ActiveReporting::Report.new(metric)
data = report.run
Expand All @@ -46,4 +61,5 @@ def test_report_runs_with_a_date_grouping
end
end
end

end
2 changes: 1 addition & 1 deletion test/active_reporting/reporting_dimension_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def test_label_can_be_passed_in_if_dimension_is_herarchical
def test_label_can_be_passed_in_if_dimension_is_datetime
refute @user_dimension.hierarchical?
assert @user_dimension.type == ActiveReporting::Dimension::TYPES[:degenerate]
if ENV['DB'] == 'pg'
if valid_db_adapter?
ActiveReporting::ReportingDimension.new(@user_dimension, label: :year)
else
assert_raises ActiveReporting::InvalidDimensionLabel do
Expand Down
1 change: 1 addition & 0 deletions test/fact_models.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class UserFactModel < ActiveReporting::FactModel
class SaleFactModel < ActiveReporting::FactModel
self.measure= :total

dimension :created_at
dimension :placed_at
dimension :item
end
1 change: 1 addition & 0 deletions test/seed.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
base_price: base,
taxes: tax,
total: base + tax,
created_at: Time.now - i.days,
placed_at: DateDimension.find("201603#{(rand(30)+1).to_s.rjust(2, '0')}")
)
end
Expand Down
Loading