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

Remove useless type cast on aggregated value #39063

Merged
merged 1 commit into from May 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -565,10 +565,6 @@ def close
pool.checkin self
end

def column_name_for_operation(operation, node) # :nodoc:
visitor.compile(node)
end

def default_index_type?(index) # :nodoc:
index.using.nil?
end
Expand Down
Expand Up @@ -427,16 +427,6 @@ def use_insert_returning?
@use_insert_returning
end

def column_name_for_operation(operation, node) # :nodoc:
OPERATION_ALIASES.fetch(operation) { operation.downcase }
end

OPERATION_ALIASES = { # :nodoc:
"maximum" => "max",
"minimum" => "min",
"average" => "avg",
}

# Returns the version of the connected PostgreSQL server.
def get_database_version # :nodoc:
@connection.server_version
Expand Down Expand Up @@ -904,6 +894,7 @@ def add_pg_decoders
"oid" => PG::TextDecoder::Integer,
"float4" => PG::TextDecoder::Float,
"float8" => PG::TextDecoder::Float,
"numeric" => PG::TextDecoder::Numeric,
"bool" => PG::TextDecoder::Boolean,
}

Expand Down
45 changes: 15 additions & 30 deletions activerecord/lib/active_record/relation/calculations.rb
Expand Up @@ -281,7 +281,8 @@ def aggregate_column(column_name)
end
end

def operation_over_aggregate_column(column, operation, distinct)
def operation_over_aggregate_column(column_name, operation, distinct)
column = aggregate_column(column_name)
operation == "count" ? column.count(distinct) : column.send(operation)
end

Expand All @@ -291,31 +292,20 @@ def execute_simple_calculation(operation, column_name, distinct) #:nodoc:
return 0 if limit_value == 0

query_builder = build_count_subquery(spawn, column_name, distinct)
skip_query_cache_if_necessary { @klass.connection.select_value(query_builder) }
else
# PostgreSQL doesn't like ORDER BY when there are no GROUP BY
relation = unscope(:order).distinct!(false)

column = aggregate_column(column_name)
select_value = operation_over_aggregate_column(column_name, operation, distinct)
select_value.distinct = true if operation == "sum" && distinct

select_value = operation_over_aggregate_column(column, operation, distinct)
if operation == "sum" && distinct
select_value.distinct = true
end

column_alias = select_value.alias
column_alias ||= @klass.connection.column_name_for_operation(operation, select_value)
relation.select_values = [select_value]

query_builder = relation.arel

result = skip_query_cache_if_necessary { @klass.connection.select_all(query_builder, nil) }
row = result.rows.first
value = row && row.first
type = result.column_types.fetch(column_alias, Type.default_value)

type_cast_calculated_value(value, type, operation)
end

value = skip_query_cache_if_necessary { @klass.connection.select_value(query_builder) }
type_cast_calculated_value(value, operation)
end

def execute_grouped_calculation(operation, column_name, distinct) #:nodoc:
Expand All @@ -334,14 +324,11 @@ def execute_grouped_calculation(operation, column_name, distinct) #:nodoc:
}
group_columns = group_aliases.zip(group_fields)

aggregate_alias = column_alias_for("#{operation} #{column_name.to_s.downcase}")
column_alias = column_alias_for("#{operation} #{column_name.to_s.downcase}")
select_value = operation_over_aggregate_column(column_name, operation, distinct)
select_value.as(column_alias)

select_values = [
operation_over_aggregate_column(
aggregate_column(column_name),
operation,
distinct).as(aggregate_alias)
]
select_values = [select_value]
select_values += self.select_values unless having_clause.empty?

select_values.concat group_columns.map { |aliaz, field|
Expand Down Expand Up @@ -374,8 +361,7 @@ def execute_grouped_calculation(operation, column_name, distinct) #:nodoc:
key = key.first if key.size == 1
key = key_records[key] if associated

type = calculated_data.column_types.fetch(aggregate_alias, Type.default_value)
result[key] = type_cast_calculated_value(row[aggregate_alias], type, operation)
result[key] = type_cast_calculated_value(row[column_alias], operation)
end
end

Expand All @@ -401,12 +387,11 @@ def type_for(field, &block)
@klass.type_for_attribute(field_name, &block)
end

def type_cast_calculated_value(value, type, operation)
def type_cast_calculated_value(value, operation)
case operation
when "count" then value.to_i
when "sum" then type.deserialize(value || 0)
when "sum" then value || 0
when "average" then value&.respond_to?(:to_d) ? value.to_d : value
else type.deserialize(value)
else value
end
end

Expand Down