From bc837892e6b17ed9e8aa58c6de539af8fa4f1526 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 27 May 2019 22:57:06 +0900 Subject: Allow symbol (i.e. quoted identifier) as safe SQL string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pluck(:id)` / `order(:id)` are very common use case, and passed symbol (i.e. quoted identifier) is obviously safe argument, but `:id.to_s.split(/\s*,\s*/).all? { |part| permit.match?(part) }` is useless and a bit expensive operation for each such safe symbols (will make extra 2 mutable strings, 1 array, 1 proc). This avoids the expensive operation to such safe symbols, it makes `pluck(:id)` / `order(:id)` itself about 9% faster. https://gist.github.com/kamipo/11d428b57f3629a72ae89c6f21721326 Before (93e640735e9363672b770b8d1c5a35f9e464f806): ``` Warming up -------------------------------------- users.pluck(:id) 1.217k i/100ms users.order(:id).to_sql 1.848k i/100ms Calculating ------------------------------------- users.pluck(:id) 12.239k (± 8.2%) i/s - 60.850k in 5.013839s users.order(:id).to_sql 19.111k (± 7.5%) i/s - 96.096k in 5.064450s ``` After (this change): ``` Warming up -------------------------------------- users.pluck(:id) 1.293k i/100ms users.order(:id).to_sql 2.036k i/100ms Calculating ------------------------------------- users.pluck(:id) 13.257k (± 6.9%) i/s - 65.943k in 5.002568s users.order(:id).to_sql 20.957k (± 7.6%) i/s - 105.872k in 5.086102s ``` --- activerecord/lib/active_record/attribute_methods.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 6e4f76aa73..fd32eaaf3a 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -185,12 +185,14 @@ module ActiveRecord /ix def disallow_raw_sql!(args, permit: COLUMN_NAME) # :nodoc: - unexpected = args.reject do |arg| - Arel.arel_node?(arg) || + unexpected = nil + args.each do |arg| + next if arg.is_a?(Symbol) || Arel.arel_node?(arg) || arg.to_s.split(/\s*,\s*/).all? { |part| permit.match?(part) } + (unexpected ||= []) << arg end - return if unexpected.none? + return unless unexpected if allow_unsafe_raw_sql == :deprecated ActiveSupport::Deprecation.warn( -- cgit v1.2.3