From 17ae99072db047accafd83eac3a7e5d802220c56 Mon Sep 17 00:00:00 2001 From: Ash Tyndall Date: Mon, 10 Jun 2019 15:36:48 +0200 Subject: [PATCH] Handle edge case where there is a GSI with the same hash key as the primary index In the current implementation, when a `where` query is performed with a hash key and range key that would match a GSI, but the hash key overlaps with the primary index, the GSI is not selected and an expensive Scan is performed instead of the cheap Query. --- lib/dynamoid/criteria/key_fields_detector.rb | 4 +--- spec/dynamoid/criteria/chain_spec.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/dynamoid/criteria/key_fields_detector.rb b/lib/dynamoid/criteria/key_fields_detector.rb index 91eb9f3f..0ce5f186 100644 --- a/lib/dynamoid/criteria/key_fields_detector.rb +++ b/lib/dynamoid/criteria/key_fields_detector.rb @@ -39,8 +39,6 @@ def detect_keys @range_key = lsi.range_key @index_name = lsi.name end - - return end # See if can use any global secondary index @@ -49,7 +47,7 @@ def detect_keys # get back full data @source.global_secondary_indexes.each do |_, gsi| next unless @query.keys.map(&:to_s).include?(gsi.hash_key.to_s) && gsi.projected_attributes == :all - next if @range_key.present? && !query_keys.include?(gsi.range_key.to_s) + next if @hash_key.present? && !query_keys.include?(gsi.range_key.to_s) @hash_key = gsi.hash_key @range_key = gsi.range_key diff --git a/spec/dynamoid/criteria/chain_spec.rb b/spec/dynamoid/criteria/chain_spec.rb index 7e0f7498..e19e6937 100644 --- a/spec/dynamoid/criteria/chain_spec.rb +++ b/spec/dynamoid/criteria/chain_spec.rb @@ -666,6 +666,7 @@ def request_params global_secondary_index hash_key: :city, range_key: :age, name: :cityage, projected_attributes: :all global_secondary_index hash_key: :city, range_key: :gender, name: :citygender, projected_attributes: :all global_secondary_index hash_key: :email, range_key: :age, name: :emailage, projected_attributes: :all + global_secondary_index hash_key: :name, range_key: :age, name: :nameage, projected_attributes: :all end end @@ -766,6 +767,15 @@ def request_params expect(chain.key_fields_detector.range_key).to eq(:gender) expect(chain.key_fields_detector.index_name).to eq(:citygender) end + + it 'uses global secondary index when secondary hash key overlaps with primary hash key and range key matches' do + chain = Dynamoid::Criteria::Chain.new(model) + expect(chain).to receive(:pages_via_query).and_call_original + expect(chain.where(name: 'Bob', age: 10).to_a.size).to eq(1) + expect(chain.key_fields_detector.hash_key).to eq(:name) + expect(chain.key_fields_detector.range_key).to eq(:age) + expect(chain.key_fields_detector.index_name).to eq(:nameage) + end end it 'supports query on global secondary index with correct start key without table range key' do