From 64ca6f608ec4456805fd7f40c8d6e0009e286415 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 1 Mar 2021 18:14:43 +0900 Subject: [PATCH] Avoid extra `BindParam` allocation to generate placeholder in queries In the Active Record usage, a `BindParam` object always has an attribute object as the value. A `BindParam` object is used to call `add_bind` to generate placeholder if `prepared_statements: true`. Since Arel is a part of Active Record now, I think that we can regard an `ActiveModel::Attribute` object as boundable without wrapping it by a `BindParam` to avoid extra `BindParam` allocation. --- activerecord/lib/active_record/persistence.rb | 10 ++-------- .../lib/active_record/relation/predicate_builder.rb | 3 +-- .../lib/active_record/relation/query_methods.rb | 3 +-- .../lib/active_record/relation/where_clause.rb | 7 +++---- activerecord/lib/arel/nodes/casted.rb | 2 +- activerecord/lib/arel/predications.rb | 2 +- activerecord/lib/arel/visitors/to_sql.rb | 8 ++++++-- 7 files changed, 15 insertions(+), 20 deletions(-) diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index fd59c889c0..9968580be8 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -368,7 +368,7 @@ module ActiveRecord if values.empty? im.insert(connection.empty_insert_statement_value(primary_key)) else - im.insert(_substitute_values(values)) + im.insert(values.transform_keys { |name| arel_table[name] }) end connection.insert(im, "#{self} Create", primary_key || false, primary_key_value) @@ -386,7 +386,7 @@ module ActiveRecord end um = Arel::UpdateManager.new(arel_table) - um.set(_substitute_values(values)) + um.set(values.transform_keys { |name| arel_table[name] }) um.wheres = constraints connection.update(um, "#{self} Update") @@ -425,12 +425,6 @@ module ActiveRecord def discriminate_class_for_record(record) self end - - def _substitute_values(values) - values.map do |name, value| - [ arel_table[name], Arel::Nodes::BindParam.new(value) ] - end - end end # Returns true if this object hasn't been saved yet -- that is, a record diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index ccf0d10d05..1a1091556e 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -65,8 +65,7 @@ module ActiveRecord end def build_bind_attribute(column_name, value) - attr = Relation::QueryAttribute.new(column_name, value, table.type(column_name)) - Arel::Nodes::BindParam.new(attr) + Relation::QueryAttribute.new(column_name, value, table.type(column_name)) end def resolve_arel_attribute(table_name, column_name, &block) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index d2285257c7..60d55bcfd5 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1263,8 +1263,7 @@ module ActiveRecord end def build_cast_value(name, value) - cast_value = ActiveModel::Attribute.with_cast_value(name, value, Type.default_value) - Arel::Nodes::BindParam.new(cast_value) + ActiveModel::Attribute.with_cast_value(name, value, Type.default_value) end def build_from diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index 37fbb12575..97ec0cdc0c 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -224,11 +224,10 @@ module ActiveRecord end def extract_node_value(node) - case node - when Array - node.map { |v| extract_node_value(v) } - when Arel::Nodes::BindParam, Arel::Nodes::Casted, Arel::Nodes::Quoted + if node.respond_to?(:value_before_type_cast) node.value_before_type_cast + elsif Array === node + node.map { |v| extract_node_value(v) } end end end diff --git a/activerecord/lib/arel/nodes/casted.rb b/activerecord/lib/arel/nodes/casted.rb index 98a7f6d0e2..bd389f3930 100644 --- a/activerecord/lib/arel/nodes/casted.rb +++ b/activerecord/lib/arel/nodes/casted.rb @@ -47,7 +47,7 @@ module Arel # :nodoc: all def self.build_quoted(other, attribute = nil) case other - when Arel::Nodes::Node, Arel::Attributes::Attribute, Arel::Table, Arel::SelectManager, Arel::Nodes::SqlLiteral + when Arel::Nodes::Node, Arel::Attributes::Attribute, Arel::Table, Arel::SelectManager, Arel::Nodes::SqlLiteral, ActiveModel::Attribute other else case attribute diff --git a/activerecord/lib/arel/predications.rb b/activerecord/lib/arel/predications.rb index 0d2d1acc93..bbcc192d2f 100644 --- a/activerecord/lib/arel/predications.rb +++ b/activerecord/lib/arel/predications.rb @@ -52,7 +52,7 @@ module Arel # :nodoc: all else left = quoted_node(other.begin) right = quoted_node(other.end) - Nodes::Between.new(self, left.and(right)) + Nodes::Between.new(self, Nodes::And.new([left, right])) end end diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb index fac8c921da..f593f4980a 100644 --- a/activerecord/lib/arel/visitors/to_sql.rb +++ b/activerecord/lib/arel/visitors/to_sql.rb @@ -103,7 +103,7 @@ module Arel # :nodoc: all row.each_with_index do |value, k| collector << ", " unless k == 0 case value - when Nodes::SqlLiteral, Nodes::BindParam + when Nodes::SqlLiteral, Nodes::BindParam, ActiveModel::Attribute collector = visit(value, collector) else collector << quote(value).to_s @@ -585,7 +585,7 @@ module Arel # :nodoc: all def visit_Arel_Nodes_Assignment(o, collector) case o.right - when Arel::Nodes::Node, Arel::Attributes::Attribute + when Arel::Nodes::Node, Arel::Attributes::Attribute, ActiveModel::Attribute collector = visit o.left, collector collector << " = " visit o.right, collector @@ -695,6 +695,10 @@ module Arel # :nodoc: all def bind_block; BIND_BLOCK; end + def visit_ActiveModel_Attribute(o, collector) + collector.add_bind(o, &bind_block) + end + def visit_Arel_Nodes_BindParam(o, collector) collector.add_bind(o.value, &bind_block) end