Skip to content

Commit

Permalink
Bugfix: avoid TypeError on not: {not: {anyOf: [...]}}.
Browse files Browse the repository at this point in the history
We have special handling for double nots to "pull up" the filter
clauses for greater efficiency. That handling didn't work right
when an `anyOf` was under the double `not`, and we got a `TypeError`:

```
TypeError: no implicit conversion of Integer into Array
  from elastic_graph/graphql/filtering/filter_interpreter.rb:121:in `concat'
  from elastic_graph/graphql/filtering/filter_interpreter.rb:121:in `block (2 levels) in process_not_expression'
  from elastic_graph/graphql/filtering/filter_interpreter.rb:121:in `each'
  from elastic_graph/graphql/filtering/filter_interpreter.rb:121:in `block in process_not_expression'
  from elastic_graph/graphql/filtering/filter_interpreter.rb:120:in `each'
  from elastic_graph/graphql/filtering/filter_interpreter.rb:120:in `process_not_expression'
  from elastic_graph/graphql/filtering/filter_interpreter.rb:66:in `block in process_filter_hash'
  from elastic_graph/graphql/filtering/filter_interpreter.rb:61:in `each'
  from elastic_graph/graphql/filtering/filter_interpreter.rb:61:in `process_filter_hash'
  from elastic_graph/graphql/filtering/filter_interpreter.rb:43:in `block (2 levels) in build_query'
  from set.rb:511:in `each_key'
  from set.rb:511:in `each'
  from elastic_graph/graphql/filtering/filter_interpreter.rb:42:in `block in build_query'
  from <internal:kernel>:90:in `tap'
  from elastic_graph/graphql/filtering/filter_interpreter.rb:337:in `build_bool_hash'
  from elastic_graph/graphql/filtering/filter_interpreter.rb:41:in `build_query'
  from elastic_graph/graphql/datastore_query.rb:318:in `to_datastore_body'
  from elastic_graph/graphql/datastore_query.rb:229:in `empty?'
  from elastic_graph/graphql/datastore_query.rb:94:in `each'
  from elastic_graph/graphql/datastore_query.rb:94:in `partition'
  from elastic_graph/graphql/datastore_query.rb:94:in `perform'
  from elastic_graph/graphql/datastore_search_router.rb:34:in `msearch'
  from elastic_graph/graphql/resolvers/query_source.rb:26:in `fetch'
  from graphql/dataloader/source.rb:136:in `run_pending_keys'
  from graphql/dataloader.rb:294:in `block (2 levels) in spawn_source_fiber'
  from graphql/dataloader.rb:292:in `each'
  from graphql/dataloader.rb:292:in `block in spawn_source_fiber'
  from graphql/dataloader.rb:245:in `block in spawn_fiber'
```

This fixes that case.
  • Loading branch information
myronmarston committed Feb 5, 2025
1 parent a073623 commit 60ee184
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,12 @@ def process_not_expression(bool_node, expression, field_path)
if sub_filter[:bool].key?(:must_not)
# Pull clauses up to current bool_node to remove negation
sub_filter[:bool][:must_not].each do |negated_clause|
negated_clause[:bool].each { |k, v| bool_node[k].concat(v) }
negated_clause_bool = negated_clause[:bool]
# `minimum_should_match` is not a list like the other clauses, and needs to be handled separately.
negated_clause_bool.except(:minimum_should_match).each { |k, v| bool_node[k].concat(v) }
if (min_should_match = negated_clause_bool[:minimum_should_match])
bool_node[:minimum_should_match] = min_should_match
end
end
end

Expand Down
47 changes: 36 additions & 11 deletions elasticgraph-graphql/spec/acceptance/search_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,18 +84,28 @@ module ElasticGraph
expect(list_widget_currencies(filter: {primary_continent: {equal_to_any_of: ["North America"]}})).to eq(unfiltered_widget_currencies)
expect_to_have_routed_to_shards_with("main", ["widget_currencies_rollover__*", "North America"])

filter = {
options: {
any_of: [
{size: {equal_to_any_of: [enum_value(:MEDIUM)]}},
{
size: {equal_to_any_of: [enum_value(:SMALL)]},
color: {equal_to_any_of: [enum_value(:BLUE)]}
}
]
options_any_of_filter = [
{size: {equal_to_any_of: [enum_value(:MEDIUM)]}},
{
size: {equal_to_any_of: [enum_value(:SMALL)]},
color: {equal_to_any_of: [enum_value(:BLUE)]}
}
}
widgets = list_widgets_with(<<~EOS, order_by: [:amount_cents_ASC], filter: filter)
]
widgets = list_widgets_with(<<~EOS, order_by: [:amount_cents_ASC], filter: {options: {any_of: options_any_of_filter}})
#{case_correctly("amount_cents")}
tags
fees {
currency
#{case_correctly("amount_cents")}
}
EOS

expect(widgets).to match([
string_hash_of(widget1, :id, :amount_cents, :tags, :fees),
string_hash_of(widget3, :id, :amount_cents, :tags, :fees)
])

widgets = list_widgets_with(<<~EOS, order_by: [:amount_cents_ASC], filter: {options: {not: {not: {any_of: options_any_of_filter}}}})
#{case_correctly("amount_cents")}
tags
fees {
Expand Down Expand Up @@ -163,6 +173,15 @@ module ElasticGraph
string_hash_of(widget3, :id, :amount_cents)
])

# Verify we can use the `not` filter operator correctly
widgets = list_widgets_with(:amount_cents,
filter: {options: {not: {not: {color: {equal_to_any_of: [enum_value(:BLUE)]}}}}},
order_by: [:amount_cents_ASC])

expect(widgets).to match([
string_hash_of(widget1, :id, :amount_cents)
])

widgets = list_widgets_with(:amount_cents,
filter: {options: {not: {color: {equal_to_any_of: [nil]}}}},
order_by: [:amount_cents_ASC])
Expand Down Expand Up @@ -320,6 +339,12 @@ module ElasticGraph
string_hash_of(widget1, :id, :amount_cents)
])

# Test `not: {not: {any_of: []}}` results in no matching documents
widgets = list_widgets_with(:amount_cents,
filter: {not: {not: {any_of: []}}})

expect(widgets).to match([])

# Test `not: {any_of: [emptyPredicate]}` results in no matching documents
widgets = list_widgets_with(:amount_cents,
filter: {not: {any_of: [{name: {equal_to_any_of: nil}}]}})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1340,6 +1340,20 @@ def expect_script_query_with_params(query, expected_params)

expect(body_for_inner_not).to eq(body_for_outer_not).and query_datastore_with(always_false_condition)
end

it "can double negate an `any_of`" do
inner_filter = {
"any_of" => [
{"age" => {"equal_to_any_of" => [25, 30]}},
{"height" => {"gt" => 10}}
]
}

query1 = new_query(filter: {"not" => {"not" => inner_filter}})
query2 = new_query(filter: inner_filter)

expect(datastore_body_of(query1)).to eq(datastore_body_of(query2))
end
end

describe "behavior of empty/null filter values" do
Expand Down

0 comments on commit 60ee184

Please sign in to comment.