Skip to content

Commit

Permalink
Change anyOf behaviour to evaluate to true for empty predicates
Browse files Browse the repository at this point in the history
  • Loading branch information
acerbusace committed Oct 29, 2024
1 parent f2c5bf4 commit b935acd
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -195,12 +195,16 @@ def process_any_of_expression(bool_node, expressions, field_path)
end
end

# When our `shoulds` array is empty, the filtering semantics we want is to match no documents.
# However, that's not the behavior the datastore will give us if we have an empty array in the
# query under `should`. To get the behavior we want, we need to pass the datastore some filter
# criteria that will evaluate to false for every document.
bool_query = shoulds.empty? ? BooleanQuery::ALWAYS_FALSE_FILTER : BooleanQuery.should(*shoulds)
bool_query.merge_into(bool_node)
# We want to match the following spemantics for `any_of`:
# * filter: {anyOf: []} -> return no results
# * filter: {anyOf: [{field: null}]} -> return all results
#
# Hence, when our `expressions` array is empty, we want to match no documents. However, that's
# not the behavior the datastore will give us if we have an empty array in the query under
# `should`. To get the behavior we want, we need to pass the datastore some filter criteria
# that will evaluate to false for every document.
bool_query = expressions.empty? ? BooleanQuery::ALWAYS_FALSE_FILTER : BooleanQuery.should(*shoulds)
bool_query.merge_into(bool_node) if expressions.empty? || !shoulds.empty?
end

def process_all_of_expression(bool_node, expressions, field_path)
Expand Down
4 changes: 4 additions & 0 deletions elasticgraph-graphql/spec/acceptance/search_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,10 @@ module ElasticGraph
}}}})
expect(results).to eq [{"id" => "t1"}, {"id" => "t2"}, {"id" => "t3"}]

# Verify `any_of: [{forbes_valuations: nil}]` returns all results.
results = query_teams_with(filter: {any_of: [{forbes_valuations: nil}]})
expect(results).to eq [{"id" => "t1"}, {"id" => "t2"}, {"id" => "t3"}, {"id" => "t4"}]

# Verify we can use `any_of` directly under `any_satisfy` on a nested field.
results = query_teams_with(filter: {current_players_nested: {any_satisfy: {any_of: [
{name: {equal_to_any_of: ["Johnny Rocket"]}},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -909,6 +909,7 @@ def search_datastore(**options, &before_msearch)
expect(search_with_filter("name_text", "matches_query", nil)).to eq ids_of(widget1, widget2)
expect(search_with_filter("name_text", "matches_phrase", nil)).to eq ids_of(widget1, widget2)
expect(search_with_freeform_filter({"id" => {}})).to eq ids_of(widget1, widget2)
expect(search_with_freeform_filter({"any_of" => [{"id" => nil}]})).to eq ids_of(widget1, widget2)
end

it "`equal_to_any_of:` with `nil` matches documents with null values or not null values" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1359,6 +1359,24 @@ def expect_script_query_with_params(query, expected_params)
expect(datastore_body_of(query)).to filter_datastore_with(terms: {"name" => []})
end

it "returns the standard always false filter for `any_of: []`" do
query = new_query(filter: {
"any_of" => []
})

expect(datastore_body_of(query)).to query_datastore_with(always_false_condition)
end

it "returns the standard always false filter for `any_of: [{any_of: []}]`" do
query = new_query(filter: {
"any_of" => [{"any_of" => []}]
})

expect(datastore_body_of(query)).to query_datastore_with({bool: {minimum_should_match: 1, should: [
always_false_condition
]}})
end

it "does not prune out `any_of: []` to be consistent with `equal_to_any_of: []`, instead providing an 'always false' condition to achieve the same behavior" do
query = new_query(filter: {
"age" => {"gt" => 18},
Expand All @@ -1371,12 +1389,12 @@ def expect_script_query_with_params(query, expected_params)
]})
end

it "reduces an `any_of` composed entirely of empty predicates to a false condition" do
it "applies no filtering to an `any_of` composed entirely of empty predicates" do
query = new_query(filter: {
"age" => {"any_of" => [{"gt" => nil}, {"lt" => nil}]}
})

expect(datastore_body_of(query)).to filter_datastore_with(always_false_condition)
expect(datastore_body_of(query)).to not_filter_datastore_at_all
end

it "does not filter at all when given only `any_of: nil` on a root field" do
Expand Down

0 comments on commit b935acd

Please sign in to comment.