Skip to content

Keep nested bool clauses scoped when converting OpenSearch queries - #155

Open
syahra712 wants to merge 2 commits into
apache:mainfrom
syahra712:fix/nested-bool-query-scope
Open

syahra712 wants to merge 2 commits into
apache:mainfrom
syahra712:fix/nested-bool-query-scope

Conversation

@syahra712

Copy link
Copy Markdown
Contributor

convert-workload translates OpenSearch search bodies with translate_to_solr_json_dsl. When a bool is nested inside another bool, it loses its scope in two ways.

1. Nested filters are hoisted to fq from anywhere. fq_list is passed down into must_not and should clauses, so a filter deep inside them becomes a top-level, required fq:

translate_to_solr_json_dsl({"query": {"bool": {"must_not": [{"bool": {"filter": [{"term": {"status": "deleted"}}]}}]}}})
# {'query': '*:*', 'filter': ['status:deleted']}      <- returns only the excluded documents

translate_to_solr_json_dsl({"query": {"bool": {"should": [
    {"bool": {"filter": [{"term": {"color": "red"}}]}},
    {"bool": {"filter": [{"term": {"color": "blue"}}]}}]}}})
# {'query': '*:*', 'filter': ['color:red', 'color:blue']}   <- red AND blue instead of red OR blue

2. A nested bool under should is not grouped. Its clauses are joined straight into the should group, so its + clauses become required for the whole query:

translate_to_solr_json_dsl({"query": {"bool": {"should": [
    {"bool": {"must": [{"term": {"a": 1}}, {"term": {"b": 2}}]}},
    {"term": {"c": 3}}]}}})
# {'query': '(+(a:1) +(b:2) c:3)'}     <- a AND b, instead of (a AND b) OR c

The converted workload then benchmarks a different query, often one that matches nothing or the wrong documents.

Fix: filters are only hoisted to fq under must, where they are required anyway. Under must_not and should they stay inline. A nested bool under should is wrapped in parentheses. The examples above now give -(+(status:deleted)), ((+(color:red)) (+(color:blue))) and ((+(a:1) +(b:2)) c:3). Hoisting under must is unchanged.

Tested: four tests in test_workload_converter.py. Three fail on main, and the fourth checks that hoisting under must still works. ruff check is clean and the full unit suite passes (1184 passed, 5 skipped).

Filters nested under must_not or should were hoisted to top-level fq,
which inverted must_not and turned should alternatives into a
conjunction. A nested bool under should was also spliced in without
parentheses, so its required clauses became required for the whole
query. Filters are now only hoisted under must, and a nested bool
under should is grouped.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant