From 6fe301797c82a4d2ac14c6deb2f9d60947ceb3fe Mon Sep 17 00:00:00 2001 From: Sergio Bobillier Date: Fri, 9 Oct 2026 16:48:06 +0200 Subject: [PATCH] [JAY-796] Fix QueryBuilder#merge docs and tests The documentation for this method was wrong, it claimed that, when two QueryBuilders are merged their query clauses are put together with a boolean 'must' clause, however, this is only true when either of the clauses is not a boolean clause. When both clauses are boolean clauses, the result is a compound boolean clause in which equivalent boolean subclauses are merged into a single one. This commit fixes the issue and expands the documentation to properly describe the correct behavior of the merge operation. An unnecessary #allow for a call to #must is also being removed from the method's tests. Both the documentation and the stray #allow are leftovers from a previous implementation. --- .../elasticsearch/query_builder.rst | 47 ++++++++++++++++++- .../query_builder/query_clauses_spec.rb | 1 - 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/documentation/source/user_guidelines/elasticsearch/query_builder.rst b/documentation/source/user_guidelines/elasticsearch/query_builder.rst index aa43a0c..87254e1 100644 --- a/documentation/source/user_guidelines/elasticsearch/query_builder.rst +++ b/documentation/source/user_guidelines/elasticsearch/query_builder.rst @@ -484,7 +484,10 @@ field. The ``merge`` method merges two ``QueryBuilder`` objects into a single one. This opens the door to the construction of compound queries. -Example: +Examples: + +When the ``QueryBuilder`` objects being merged have only non-boolean query +clauses, they are wrapped in a boolean ``must`` query: .. code-block:: ruby @@ -512,6 +515,47 @@ Would be equivalent to: bool_query.query_string(query: "user.bio: painter OR poet") end +-------- + +When the ``QueryBuilder`` objects being merged have boolean query clauses, they +are combined into a single boolean query: + +.. code-block:: ruby + + users_query = QueryBuilder.new + .query.bool.must do |bool_query| + bool_query.terms(field: 'user.id', terms: %w[kimchy elkbee]) + bool_query.terms(field: 'user.active', terms: 'true']) + end + + bio_query = QueryBuilder.new + .query.bool.must do |bool_query| + bool_query.query_string(query: "user.bio: painter OR poet") + end + + pet_query = QueryBuilder.new + .query.bool.must_not do |bool_query| + bool_query.term(field: 'user.pet', value: 'cat') + end + + compound_query = users_query.merge(bio_query).merge(pet_query) + +Would be equivalent to: + +.. code-block:: ruby + + compound_query = QueryBuilder.new + + compound_query.query.bool.must do |bool_query| + bool_query.terms(field: 'user.id', terms: %w[kimchy elkbee]) # From user_query + bool_query.terms(field: 'user.active', terms: 'true']) # From user_query + bool_query.query_string(query: "user.bio: painter OR poet") # From bio_query + end + + compound_query.query.bool.must_not do |bool_query| + bool_query.term(field: 'user.pet', value: 'cat') # From pet_query + end + .. note:: This method returns a new ``QueryBuilder`` object which can be further @@ -526,7 +570,6 @@ The different clauses of the query are merged like this: ``sort``, ``query`` and ``aggregations``: These are merged, the result is a compound of the clauses in both objects. - Query clauses are merged with a boolean ``must`` query (equivalent to ``AND``) #union ------ 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 7d5f84e..6bc3d87 100644 --- a/spec/jay_api/elasticsearch/query_builder/query_clauses_spec.rb +++ b/spec/jay_api/elasticsearch/query_builder/query_clauses_spec.rb @@ -458,7 +458,6 @@ 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(:merge!).and_return(bool_clause) query_clauses.exists(field: 'build_job_name')