Avoid quite useless loop in find_by_sql

`column_types` is empty except PostgreSQL adapter, and
`attribute_types.each_key { |k| column_types.delete k }` is also empty
even if PostgreSQL adapter almost all case, so that code is quite
useless. This improves performance for `find_by_sql` to avoid that
useless loop as much as possible.

```ruby
ActiveRecord::Schema.define do
  create_table :active_storage_blobs do |t|
    t.string   :key,          null: false
    t.string   :filename,     null: false
    t.string   :content_type
    t.text     :metadata
    t.string   :service_name, null: false
    t.bigint   :byte_size,    null: false
    t.string   :checksum,     null: false
    t.datetime :created_at,   null: false

    t.index [ :key ], unique: true
  end
end

class ActiveStorageBlob < ActiveRecord::Base
end

Benchmark.ips do |x|
  x.report("find_by") { ActiveStorageBlob.find_by(id: 1) }
end
```

Before:

```
Warming up --------------------------------------
             find_by     1.256k i/100ms
Calculating -------------------------------------
             find_by     12.595k (± 3.4%) i/s -     64.056k in   5.091599s
```

After:

```
Warming up --------------------------------------
             find_by     1.341k i/100ms
Calculating -------------------------------------
             find_by     13.170k (± 3.5%) i/s -     67.050k in   5.097439s
```

To avoid column types loop for PostgreSQL adapter, this skips returning
additional column types if a column has already been type casted by pg
decoders. Fortunately this fixes #36186 partly for common types.
This commit is contained in:
Ryuta Kamizono 2020-05-01 09:46:23 +09:00
parent 348e142b26
commit 71e45582b7
3 changed files with 33 additions and 3 deletions

@ -54,7 +54,11 @@ def exec_query(sql, name = "SQL", binds = [], prepare: false)
fields.each_with_index do |fname, i|
ftype = result.ftype i
fmod = result.fmod i
types[fname] = get_oid_type(ftype, fmod, fname)
case type = get_oid_type(ftype, fmod, fname)
when Type::Integer, Type::Float, Type::Decimal, Type::String, Type::DateTime, Type::Boolean
# skip if a column has already been type casted by pg decoders
else types[fname] = type
end
end
build_result(columns: fields, rows: result.values, column_types: types)
end

@ -44,8 +44,12 @@ module Querying
# Post.find_by_sql ["SELECT body FROM comments WHERE author = :user_id OR approved_by = :user_id", { :user_id => user_id }]
def find_by_sql(sql, binds = [], preparable: nil, &block)
result_set = connection.select_all(sanitize_sql(sql), "#{name} Load", binds, preparable: preparable)
column_types = result_set.column_types.dup
attribute_types.each_key { |k| column_types.delete k }
column_types = result_set.column_types
unless column_types.empty?
column_types = column_types.reject { |k, _| attribute_types.key?(k) }
end
message_bus = ActiveSupport::Notifications.instrumenter
payload = {

@ -861,6 +861,28 @@ def test_pluck_columns_with_same_name
assert_equal expected, actual
end
def test_pluck_functions_with_alias
assert_equal [
[1, "The First Topic"], [2, "The Second Topic of the day"],
[3, "The Third Topic of the day"], [4, "The Fourth Topic of the day"],
[5, "The Fifth Topic of the day"]
], Topic.order(:id).pluck(
Arel.sql("COALESCE(id, 0) id"),
Arel.sql("COALESCE(title, 'untitled') title")
)
end
def test_pluck_functions_without_alias
assert_equal [
[1, "The First Topic"], [2, "The Second Topic of the day"],
[3, "The Third Topic of the day"], [4, "The Fourth Topic of the day"],
[5, "The Fifth Topic of the day"]
], Topic.order(:id).pluck(
Arel.sql("COALESCE(id, 0)"),
Arel.sql("COALESCE(title, 'untitled')")
)
end
def test_calculation_with_polymorphic_relation
part = ShipPart.create!(name: "has trinket")
part.trinkets.create!