Skip to content

Commit

Permalink
Use empty? instead of count for validate uniqueness
Browse files Browse the repository at this point in the history 
Using `empty?` instead of `count` for performance reasons. The `empty?` checks for
the existence of any record and is generally faster than `count`.
  • Loading branch information
@numbata
numbata committed Jan 19, 2024
1 parent 150e754 commit c457c1e
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 32 deletions.
2 changes: 1 addition & 1 deletion lib/sequel/plugins/validation_helpers.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
14 changes: 11 additions & 3 deletions spec/extensions/auto_validations_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
56 changes: 28 additions & 28 deletions spec/extensions/validation_helpers_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -466,93 +466,93 @@ 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
@c.columns(:id, :username, :password)
@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
@c.columns(:id, :username, :password)
@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?
DB.sqls.must_equal []

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
Expand Down

0 comments on commit c457c1e

Please sign in to comment.