apache / apache/lucene

MultiFieldQueryParser + per-field boosts produce wrong Occur under AND default operator

Open
#16,441 0 comments 0 reactions 0 assignees View on GitHub
type:bug
Dominant language
Java
Stars
3.6k
Forks
1.4k
Avg merge
2d 11h
Merged PRs (30d)
88

Description

### Description

[Edit corresponding pull request : #16442 ]

## Disclaimer

I found this bug in our codebase and manually wrote the unit tests covering the various cases. I then used Claude AI to help identify the root cause and draft this report. Finally, I verified both the reported behavior and the proposed patch against Lucene's main branch (running all tests) before submitting this report and corresponding merge request.

I could not find any policy in the [Contributing to Lucene Guide](https://github.com/apache/lucene/blob/main/CONTRIBUTING.md) preventing me from going this way.
My apologies if this is not authorized.

## Summary

When `MultiFieldQueryParser` is configured with per-field boosts and the default operator is set to `AND`, a single-term query whose text happens to be sent through the "multi-term" (whitespace-splitting) code path produces a query where **every field clause is forced to `MUST`**, even though the parser is expected to keep the per-field alternatives as `SHOULD` inside an optional group. Without boosts, the same input produces the expected (unforced) result.

**Correct behavior with "simple" term query (no boost):**
```
MultiFieldQueryParser parser = new MultiFieldQueryParser(new String[] {"field1", "field2"}, new StandardAnalyzer());
parser.setDefaultOperator(QueryParser.Operator.AND);
assertEquals("field1:hello field2:hello",
parser.parse(QueryParser.escape("hello !")).toString());
```

**Incorrect behavior with boosted query:**
```
MultiFieldQueryParser parser2 = new MultiFieldQueryParser(new String[] {"field1", "field2"}, new StandardAnalyzer(), Map.of("field1", 2.0f));
parser2.setDefaultOperator(QueryParser.Operator.AND);
assertEquals("field1:hello field2:hello",
parser2.parse(QueryParser.escape("hello !")).toString()); // KO : +(field1:hello)^2.0 +field2:hello
```

# Root cause

`QueryParser.escape("hello !")` escapes the `!`, so the query grammar sees two
adjacent `TERM` tokens (`hello`, `\!`) rather than a `NOT` operator. With
`splitOnWhitespace=false` (the default), this hits the `MultiTerm` production in
`QueryParser.jj`, which concatenates the tokens into a single string and calls:

```java
firstQuery = getFieldQuery(field, discardEscapeChar(text.image), false);
addMultiTermClauses(clauses, firstQuery);
```

`StandardAnalyzer` reduces `"hello !"` to the single token `hello` (the `!` is
punctuation and is dropped), so `MultiFieldQueryParser.getFieldQuery(null, "hello !", false)`
returns a flat `BooleanQuery` combining one `SHOULD` clause per field:

- **Without boost:** `BooleanQuery(SHOULD(TermQuery field1:hello), SHOULD(TermQuery field2:hello))`
- **With boost:** `BooleanQuery(SHOULD(BoostQuery(TermQuery field1:hello, 2.0)), SHOULD(TermQuery field2:hello))`

The bug is in `QueryParserBase#addMultiTermClauses`:

```java
protected void addMultiTermClauses(List clauses, Query q) {
if (q == null) return;
boolean allNestedTermQueries = false;
if (q instanceof BooleanQuery) {
allNestedTermQueries = true;
for (BooleanClause clause : ((BooleanQuery) q).clauses()) {
if (!(clause.getQuery() instanceof TermQuery)) {
allNestedTermQueries = false;
break;
}
}
}
if (allNestedTermQueries) {
clauses.addAll(((BooleanQuery) q).clauses()); // keeps original Occur
} else {
BooleanClause.Occur occur = operator == OR_OPERATOR ? SHOULD : MUST;
if (q instanceof BooleanQuery) {
for (BooleanClause clause : ((BooleanQuery) q).clauses()) {
clauses.add(newBooleanClause(clause.getQuery(), occur)); // overwrites Occur
}
} else {
clauses.add(newBooleanClause(q, occur));
}
}
}
```

The `allNestedTermQueries` heuristic is meant to detect "this `BooleanQuery` is just
the result of the analyzer splitting one field's text into several terms", in which
case the original per-clause `Occur` is preserved as-is. It uses a strict
`clause.getQuery() instanceof TermQuery` check.

- Without a boost, every clause is a bare `TermQuery` → the check passes → the
original `SHOULD` occurs are copied through untouched (this happens to look correct
here, though it also means the default operator is silently ignored for this path
in general — a separate, pre-existing quirk).
- With a boost, the `field1` clause becomes a `BoostQuery` wrapping a `TermQuery`, so
`instanceof TermQuery` is `false` → the check fails → the code falls into the `else`
branch. Because `q` is still a `BooleanQuery`, it is unwrapped and **every nested
clause is re-added with a single, forced `Occur`** (`MUST` here, since the default
operator is `AND`), discarding the `SHOULD` semantics that expressed "any of these
fields matching is enough". The result: two independently required clauses instead
of an optional group with boosts.

In short: `addMultiTermClauses` cannot distinguish "a nested `BooleanQuery` that
happens to contain something more than a bare `TermQuery` (e.g. a boosted term)" from
"a nested `BooleanQuery` that legitimately needs its per-clause structure discarded
and replaced by a single shared `Occur`" (e.g. synonym expansion). The multi-field
disjunction built by `MultiFieldQueryParser` falls victim to the latter path purely
because one of its clauses is boosted.

## Impact

Any use of `MultiFieldQueryParser` with per-field boosts, `AND` as the default operator, and query text that is analyzed down to a single term while going through the whitespace-splitting (`MultiTerm`) grammar path will silently produce an over-restrictive query (`MUST` on every field) instead of the intended optional/boosted disjunction. This can happen with escaped punctuation (as above), but also with any input where the analyzer collapses multiple raw tokens into one term (stop-word removal, synonym filtering that drops a token, etc.), so it is not limited to this specific escaping scenario.

### Version and environment details

Affected version: main and 10.4.0
Component: lucene-queryparser (classic package)
Affected classes:
- org.apache.lucene.queryparser.classic.MultiFieldQueryParser,
- org.apache.lucene.queryparser.classic.QueryParserBase

Contributor guide

Open the contributing guide

Research direction

Start with QueryParserBase#addMultiTermClauses and the MultiFieldQueryParser behavior described in the issue. Reproduce the boosted and unboosted examples under the AND default operator, then inspect the existing query-parser tests mentioned by the pull request. Done means boosted per-field clauses retain the intended optional structure and regression coverage passes.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
search
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.