Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`).

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
------

Expand Down Expand Up @@ -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
------------

Expand Down
20 changes: 20 additions & 0 deletions lib/jay_api/elasticsearch/query_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# A helper class to build simple and common queries for Elasticsearch.
# Queries are created with the Elasticsearch Query DSL:
# https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl.html
class QueryBuilder

Check warning on line 13 in lib/jay_api/elasticsearch/query_builder.rb

View workflow job for this annotation

GitHub Actions / lint

[rubocop] reported by reviewdog 🐶 Class has too many lines. [117/100] Raw Output: lib/jay_api/elasticsearch/query_builder.rb:13:5: R: Metrics/ClassLength: Class has too many lines. [117/100]
# @return [JayAPI::Elasticsearch::QueryBuilder::Aggregations]
attr_reader :aggregations

Expand Down Expand Up @@ -149,6 +149,26 @@
)
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
Expand Down
17 changes: 17 additions & 0 deletions lib/jay_api/elasticsearch/query_builder/query_clauses.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
75 changes: 75 additions & 0 deletions spec/integration/jay_api/elasticsearch/query_builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -470,4 +470,79 @@
end
end
end

describe '#intersection' do

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great to have some test cases to test union and intersection operations combined in a more sophisticated AND/OR clause

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I have added an integration test with such a case, it also servers as an illustration of one of those cases when one might want an explicit intersection instead of a regular merge.

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
72 changes: 72 additions & 0 deletions spec/jay_api/elasticsearch/query_builder/query_clauses_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,78 @@
end
end

describe '#intersection' do
Comment thread
sergio-bobillier marked this conversation as resolved.
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(
Expand Down
13 changes: 13 additions & 0 deletions spec/jay_api/elasticsearch/query_builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down
Loading