From 2363c985abc586a4b5ba68b9c7aa80a47e6bba9b Mon Sep 17 00:00:00 2001 From: Nathan Wilson Date: Fri, 2 Jan 2015 13:32:43 +0000 Subject: [PATCH] Index page speed up by separating counting everything from getting what we need --- app/models/abstract_query.rb | 28 +++- config/consts.rb | 4 + test/unit/query_test.rb | 259 ++++++++++++++++++----------------- 3 files changed, 160 insertions(+), 131 deletions(-) diff --git a/app/models/abstract_query.rb b/app/models/abstract_query.rb index 31697a20a5..46de6c967f 100644 --- a/app/models/abstract_query.rb +++ b/app/models/abstract_query.rb @@ -1656,7 +1656,12 @@ def uses_join_sub(tree, arg) # :nodoc: # Number of results the query returns. def num_results(args={}) - result_ids(args).length + @result_count ||= + if @result_ids + @result_ids.count + else + rows = select_rows(args.merge(:select => "count(*)"))[0][0] + end end # Array of all results, just ids. @@ -1692,6 +1697,7 @@ def results(args={}) # better all be valid instances of +model+ -- no error checking is done!! def results=(list) @result_ids = list.map(&:id) + @result_count = list.count @results = list.inject({}) do |map,obj| map[obj.id] ||= obj map @@ -1703,6 +1709,7 @@ def results=(list) # better all be valid Fixnum ids -- no error checking is done!! def result_ids=(list) @result_ids = list + @result_count = list.count end # Get index of a given record / id in the results. @@ -1720,6 +1727,7 @@ def need_letters=(x) raise "You need to pass in a SQL expression to 'need_letters'." elsif @need_letters != x @result_ids = nil + @result_count = nil @need_letters = x end end @@ -1732,20 +1740,25 @@ def paginate_ids(paginator, args={}) # Get list of letters used in results. if need_letters - num_results + @result_ids = nil + @result_count = nil + result_ids(results_args) map = letters paginator.used_letters = map.values.uniq # Filter by letter. (paginator keeps letter upper case, as do we) if letter = paginator.letter @result_ids = @result_ids.select {|id| map[id] == letter} + @result_count = @result_ids.count end + paginator.num_total = num_results(results_args) + @result_ids[paginator.from..paginator.to] || [] + else + # Paginate remaining results. + paginator.num_total = num_results(results_args) + results_args[:limit] = "#{paginator.from},#{paginator.num_per_page}" + result_ids(results_args) || [] end - - # Paginate remaining results. - paginator.num_total = num_results(results_args) - from, to = paginator.from, paginator.to - result_ids(results_args)[from..to] || [] end # Returns a subset of the results (as ActiveRecord instances). @@ -1784,6 +1797,7 @@ def instantiate(ids, args={}) def clear_cache @results = nil @result_ids = nil + @result_count = nil @letters = nil end diff --git a/config/consts.rb b/config/consts.rb index deeae95c0c..cfbdf9b793 100644 --- a/config/consts.rb +++ b/config/consts.rb @@ -129,4 +129,8 @@ # Limit size of image uploads (ImageMagick bogs down on large images). config.image_upload_max_size = 20000000 + + # Flag intended for controller when the debugger gets invoked. + # Use with lines like: debugger if MO.debugger_flag + config.debugger_flag = false end diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index c665f00915..5a6e4e6420 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -128,28 +128,28 @@ def test_validate_params assert_raises(RuntimeError) { Query.lookup(:Name, :all, :order => ['1', '2']) } end - def test_initialize_helpers - query = Query.lookup(:Name) + def test_initialize_helpers + query = Query.lookup(:Name) - assert_equal('4,1,2', query.clean_id_set(['4', 1, 4, 2, 4, 1, 2])) - assert_equal('-1', query.clean_id_set([])) + assert_equal('4,1,2', query.clean_id_set(['4', 1, 4, 2, 4, 1, 2])) + assert_equal('-1', query.clean_id_set([])) - assert_equal('blah', query.clean_pattern('blah')) - assert_equal('foo bar', query.clean_pattern('foo bar')) - assert_equal('\\"foo\\%bar\\"', query.clean_pattern('"foo%bar"')) - assert_equal('one\\\\two', query.clean_pattern('one\\two')) - assert_equal("foo%bar", query.clean_pattern('foo*bar')) + assert_equal('blah', query.clean_pattern('blah')) + assert_equal('foo bar', query.clean_pattern('foo bar')) + assert_equal('\\"foo\\%bar\\"', query.clean_pattern('"foo%bar"')) + assert_equal('one\\\\two', query.clean_pattern('one\\two')) + assert_equal("foo%bar", query.clean_pattern('foo*bar')) - assert_equal(nil, query.and_clause()) - assert_equal('one', query.and_clause('one')) - assert_equal('(one AND two)', query.and_clause('one', 'two')) - assert_equal('(one AND two AND three)', query.and_clause('one', 'two', 'three')) + assert_equal(nil, query.and_clause()) + assert_equal('one', query.and_clause('one')) + assert_equal('(one AND two)', query.and_clause('one', 'two')) + assert_equal('(one AND two AND three)', query.and_clause('one', 'two', 'three')) - assert_equal(nil, query.or_clause()) - assert_equal('one', query.or_clause('one')) - assert_equal('(one OR two)', query.or_clause('one', 'two')) - assert_equal('(one OR two OR three)', query.or_clause('one', 'two', 'three')) - end + assert_equal(nil, query.or_clause()) + assert_equal('one', query.or_clause('one')) + assert_equal('(one OR two)', query.or_clause('one', 'two')) + assert_equal('(one OR two OR three)', query.or_clause('one', 'two', 'three')) + end def test_google_parse query = Query.lookup(:Name) @@ -437,118 +437,129 @@ def test_join_direction assert_match(/rss_logs.observation_id = observations.id/, query.query) end - def test_low_levels - query = Query.lookup(:Name, :all, :misspellings => :either, :by => :id) - - @fungi = names(:fungi) - @agaricus = names(:agaricus) - num = Name.count - num_agaricus = Name.count(:conditions => 'text_name LIKE "Agaricus%"') - - assert_equal(num, query.select_count) - assert_equal(num, query.select_count(:limit => 10)) # limit limits number of counts!! - assert_equal(num_agaricus, query.select_count(:where => 'text_name LIKE "Agaricus%"')) - - assert_equal('1', query.select_value.to_s) # first id - assert_equal('11', query.select_value(:limit => '10, 10').to_s) # tenth id - assert_equal(num.to_s, query.select_value(:order => :reverse).to_s) # last id - assert_equal('Fungi', query.select_value(:select => 'text_name').to_s) - - assert_equal((1..num).map(&:to_s), query.select_values.map(&:to_s)) - assert_equal(['3','18','19','20','21'], query.select_values(:where => 'text_name LIKE "Agaricus%"').map(&:to_s)) - agaricus = query.select_values(:select => 'text_name', :where => 'text_name LIKE "Agaricus%"').map(&:to_s) - assert_equal(num_agaricus, agaricus.uniq.length) - assert_equal(num_agaricus, agaricus.select {|x| x[0,8] == 'Agaricus'}.length) - - if RUBY_VERSION < '1.9' - assert_equal((1..num).map {|x| [x.to_s]}, query.select_rows) - assert_equal((1..num).map {|x| {'id' => x.to_s}}, query.select_all) - assert_equal({'id' => '1'}, query.select_one) - else - assert_equal((1..num).map {|x| [x]}, query.select_rows) - assert_equal((1..num).map {|x| {'id' => x}}, query.select_all) - assert_equal({'id' => 1}, query.select_one) - end - assert_equal([@fungi], query.find_by_sql(:limit => 1)) - assert_equal(@agaricus.children.sort_by(&:id), query.find_by_sql(:where => 'text_name LIKE "Agaricus %"')) + def test_low_levels + query = Query.lookup(:Name, :all, :misspellings => :either, :by => :id) + + @fungi = names(:fungi) + @agaricus = names(:agaricus) + num = Name.count + num_agaricus = Name.count(:conditions => 'text_name LIKE "Agaricus%"') + + assert_equal(num, query.select_count) + assert_equal(num, query.select_count(:limit => 10)) # limit limits number of counts!! + assert_equal(num_agaricus, query.select_count(:where => 'text_name LIKE "Agaricus%"')) + + assert_equal('1', query.select_value.to_s) # first id + assert_equal('11', query.select_value(:limit => '10, 10').to_s) # tenth id + assert_equal(num.to_s, query.select_value(:order => :reverse).to_s) # last id + assert_equal('Fungi', query.select_value(:select => 'text_name').to_s) + + assert_equal((1..num).map(&:to_s), query.select_values.map(&:to_s)) + assert_equal(['3','18','19','20','21'], query.select_values(:where => 'text_name LIKE "Agaricus%"').map(&:to_s)) + agaricus = query.select_values(:select => 'text_name', :where => 'text_name LIKE "Agaricus%"').map(&:to_s) + assert_equal(num_agaricus, agaricus.uniq.length) + assert_equal(num_agaricus, agaricus.select {|x| x[0,8] == 'Agaricus'}.length) + + if RUBY_VERSION < '1.9' + assert_equal((1..num).map {|x| [x.to_s]}, query.select_rows) + assert_equal((1..num).map {|x| {'id' => x.to_s}}, query.select_all) + assert_equal({'id' => '1'}, query.select_one) + else + assert_equal((1..num).map {|x| [x]}, query.select_rows) + assert_equal((1..num).map {|x| {'id' => x}}, query.select_all) + assert_equal({'id' => 1}, query.select_one) end + assert_equal([@fungi], query.find_by_sql(:limit => 1)) + assert_equal(@agaricus.children.sort_by(&:id), query.find_by_sql(:where => 'text_name LIKE "Agaricus %"')) + end - def test_tables_used - query = Query.lookup(:Observation, :all, :by => :id) - assert_equal([:observations], query.tables_used) + def test_tables_used + query = Query.lookup(:Observation, :all, :by => :id) + assert_equal([:observations], query.tables_used) - query = Query.lookup(:Observation, :all, :by => :name) - assert_equal([:names,:observations], query.tables_used) + query = Query.lookup(:Observation, :all, :by => :name) + assert_equal([:names,:observations], query.tables_used) - query = Query.lookup(:Image, :all, :by => :name) - assert_equal([:images,:images_observations,:names,:observations], query.tables_used) - assert_equal(true, query.uses_table?(:images)) - assert_equal(true, query.uses_table?(:images_observations)) - assert_equal(true, query.uses_table?(:names)) - assert_equal(true, query.uses_table?(:observations)) - assert_equal(false, query.uses_table?(:comments)) - end + query = Query.lookup(:Image, :all, :by => :name) + assert_equal([:images,:images_observations,:names,:observations], query.tables_used) + assert_equal(true, query.uses_table?(:images)) + assert_equal(true, query.uses_table?(:images_observations)) + assert_equal(true, query.uses_table?(:names)) + assert_equal(true, query.uses_table?(:observations)) + assert_equal(false, query.uses_table?(:comments)) + end - def test_results - query = Query.lookup(:User, :all, :by => :id) - assert_equal(Set.new, Set.new([1,2,3,4,5,6]) - query.result_ids) - assert_equal(roy.location_format, :scientific) - assert_equal(Set.new, Set.new([rolf,mary,junk,dick,katrina,roy]) - query.results) - assert_equal(2, query.index(3)) - assert_equal(3, query.index('4')) - assert_equal(1, query.index(mary)) - - # Verify that it's getting all this crap from cache. - query.result_ids = [1,3,5,100] - assert_equal([rolf,junk,katrina], query.results) - - # Should be able to set it this way, to. - query.results = [dick, mary, rolf] - assert_equal(3, query.num_results) - assert_equal([4,2,1], query.result_ids) - assert_equal([dick,mary,rolf], query.results) - assert_equal(1, query.index(mary)) - assert_equal(2, query.index(1)) - end + def test_results + query = Query.lookup(:User, :all, :by => :id) + assert_equal(Set.new, Set.new([1,2,3,4,5,6]) - query.result_ids) + assert_equal(roy.location_format, :scientific) + assert_equal(Set.new, Set.new([rolf,mary,junk,dick,katrina,roy]) - query.results) + assert_equal(2, query.index(3)) + assert_equal(3, query.index('4')) + assert_equal(1, query.index(mary)) - def test_paginate - # The only methods of Paginator used by Query are: - # from, to index of first and last itemto show - # num_total= lets Query tell it how many results there are - # letter selected letter (if any) - # used_letters= lets Query tell it which letters have results - query = Query.lookup(:Name, :all, :misspellings => :either, :by => :id) - @names = Name.all - - pages = Wrapper.new(:from => 1, :to => 4) - assert_equal([2,3,4,5], query.paginate_ids(pages)) - assert_equal(@names.size, pages.num_total) - assert_equal(@names[1..4], query.paginate(pages)) - - pages = Wrapper.new(:from => 5, :to => 8) - assert_equal([6,7,8,9], query.paginate_ids(pages)) - assert_equal(@names.size, pages.num_total) - assert_equal(@names[5..8], query.paginate(pages)) - - pages = Wrapper.new(:from => 1, :to => 4) - query.need_letters = 'names.text_name' - assert_equal([2,3,4,5], query.paginate_ids(pages)) - assert_equal(@names.size, pages.num_total) - assert_equal(@names[1..4], query.paginate(pages)) - letters = @names.map {|n| n.text_name[0,1]}.uniq - assert_equal(letters.sort, pages.used_letters.sort) - - # Make sure we have a bunch of Lactarii, Leptiotas, etc. - @ells = @names.select {|n| n.text_name[0,1] == 'L'} - assert(@ells.length >= 9) - - pages = Wrapper.new(:from => 3, :to => 6, :letter => 'L') - # Need to clear out cache or used_letters will be wrong. - query.clear_cache - assert_equal(@ells[3..6].map(&:id), query.paginate_ids(pages)) - assert_equal(letters.sort, pages.used_letters.sort) - assert_equal(@ells[3..6], query.paginate(pages)) - end + # Verify that it's getting all this crap from cache. + query.result_ids = [1,3,5,100] + assert_equal([rolf,junk,katrina], query.results) + + # Should be able to set it this way, to. + query.results = [dick, mary, rolf] + assert_equal(3, query.num_results) + assert_equal([4,2,1], query.result_ids) + assert_equal([dick,mary,rolf], query.results) + assert_equal(1, query.index(mary)) + assert_equal(2, query.index(1)) + end + + def paginate_test_setup(from, to) + @names = Name.all + @pages = Wrapper.new(from: from, to: to, num_per_page: to-from+1) + @query = Query.lookup(:Name, :all, misspellings: :either, by: :id) + end + + def paginate_test(from, to, expected) + paginate_test_setup(from, to) + paginate_assertions(from, to, expected) + end + + def paginate_assertions(from, to, expected) + assert_equal(expected, @query.paginate_ids(@pages)) + assert_equal(@names.size, @pages.num_total) + assert_equal(@names[from..to], @query.paginate(@pages)) + end + + def test_paginate_start + paginate_test(1, 4, [2,3,4,5]) + end + + def test_paginate_middle + MO.debugger_flag = true + paginate_test(5, 8, [6,7,8,9]) + end + + def paginate_test_letter_setup(to, from) + paginate_test_setup(to, from) + @query.need_letters = 'names.text_name' + @letters = @names.map {|n| n.text_name[0,1]}.uniq.sort + end + + def test_paginate_need_letters + paginate_test_letter_setup(1, 4) + paginate_assertions(1, 4, [2,3,4,5]) + assert_equal(@letters, @pages.used_letters.sort) + end + + def test_paginate_ells + paginate_test_letter_setup(3, 6) + @pages = Wrapper.new(from: 3, to: 6, letter: 'L', num_per_page: 4) + + # Make sure we have a bunch of Lactarii, Leptiotas, etc. + @ells = @names.select {|n| n.text_name[0,1] == 'L'} + assert(@ells.length >= 9) + assert_equal(@ells[3..6].map(&:id), @query.paginate_ids(@pages)) + assert_equal(@letters, @pages.used_letters.sort) + assert_equal(@ells[3..6], @query.paginate(@pages)) + end def test_eager_instantiator query = Query.lookup(:Observation)