From c457c1edf13d45baa47dcb3a5ee276665b700ea9 Mon Sep 17 00:00:00 2001 From: Andrey Subbota Date: Sun, 1 Jan 2023 19:37:03 +0100 Subject: [PATCH] Use empty? instead of count for validate uniqueness Using `empty?` instead of `count` for performance reasons. The `empty?` checks for the existence of any record and is generally faster than `count`. --- lib/sequel/plugins/validation_helpers.rb | 2 +- spec/extensions/auto_validations_spec.rb | 14 ++++-- spec/extensions/validation_helpers_spec.rb | 56 +++++++++++----------- 3 files changed, 40 insertions(+), 32 deletions(-) diff --git a/lib/sequel/plugins/validation_helpers.rb b/lib/sequel/plugins/validation_helpers.rb index b9e4d85923..7dfc0b4bcc 100644 --- a/lib/sequel/plugins/validation_helpers.rb +++ b/lib/sequel/plugins/validation_helpers.rb @@ -295,7 +295,7 @@ def validates_unique(*atts) h = ds.joined_dataset? ? qualified_pk_hash : pk_hash ds = ds.exclude(h) end - errors.add(a, message) unless ds.count == 0 + errors.add(a, message) unless ds.empty? end end diff --git a/spec/extensions/auto_validations_spec.rb b/spec/extensions/auto_validations_spec.rb index 3116c3083f..28c835292e 100644 --- a/spec/extensions/auto_validations_spec.rb +++ b/spec/extensions/auto_validations_spec.rb @@ -2,7 +2,15 @@ describe "Sequel::Plugins::AutoValidations" do before do - db = Sequel.mock(:fetch=>proc{|sql| sql =~ /'a{51}'|'uniq'/ ? {:v=>0} : {:v=>1}}) + fetch_proc = proc do |sql| + if sql =~ /'a{51}'|'uniq'/ + sql =~ /1 AS one/ ? {:v=>nil} : {:v=>0} + else + {:v=>1} + end + end + + db = Sequel.mock(:fetch=>fetch_proc) def db.schema_parse_table(*) true; end def db.schema(t, *) t = t.first_source if t.is_a?(Sequel::Dataset) @@ -86,7 +94,7 @@ def (@c.db).indexes(t, *) it "should validate using the underlying column values" do @c.send(:define_method, :name){super() * 2} - @c.db.fetch = {:v=>0} + @c.db.fetch = {:v=>nil} @m.set(:d=>Date.today, :num=>1, :name=>'b'*26) @m.valid?.must_equal true end @@ -247,7 +255,7 @@ def (@m.db).supports_index_parsing?() false end @m.set(:d=>Date.today, :num=>1) @m.valid?.must_equal false @m.errors.must_equal([:name, :num]=>["is already taken"]) - @m.db.sqls.must_equal ["SELECT count(*) AS count FROM test WHERE ((name = '1') AND (num = 1)) LIMIT 1"] + @m.db.sqls.must_equal ["SELECT 1 AS one FROM test WHERE ((name = '1') AND (num = 1)) LIMIT 1"] @m.set(:name=>'a'*51) @m.valid?.must_equal false diff --git a/spec/extensions/validation_helpers_spec.rb b/spec/extensions/validation_helpers_spec.rb index 6b8a292c14..c30b77c236 100644 --- a/spec/extensions/validation_helpers_spec.rb +++ b/spec/extensions/validation_helpers_spec.rb @@ -395,9 +395,9 @@ def @m.db_schema; {:value=>{:type=>:integer}} end @c.set_validations{validates_unique(:username, :only_if_modified=>false)} @c.dataset = @c.dataset.with_fetch(proc do |sql| case sql - when /count.*username = '0records'/ - {:v => 0} - when /count.*username = '1record'/ + when /1 AS one.*username = '0records'/ + {:v => nil} + when /1 AS one.*username = '1record'/ {:v => 1} end end) @@ -421,10 +421,10 @@ def @m.db_schema; {:value=>{:type=>:integer}} end @user = @c.load(:id=>1, :username => "0records", :password => "anothertest") @user.must_be :valid? - DB.sqls.last.must_equal "SELECT count(*) AS count FROM items WHERE ((username = '0records') AND (id != 1)) LIMIT 1" + DB.sqls.last.must_equal "SELECT 1 AS one FROM items WHERE ((username = '0records') AND (id != 1)) LIMIT 1" @user = @c.new(:username => "0records", :password => "anothertest") @user.must_be :valid? - DB.sqls.last.must_equal "SELECT count(*) AS count FROM items WHERE (username = '0records') LIMIT 1" + DB.sqls.last.must_equal "SELECT 1 AS one FROM items WHERE (username = '0records') LIMIT 1" end it "should support validates_unique with multiple attributes" do @@ -433,9 +433,9 @@ def @m.db_schema; {:value=>{:type=>:integer}} end @c.set_validations{validates_unique([:username, :password], :only_if_modified=>false)} @c.dataset = @c.dataset.with_fetch(proc do |sql| case sql - when /count.*username = '0records'/ - {:v => 0} - when /count.*username = '1record'/ + when /1 AS one.*username = '0records'/ + {:v => nil} + when /1 AS one.*username = '1record'/ {:v => 1} end end) @@ -466,36 +466,36 @@ def @m.db_schema; {:value=>{:type=>:integer}} end @user = @c.load(:id=>1, :username => "0records", :password => "anothertest") @user.must_be :valid? - DB.sqls.last.must_equal "SELECT count(*) AS count FROM items WHERE ((username = '0records') AND (password = 'anothertest') AND (id != 1)) LIMIT 1" + DB.sqls.last.must_equal "SELECT 1 AS one FROM items WHERE ((username = '0records') AND (password = 'anothertest') AND (id != 1)) LIMIT 1" @user = @c.new(:username => "0records", :password => "anothertest") @user.must_be :valid? - DB.sqls.last.must_equal "SELECT count(*) AS count FROM items WHERE ((username = '0records') AND (password = 'anothertest')) LIMIT 1" + DB.sqls.last.must_equal "SELECT 1 AS one FROM items WHERE ((username = '0records') AND (password = 'anothertest')) LIMIT 1" end it "should support validates_unique with a block" do @c.columns(:id, :username, :password) @c.set_dataset DB[:items] @c.set_validations{validates_unique(:username, :only_if_modified=>false){|ds| ds.filter(:active)}} - @c.dataset = @c.dataset.with_fetch(:v=>0) + @c.dataset = @c.dataset.with_fetch(:v=>nil) DB.reset @c.new(:username => "0records", :password => "anothertest").must_be :valid? @c.load(:id=>3, :username => "0records", :password => "anothertest").must_be :valid? - DB.sqls.must_equal ["SELECT count(*) AS count FROM items WHERE ((username = '0records') AND active) LIMIT 1", - "SELECT count(*) AS count FROM items WHERE ((username = '0records') AND active AND (id != 3)) LIMIT 1"] + DB.sqls.must_equal ["SELECT 1 AS one FROM items WHERE ((username = '0records') AND active) LIMIT 1", + "SELECT 1 AS one FROM items WHERE ((username = '0records') AND active AND (id != 3)) LIMIT 1"] end it "should support validates_unique with :where option" do @c.columns(:id, :username, :password) @c.set_dataset DB[:items] @c.set_validations{validates_unique(:username, :only_if_modified=>false, :where=>proc{|ds, obj, cols| ds.where(cols.map{|c| [Sequel.function(:lower, c), obj.send(c).downcase]})})} - @c.dataset = @c.dataset.with_fetch(:v=>0) - + @c.dataset = @c.dataset.with_fetch(:v=>nil) + DB.reset @c.new(:username => "0RECORDS", :password => "anothertest").must_be :valid? @c.load(:id=>3, :username => "0RECORDS", :password => "anothertest").must_be :valid? - DB.sqls.must_equal ["SELECT count(*) AS count FROM items WHERE (lower(username) = '0records') LIMIT 1", - "SELECT count(*) AS count FROM items WHERE ((lower(username) = '0records') AND (id != 3)) LIMIT 1"] + DB.sqls.must_equal ["SELECT 1 AS one FROM items WHERE (lower(username) = '0records') LIMIT 1", + "SELECT 1 AS one FROM items WHERE ((lower(username) = '0records') AND (id != 3)) LIMIT 1"] end it "should support validates_unique with :dataset option" do @@ -503,13 +503,13 @@ def @m.db_schema; {:value=>{:type=>:integer}} end @c.set_dataset DB[:items] c = @c @c.set_validations{validates_unique(:username, :only_if_modified=>false, :dataset=>c.where(:a=>[1,2,3]))} - @c.dataset = @c.dataset.with_fetch(:v=>0) + @c.dataset = @c.dataset.with_fetch(:v=>nil) DB.reset @c.new(:username => "0records", :password => "anothertest").must_be :valid? @c.load(:id=>3, :username => "0records", :password => "anothertest").must_be :valid? - DB.sqls.must_equal ["SELECT count(*) AS count FROM items WHERE ((a IN (1, 2, 3)) AND (username = '0records')) LIMIT 1", - "SELECT count(*) AS count FROM items WHERE ((a IN (1, 2, 3)) AND (username = '0records') AND (id != 3)) LIMIT 1"] + DB.sqls.must_equal ["SELECT 1 AS one FROM items WHERE ((a IN (1, 2, 3)) AND (username = '0records')) LIMIT 1", + "SELECT 1 AS one FROM items WHERE ((a IN (1, 2, 3)) AND (username = '0records') AND (id != 3)) LIMIT 1"] end it "should use qualified primary keys for validates_unique when the dataset is joined" do @@ -517,24 +517,24 @@ def @m.db_schema; {:value=>{:type=>:integer}} end @c.set_dataset DB[:items] c = @c @c.set_validations{validates_unique(:username, :only_if_modified=>false, :dataset=>c.cross_join(:a))} - @c.dataset = @c.dataset.with_fetch(:v=>0) + @c.dataset = @c.dataset.with_fetch(:v=>nil) DB.reset @c.new(:username => "0records", :password => "anothertest").must_be :valid? @c.load(:id=>3, :username => "0records", :password => "anothertest").must_be :valid? - DB.sqls.must_equal ["SELECT count(*) AS count FROM items CROSS JOIN a WHERE (username = '0records') LIMIT 1", - "SELECT count(*) AS count FROM items CROSS JOIN a WHERE ((username = '0records') AND (items.id != 3)) LIMIT 1"] + DB.sqls.must_equal ["SELECT 1 AS one FROM items CROSS JOIN a WHERE (username = '0records') LIMIT 1", + "SELECT 1 AS one FROM items CROSS JOIN a WHERE ((username = '0records') AND (items.id != 3)) LIMIT 1"] end it "should not have validates_unique check uniqueness for existing records if values haven't changed" do @c.columns(:id, :username, :password) @c.set_dataset DB[:items] @c.set_validations{validates_unique([:username, :password])} - @c.dataset = @c.dataset.with_fetch(:v=>0) + @c.dataset = @c.dataset.with_fetch(:v=>nil) DB.reset @c.new(:username => "0records", :password => "anothertest").must_be :valid? - DB.sqls.must_equal ["SELECT count(*) AS count FROM items WHERE ((username = '0records') AND (password = 'anothertest')) LIMIT 1"] + DB.sqls.must_equal ["SELECT 1 AS one FROM items WHERE ((username = '0records') AND (password = 'anothertest')) LIMIT 1"] DB.reset m = @c.load(:id=>3, :username => "0records", :password => "anothertest") m.must_be :valid? @@ -542,17 +542,17 @@ def @m.db_schema; {:value=>{:type=>:integer}} end m.username = '1' m.must_be :valid? - DB.sqls.must_equal ["SELECT count(*) AS count FROM items WHERE ((username = '1') AND (password = 'anothertest') AND (id != 3)) LIMIT 1"] + DB.sqls.must_equal ["SELECT 1 AS one FROM items WHERE ((username = '1') AND (password = 'anothertest') AND (id != 3)) LIMIT 1"] m = @c.load(:id=>3, :username => "0records", :password => "anothertest") DB.reset m.password = '1' m.must_be :valid? - DB.sqls.must_equal ["SELECT count(*) AS count FROM items WHERE ((username = '0records') AND (password = '1') AND (id != 3)) LIMIT 1"] + DB.sqls.must_equal ["SELECT 1 AS one FROM items WHERE ((username = '0records') AND (password = '1') AND (id != 3)) LIMIT 1"] DB.reset m.username = '2' m.must_be :valid? - DB.sqls.must_equal ["SELECT count(*) AS count FROM items WHERE ((username = '2') AND (password = '1') AND (id != 3)) LIMIT 1"] + DB.sqls.must_equal ["SELECT 1 AS one FROM items WHERE ((username = '2') AND (password = '1') AND (id != 3)) LIMIT 1"] end it "should not attempt a database query if the underlying columns have validation errors" do