Skip to content

Commit

Permalink
Add support for anyOf: [{field: null}, {field: {...}}] to return all …
Browse files Browse the repository at this point in the history
…documents
  • Loading branch information
acerbusace committed Nov 7, 2024
1 parent 8f9f4f9 commit 2bc1833
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,7 @@ def merge_into(bool_node)
bool_node[occurrence].concat(clauses)
end

# For `any_of: []` we need a way to force the datastore to match no documents, but
# I haven't found any sort of literal `false` we can pass in the compound expression
# or even a literal `1 = 0` as is sometimes used in SQL. Instead, we use this for that
# case.
empty_array = [] # : ::Array[untyped]
ALWAYS_FALSE_FILTER = filter({ids: {values: empty_array}})
ALWAYS_FALSE_FILTER = filter({match_none: {}})
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def process_filter_hash(bool_node, filter_hash, field_path)
filter_hash.each do |field_or_op, expression|
case filter_node_interpreter.identify_node_type(field_or_op, expression)
when :empty
# This is an "empty" filter predicate and we can ignore it.
# This is an "empty" filter predicate and can be treated as `true`.
when :not
process_not_expression(bool_node, expression, field_path)
when :list_any_filter
Expand Down Expand Up @@ -187,25 +187,32 @@ def process_any_satisfy_filter_expression_on_scalar_list(bool_node, filter, fiel
end
end

# We want to provide the following semantics for `any_of`:
#
# * `filter: {anyOf: []}` -> return no results
# * `filter: {anyOf: [{field: null}]}` -> return all results
# * `filter: {anyOf: [{field: null}, {field: ...}]}` -> return all results
def process_any_of_expression(bool_node, expressions, field_path)
return if expressions.nil? || expressions == {}

if expressions.empty?
# 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.
BooleanQuery::ALWAYS_FALSE_FILTER.merge_into(bool_node)
return
end

shoulds = expressions.filter_map do |expression|
build_bool_hash do |inner_bool_node|
process_filter_hash(inner_bool_node, expression, field_path)
end
end

# 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?
return if shoulds.size < expressions.size

BooleanQuery.should(*shoulds).merge_into(bool_node)
end

def process_all_of_expression(bool_node, expressions, field_path)
Expand Down Expand Up @@ -337,7 +344,7 @@ def process_list_count_expression(bool_node, expression, field_path)
def build_bool_hash(&block)
bool_node = Hash.new { |h, k| h[k] = [] }.tap(&block)

# To ignore "empty" filter predicates we need to return `nil` here.
# To threat "empty" filter predicates as `true` we need to return `nil` here.
return nil if bool_node.empty?

# According to https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-bool-query.html#bool-min-should-match,
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 @@ -634,6 +634,10 @@ module ElasticGraph
results = query_teams_with(filter: {any_of: [{forbes_valuations: nil}]})
expect(results).to eq [{"id" => "t1"}, {"id" => "t2"}, {"id" => "t3"}, {"id" => "t4"}]

# Verify `any_of: [{forbes_valuations: nil}]` returns all results.
results = query_teams_with(filter: {any_of: [{forbes_valuations: nil}, {id: {equal_to_any_of: ["t3"]}}]})
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 @@ -889,7 +889,7 @@ def search_datastore(**options, &before_msearch)
expect(results).to match_array(ids_of(widget2, widget4))
end

specify "`equal_to_any_of: []` or `any_of: []` matches no documents, but `any_predicate: nil` or `field: {}` is ignored, matching all documents" do
specify "`equal_to_any_of: []` or `any_of: []` matches no documents, but `any_predicate: nil` or `field: {}` is treated as `true`, matching all documents" do
index_into(
graphql,
widget1 = build(:widget),
Expand All @@ -898,6 +898,7 @@ def search_datastore(**options, &before_msearch)

expect(search_with_filter("id", "equal_to_any_of", [])).to eq []
expect(search_with_filter("id", "any_of", [])).to eq []
expect(search_with_filter("id", "any_of", [{"any_of" => []}])).to eq []

expect(search_with_filter("id", "equal_to_any_of", nil)).to eq ids_of(widget1, widget2)
expect(search_with_filter("amount_cents", "lt", nil)).to eq ids_of(widget1, widget2)
Expand All @@ -910,6 +911,21 @@ def search_datastore(**options, &before_msearch)
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)
expect(search_with_freeform_filter({"any_of" => [{"id" => nil}, {"amount_cents" => {"lt" => 0}}]})).to eq ids_of(widget1, widget2)
end

specify "`not: {any_of: []}` matches all documents, but `not: {any_of: [field: nil, ...]}` is treated as `false` matching no documents" do
index_into(
graphql,
widget1 = build(:widget, id: "one"),
widget2 = build(:widget, id: "two")
)

expect(search_with_freeform_filter({"not" => {"any_of" => []}})).to eq ids_of(widget1, widget2)

pending "`not` doesn't yet compose correctly with an `any_of: [{}, ...]`, so these currently fail"
expect(search_with_freeform_filter({"not" => {"any_of" => [{"id" => nil}]}})).to eq []
expect(search_with_freeform_filter({"not" => {"any_of" => [{"id" => nil}, {"amount_cents" => {"lt" => 0}}]}})).to eq []
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 @@ -1373,7 +1373,7 @@ def expect_script_query_with_params(query, expected_params)
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
it "returns an always false filter for `any_of: [{any_of: []}]`" do
query = new_query(filter: {
"any_of" => [{"any_of" => []}]
})
Expand Down Expand Up @@ -1403,6 +1403,14 @@ def expect_script_query_with_params(query, expected_params)
expect(datastore_body_of(query)).to not_filter_datastore_at_all
end

it "applies no filtering for an `any_of` composed of an empty predicate and non empty predicate" do
query = new_query(filter: {
"any_of" => [{"age" => {}}, {"equal_to_any_of" => [36]}]
})

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
query = new_query(filter: {
"age" => {"any_of" => nil}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,24 @@ class GraphQL

expect(parts).to target_all_widget_indices
end

it "excludes no indices when we have an `any_of: [{anyof: []}]` filter because that will match all results" do
parts = search_index_expression_parts_for({"any_of" => [{"any_of" => []}]})

expect(parts).to target_all_widget_indices
end

it "excludes no indices when we have an `any_of: [{field: nil}]` filter because that will match all results" do
parts = search_index_expression_parts_for({"any_of" => [{"created_at" => nil}]})

expect(parts).to target_all_widget_indices
end

it "excludes no indices when we have an `any_of: [{field: nil}, {...}]` filter because that will match all results" do
parts = search_index_expression_parts_for({"any_of" => [{"created_at" => nil}, {"id" => {"equal_to_any_of" => "some-id"}}]})

expect(parts).to target_all_widget_indices
end
end

context "for a query that includes aggregations" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,24 @@ def shard_routing_for(route_with_field_paths, filter_or_filters)
})).to search_all_shards
end

it "searches all shards when we have an `any_of: [{anyof: []}]` filter because that will match all results" do
expect(shard_routing_for(["name"], {
"any_of" => [{"any_of" => []}]
})).to search_all_shards
end

it "searches all shards when we have an `any_of: [{field: nil}]` filter because that will match all results" do
expect(shard_routing_for(["name"], {
"any_of" => [{"name" => nil}]
})).to search_all_shards
end

it "searches all shards when we have an `any_of: [{field: nil}, {...}]` filter because that will match all results" do
expect(shard_routing_for(["name"], {
"any_of" => [{"name" => nil}, {"id" => {"equal_to_any_of" => ["abc"]}}]
})).to search_all_shards
end

describe "not" do
it "searches all shards when there are values in an `equal_to_any_of` filter" do
expect(shard_routing_for(["name"],
Expand Down

0 comments on commit 2bc1833

Please sign in to comment.