From 2a4bb8a602aed6d0ab009bec41f245b71e9b0bbe Mon Sep 17 00:00:00 2001 From: Sergio Bobillier Date: Fri, 9 Oct 2026 15:36:09 +0200 Subject: [PATCH] [JAY-796] Add intersection support to QueryBuilder Add QueryBuilder#intersection to combine two queries with a boolean 'must' clause, equivalent to AND, while retaining the existing merge behavior for other builder options. Cover empty and non-empty query combinations, and document the method. This behavior is desired when combining query clauses that involve one or more 'should' subclauses. Combining the following two queries with the #merge method: { "bool": { "must": [ { "range": { "timestamp": { "gte": "now-1d/d" } } } ] } } { "bool": { "should": [ { "term": { "field": "value" } }, { "term": { "another_field": "another_value" } } ] } } The resulting query is: { "bool": { "must": [ { "range": { "timestamp": { "gte": "now-1d/d" } } } ], "should": [ { "term": { "field": "value" } }, { "term": { "another_field": "another_value" } } ] } } Because of the way 'should' behaves when paired with 'must', the results produced by this query might not be what one expects. From the Elasticsearch documentation: Matches in the should clauses are optional. They will only boost the relevance scores of documents that already match the required must criteria and don't add new documents to the result set. So, in the above example, the query will basically match all documents whose timestamp is within the last day. If they happen to match the 'should' clauses, it will boost their relevance scores. However, one might actually want to get only the documents that are within the last day and also match either of the clauses inside 'should'. For that, the QueryBuilder#intersection method can be used to combine the queries with a 'must' clause, ensuring that only documents matching both conditions are returned. The resulting query from doing that would be: { "bool": { "must": [ { "bool": { "must": [ { "range": { "timestamp": { "gte": "now-1d/d" } } } ] } }, { "bool": { "should": [ { "term": { "field": "value" } }, { "term": { "another_field": "another_value" } } ] } } ] } } Which produces the intended result set. --- CHANGELOG.md | 2 + .../elasticsearch/query_builder.rst | 54 +++++++++++++ lib/jay_api/elasticsearch/query_builder.rb | 20 +++++ .../query_builder/query_clauses.rb | 17 +++++ .../query_builder/query_clauses_spec.rb | 37 +++++++++ .../elasticsearch/query_builder_spec.rb | 75 +++++++++++++++++++ .../query_builder/query_clauses_spec.rb | 72 ++++++++++++++++++ .../elasticsearch/query_builder_spec.rb | 13 ++++ 8 files changed, 290 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79c13c5..7fe3ae5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ Please mark backwards incompatible changes with an exclamation mark at the start ## [Unreleased] ### Added +- The `#intersection` method to `JayAPI::Elasticsearch::QueryBuilder`, which + combines two queries using a boolean `must` clause (equivalent to `AND`). - The `#union` method to `JayAPI::Elasticsearch::QueryBuilder`, which combines two queries using a boolean `should` clause (equivalent to `OR`). diff --git a/documentation/source/user_guidelines/elasticsearch/query_builder.rst b/documentation/source/user_guidelines/elasticsearch/query_builder.rst index b2c26c6..aa43a0c 100644 --- a/documentation/source/user_guidelines/elasticsearch/query_builder.rst +++ b/documentation/source/user_guidelines/elasticsearch/query_builder.rst @@ -476,6 +476,8 @@ field. # The documents matching the given IDs will be matched query_builder.query.ids(ids: %w[eSO6FZ8BUAdcYcWLRszq of2yFZ8BXEjZhYMm9Goe w1KlFZ8BK-fBiJSF1ptV]) +.. _`QueryBuilder#merge`: + #merge ------ @@ -570,6 +572,58 @@ The ``from``, ``size``, ``source`` and ``collapse`` settings, as well as This method returns a new ``QueryBuilder`` object which can be further modified without affecting the source objects. +#intersection +------------- + +The ``intersection`` method combines two ``QueryBuilder`` objects into a +single one. It combines their top-level query clauses with a boolean ``must`` +query (equivalent to ``AND``), so documents must match both queries. + +Example: + +.. code-block:: ruby + + users_query = QueryBuilder.new + .terms(field: 'user.id', terms: %w[kimchy elkbee]) + .sort('user.created_at' => :desc) + .size(100) + + bio_query = QueryBuilder.new + .query_string(query: "user.bio: painter OR poet") + .size(10) + + combined_query = users_query.intersection(bio_query) + +Would be equivalent to: + +.. code-block:: ruby + + combined_query = QueryBuilder.new + .sort('user.created_at' => :desc) # Kept from the users_query + .size(10) # From bio_query, replacing the first size + + combined_query.query.bool.must do |bool_query| + bool_query.terms(field: 'user.id', terms: %w[kimchy elkbee]) + bool_query.query_string(query: "user.bio: painter OR poet") + end + +The ``from``, ``size``, ``source`` and ``collapse`` settings, as well as +``sort`` and ``aggregations``, are combined using the same rules as ``merge``. + +.. note:: + + This method returns a new ``QueryBuilder`` object which can be further + modified without affecting the source objects. + +.. note:: + + This method differs from :ref:`QueryBuilder#merge` because it doesn't merge + the top-level query clauses if both of them are boolean queries, instead it + just wraps them in a boolean ``must`` query. + + In some cases, this is the desired behaviour, especially when ``should`` + clauses are involved. + aggregations ------------ diff --git a/lib/jay_api/elasticsearch/query_builder.rb b/lib/jay_api/elasticsearch/query_builder.rb index cd30971..5458d7e 100644 --- a/lib/jay_api/elasticsearch/query_builder.rb +++ b/lib/jay_api/elasticsearch/query_builder.rb @@ -149,6 +149,26 @@ def union(other) ) end + # Creates a new +QueryBuilder+ object which is the result of taking the + # intersection of the receiver with +other+. + # @param [JayAPI::Elasticsearch::QueryBuilder] other Another instance of + # +QueryBuilder+. + # @return [JayAPI::Elasticsearch::QueryBuilder] A new +QueryBuilder+, the + # result of the intersection of both objects. + # @see JayAPI::Elasticsearch::QueryClauses#intersection + # @raise [TypeError] If the given object is not a +QueryBuilder+. + def intersection(other) + klass = self.class + raise TypeError, "Cannot create an intersection of #{klass} and #{other.class}" unless other.is_a?(klass) + + query_intersection = query.intersection(other.query) + + other.combine( + from: @from, size: @size, source: @source, collapse: @collapse, + sort: @sort, query: query_intersection, aggregations: @aggregations + ) + end + # @return [JayAPI::Elasticsearch::QueryBuilder] A copy of the receiver. def clone copy = super diff --git a/lib/jay_api/elasticsearch/query_builder/query_clauses.rb b/lib/jay_api/elasticsearch/query_builder/query_clauses.rb index c93acef..56b4151 100644 --- a/lib/jay_api/elasticsearch/query_builder/query_clauses.rb +++ b/lib/jay_api/elasticsearch/query_builder/query_clauses.rb @@ -110,6 +110,23 @@ def union(other) end end + # Creates a new +QueryClauses+ object representing the intersection of + # the receiver and the given object. The individual top-query clauses + # are combined using a +must+ boolean clause. + # @param [JayAPI::Elasticsearch::QueryBuilder::QueryClauses] other The + # +QueryClauses+ object the receiver should be combined with. + # @return [JayAPI::Elasticsearch::QueryBuilder::QueryClauses] A new + # +QueryClauses+ object which is a combination of the receiver and + # the given object. + def intersection(other) + combine(other, __method__) do |combined| + combined.bool.must do |bool| + bool << top_level_clause + bool << other.top_level_clause + end + end + end + # Negates the receiver by wrapping its top-level query clause in a # +must_not+ boolean clause or replacing it by its inverse clause. # @return [self] Returns itself. diff --git a/spec/integration/jay_api/elasticsearch/query_builder/query_clauses_spec.rb b/spec/integration/jay_api/elasticsearch/query_builder/query_clauses_spec.rb index fd19b62..97039a3 100644 --- a/spec/integration/jay_api/elasticsearch/query_builder/query_clauses_spec.rb +++ b/spec/integration/jay_api/elasticsearch/query_builder/query_clauses_spec.rb @@ -129,4 +129,41 @@ end end end + + describe '#intersection' do + subject(:method_call) { query_clauses.intersection(other) } + + let(:other) { described_class.new } + + let(:expected_query) do + { + bool: { + must: [ + { + bool: { + should: [ + { term: { 'test_case.result' => { value: 'fail' } } }, + { exists: { field: 'test_case.refs' } } + ] + } + }, + { range: { 'test_case.date' => { gte: 'now-14d/d' } } } + ] + } + } + end + + before do + query_clauses.bool.should do |bool_query| + bool_query.term(field: 'test_case.result', value: 'fail') + bool_query.exists(field: 'test_case.refs') + end + + other.range(field: 'test_case.date', gte: 'now-14d/d') + end + + it 'returns the expected query' do + expect(method_call.to_h).to eq(expected_query) + end + end end diff --git a/spec/integration/jay_api/elasticsearch/query_builder_spec.rb b/spec/integration/jay_api/elasticsearch/query_builder_spec.rb index 47fa019..40c15bb 100644 --- a/spec/integration/jay_api/elasticsearch/query_builder_spec.rb +++ b/spec/integration/jay_api/elasticsearch/query_builder_spec.rb @@ -470,4 +470,79 @@ end end end + + describe '#intersection' do + subject(:method_call) { query_builder.intersection(other) } + + let(:other) { described_class.new } + + it_behaves_like '#combine' + + context 'when two simple queries are merged' do + before do + query_builder.query.term(field: 'user.name', value: 'kimchy') + other.query.range(field: 'age', gte: 10, lte: 20) + end + + let(:expected_query) do + { + query: { + bool: { + must: [ + { term: { 'user.name' => { value: 'kimchy' } } }, + { range: { 'age' => { gte: 10, lte: 20 } } } + ] + } + } + } + end + + it "merges both query using a boolean 'must' (equivalent to AND)" do + expect(method_call.to_query).to eq(expected_query) + end + end + + context 'when two boolean queries are merged' do + before do + query_builder.query.bool.must do |bool_query| + bool_query.term(field: 'user.name', value: 'kimchy') + bool_query.range(field: 'age', gte: 10, lte: 20) + end + + other.query.bool.must_not do |bool_query| + bool_query.exists(field: 'user.pet') + end + end + + let(:expected_query) do + { + query: { + bool: { + must: [ + { + bool: { + must: [ + { term: { 'user.name' => { value: 'kimchy' } } }, + { range: { 'age' => { gte: 10, lte: 20 } } } + ] + } + }, + { + bool: { + must_not: [ + { exists: { field: 'user.pet' } } + ] + } + } + ] + } + } + } + end + + it "merges the two queries with a boolean 'must' (equivalent to AND)" do + expect(method_call.to_query).to eq(expected_query) + end + end + end end diff --git a/spec/jay_api/elasticsearch/query_builder/query_clauses_spec.rb b/spec/jay_api/elasticsearch/query_builder/query_clauses_spec.rb index e8c7ada..7d5f84e 100644 --- a/spec/jay_api/elasticsearch/query_builder/query_clauses_spec.rb +++ b/spec/jay_api/elasticsearch/query_builder/query_clauses_spec.rb @@ -564,6 +564,78 @@ end end + describe '#intersection' do + subject(:method_call) { query_clauses.intersection(other) } + + it_behaves_like '#combine', 'intersection' + + context "when neither the receiver nor 'other' are empty" do + let(:other) { described_class.new } + + let(:bool_clause) do + instance_double( + JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Bool, + to_h: { bool: { must: [exists_clause_clone.to_h, range_clause_clone.to_h] } } + ) + end + + let(:expected_hash) do + { + bool: { + must: [ + { exists: { field: 'build_job_name' } }, + { range: { 'build_number' => { gt: 150 } } } + ] + } + } + end + + include_context 'with a mocked Range QueryClause' + include_context 'with a mocked Exist QueryClause' + + before do + allow(JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Bool) + .to receive(:new).and_return(bool_clause) + + allow(bool_clause).to receive(:must).and_yield(bool_clause) + allow(bool_clause).to receive(:<<).and_return(bool_clause) + + query_clauses.exists(field: 'build_job_name') + other.range(field: 'build_number', gt: 150) + end + + it 'returns a new instance of the class' do + expect(method_call).to be_a(described_class) + end + + it 'creates a boolean clause to put together the two enclosed clauses' do + expect(JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Bool).to receive(:new) + method_call + end + + it "uses a 'must' boolean clause to join the two enclosed clauses" do + expect(bool_clause).to receive(:must) + method_call + end + + it "adds the receiver's top-level clause to the created boolean clause" do + expect(bool_clause).to receive(:<<).with(exists_clause) + method_call + end + + it "adds 'other's top-level clause to the created boolean clause" do + expect(bool_clause).to receive(:<<).with(range_clause) + method_call + end + + it 'sets the created boolean clause as top-level clause for the returned object' do + # The #to_h call is used to make sure that the +bool_clause+ double was + # set as top-level clause for the returned object. + expect(method_call.to_h).to eq(expected_hash) + end + end + end + shared_context 'with a mocked MatchNone clause' do let(:match_none) do instance_double( diff --git a/spec/jay_api/elasticsearch/query_builder_spec.rb b/spec/jay_api/elasticsearch/query_builder_spec.rb index 50e06a1..4aae318 100644 --- a/spec/jay_api/elasticsearch/query_builder_spec.rb +++ b/spec/jay_api/elasticsearch/query_builder_spec.rb @@ -738,6 +738,19 @@ end end + describe '#intersection' do + subject(:method_call) { query_builder.intersection(other) } + + let(:expected_error) { "Cannot create an intersection of #{described_class} and Array" } + + it_behaves_like '#combine', operation: :intersection do + it "creates an intersection of the receiver's query and the query from 'other'" do + expect(query_clauses).to receive(:intersection).with(other_query_clauses) + method_call + end + end + end + describe '#clone' do subject(:method_call) { query_builder.clone }