diff --git a/ee/config/feature_flags/gitlab_com_derisk/search_glql_fix_null_field_pagination.yml b/ee/config/feature_flags/gitlab_com_derisk/search_glql_fix_null_field_pagination.yml deleted file mode 100644 index 3fe2a05ff47f949ef62ba0d8f3071deb04bd1edc..0000000000000000000000000000000000000000 --- a/ee/config/feature_flags/gitlab_com_derisk/search_glql_fix_null_field_pagination.yml +++ /dev/null @@ -1,10 +0,0 @@ ---- -name: search_glql_fix_null_field_pagination -description: Fix Elasticsearch pagination for queries sorting by nullable fields by converting sentinel values to nil and including must_not exists clauses -feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/583599 -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/215945 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/583611 -milestone: '18.7' -group: group::global search -type: gitlab_com_derisk -default_enabled: false diff --git a/ee/lib/search/elastic/pagination.rb b/ee/lib/search/elastic/pagination.rb index 0abcb1d1ed7f139a5c36e5077bb3a7496689b905..97b84a09d739b479038e82e0e474253498d319b8 100644 --- a/ee/lib/search/elastic/pagination.rb +++ b/ee/lib/search/elastic/pagination.rb @@ -49,12 +49,8 @@ def last(size) end def paginate - if Feature.enabled?(:search_glql_fix_null_field_pagination, Feature.current_request) - # Allow nil sort_property_value (for null values in ES), but require tie_breaker to be set - return query_hash if tie_breaker_property_value.nil? - else - return query_hash unless sort_property_value && tie_breaker_property_value - end + # Allow nil sort_property_value (for null values in ES), but require tie_breaker to be set + return query_hash if tie_breaker_property_value.nil? add_filter(query_hash, :query, :bool, :filter) do pagination_filter @@ -70,9 +66,6 @@ def paginate :is_after def pagination_filter - return legacy_pagination_filter unless Feature.enabled?(:search_glql_fix_null_field_pagination, - Feature.current_request) - # When the sort property value is nil, we're paginating past records with null values. # Elasticsearch sorts nulls last (using Long.MAX_VALUE internally), so we need special handling. return pagination_filter_for_null_cursor if sort_property_value.nil? @@ -143,41 +136,6 @@ def pagination_filter_for_non_null_cursor { bool: { should: should_clauses } } end - def legacy_pagination_filter - # Original pagination logic without null handling - { - bool: { - should: [ - { - range: { - sort_property => { - document_matching_operator(sort_direction) => sort_property_value - } - } - }, - { - bool: { - must: [ - { - term: { - sort_property => sort_property_value - } - }, - { - range: { - tie_breaker_property => { - document_matching_operator(tie_breaker_sort_direction) => tie_breaker_property_value - } - } - } - ] - } - } - ] - } - } - end - def sort_property @sort_property ||= original_sort.each_key.first end diff --git a/ee/lib/search/elastic/relation.rb b/ee/lib/search/elastic/relation.rb index dc1b83705f4393500bbe2923dd9c7c11725bf084..46c86b96268ec893121a07d048ab051feef40e00 100644 --- a/ee/lib/search/elastic/relation.rb +++ b/ee/lib/search/elastic/relation.rb @@ -41,7 +41,6 @@ def last(limit) def cursor_for(record) hit = hit_for(record) sort_values = hit['sort'] - return sort_values unless Feature.enabled?(:search_glql_fix_null_field_pagination, Feature.current_request) # Elasticsearch uses sentinel values for null fields in sorting: # - ASC order: LONG_MAX_VALUE - nulls sort last diff --git a/ee/spec/lib/search/elastic/pagination_spec.rb b/ee/spec/lib/search/elastic/pagination_spec.rb index d5d2b5bbfce85add343d4b681ca5c0df6aac2225..452f05c681f3a864c05597dd9157e797f7230744 100644 --- a/ee/spec/lib/search/elastic/pagination_spec.rb +++ b/ee/spec/lib/search/elastic/pagination_spec.rb @@ -647,86 +647,5 @@ }) end end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(search_glql_fix_null_field_pagination: false) - paginator.after(1768003200000, 100) - end - - subject(:first_10_records_query) { paginator.first(10) } - - it 'uses legacy pagination without null handling' do - expect(first_10_records_query).to eq({ - query: { - bool: { - should: [], - must_not: [], - must: [], - filter: [ - { - bool: { - should: [ - { - range: { - milestone_due_date: { gt: 1768003200000 } - } - }, - { - bool: { - must: [ - { - term: { - milestone_due_date: 1768003200000 - } - }, - { - range: { - id: { gt: 100 } - } - } - ] - } - } - ] - } - } - ] - } - }, - sort: [ - { milestone_due_date: { order: :asc } }, - { id: { order: :asc } } - ], - size: 10 - }) - end - - context 'when sort_property_value is nil' do - before do - paginator.after(nil, 100) - end - - subject(:first_10_records_query) { paginator.first(10) } - - it 'returns query without pagination filters' do - expect(first_10_records_query).to eq({ - query: { - bool: { - should: [], - must_not: [], - must: [], - filter: [] - } - }, - sort: [ - { milestone_due_date: { order: :asc } }, - { id: { order: :asc } } - ], - size: 10 - }) - end - end - end end end diff --git a/ee/spec/lib/search/elastic/relation_spec.rb b/ee/spec/lib/search/elastic/relation_spec.rb index 97b158c5b988ffe86a0e60e4e0e4648f1dc9aa2f..7d38c33a4ab1d152324bdbbe7777f137fc940678 100644 --- a/ee/spec/lib/search/elastic/relation_spec.rb +++ b/ee/spec/lib/search/elastic/relation_spec.rb @@ -150,16 +150,6 @@ expect(cursor).to match([an_instance_of(Integer), record.id]) end - context 'when feature flag is disabled' do - before do - stub_feature_flags(search_glql_fix_null_field_pagination: false) - end - - it 'returns the raw sort values without sentinel conversion' do - expect(cursor).to match([an_instance_of(Integer), record.id]) - end - end - context 'when sort values contain Elasticsearch sentinel values' do let(:sentinel_max) { described_class::ELASTICSEARCH_LONG_MAX_VALUE } let(:sentinel_min) { described_class::ELASTICSEARCH_LONG_MIN_VALUE } @@ -208,19 +198,6 @@ expect(mocked_relation.cursor_for(record)).to eq([nil, nil]) end end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(search_glql_fix_null_field_pagination: false) - allow(mock_response_mapper).to receive(:results).and_return([ - { '_id' => record.id.to_s, 'sort' => [sentinel_max, record.id] } - ]) - end - - it 'returns sentinel values unchanged' do - expect(mocked_relation.cursor_for(record)).to eq([sentinel_max, record.id]) - end - end end end