Repository navigation
Fixing the exclude query - #450
Conversation
gaurav
left a comment
gaurav
left a comment
There was a problem hiding this comment.
LGTM! I've made some minor suggestions for documentation and improving the assertion output, but feel free to ignore them if they're not useful.
| body = _build_all_query_builders()[builder_name] | ||
| must = body["query"]["bool"]["must"] | ||
| function_score = (must[0] if isinstance(must, list) else must)["function_score"] | ||
| should = function_score["query"]["bool"]["should"] |
There was a problem hiding this comment.
Is it really the case that:
must = body["query"]["bool"]["must"]
should = body["query"]["bool"]["must"]["function_score"]["query"]["bool"]["should"]
If so, it might be useful to add a comment explaining why there's a query instead a function score inside another query (I'm guessing the internal query calculates a score for the outer query?).
| "query": query, | ||
| "default_operator": "and", | ||
| "flags": "OR|AND|NOT|PHRASE|PREFIX" | ||
| "flags": "OR|AND|NOT|PHRASE|PREFIX|WHITESPACE" |
There was a problem hiding this comment.
It might be useful to link to the relevant documentation (https://www.elastic.co/docs/reference/query-languages/query-dsl/query-dsl-simple-query-string-query#supported-flags) unless that makes this code harder to read.
| for clause in should: | ||
| sqs = clause.get("simple_query_string") or clause["nested"]["query"]["simple_query_string"] | ||
| flags.add(sqs["flags"]) | ||
| self.assertEqual(len(flags), 1, "expected identical flags across should clauses") |
There was a problem hiding this comment.
I think it would be cleaner to assert identical flags across should clauses within the loop, so that the assertion error clearly indicates which clause e.g.
first_flags = set()
for clause in should:
...
flags = sqs["flags"]
self.assertTrue(flags, f"Empty flags found in clause {clause}")
if not first_flags:
first_flags = sqs["flags"]
self.assertEqual(sqs["flags"], first_flags, f"Flags in clause {clause} ({flags}) differ from expected flags {first_flags} found before it in the query {should}")If it is better to put that outside the clause loop, then I would change this write out the different flags we've seen:
| self.assertEqual(len(flags), 1, "expected identical flags across should clauses") | |
| self.assertEqual(len(flags), 1, f"expected identical flags across should clauses but got {len(flags)} combinations: {flags}") |
3 participants
No description provided.