apache / apache/lucene

Consider merging Query.rewrite() into Query.createWeight() [LUCENE-6785]

Open
#7,843 15 comments 0 reactions 0 assignees View on GitHub
legacy-jira-priority:Major type:enhancement
Dominant language
Java
Stars
3.6k
Forks
1.4k
Avg merge
2d 11h
Merged PRs (30d)
88

Description

Prompted by the discussion on #7648.

Query.rewrite() is a bit of an oddity. You call it to create a query for a specific IndexSearcher, and to ensure that you get a query implementation that has a working createWeight() method. However, Weight itself already encapsulates the notion of a per-searcher query.

You also need to repeatedly call rewrite() until the query has stopped rewriting itself, which is a bit trappy - there are a few places (in highlighting code for example) that just call rewrite() once, rather than looping round as IndexSearcher.rewrite() does. Most queries don't need to be called multiple times, however, so this seems a bit redundant. And the ones that do currently return un-rewritten queries can be changed simply enough to rewrite them.

Finally, in pretty much every case I can find in the codebase, rewrite() is called purely as a prelude to createWeight(). This means, in the case of for example large BooleanQueries, we end up cloning the whole query structure, only to throw it away immediately.

I'd like to try removing rewrite() entirely, and merging the logic into createWeight(), simplifying the API and removing the trap where code only calls rewrite once. What do people think?

---
Migrated from [LUCENE-6785](https://issues.apache.org/jira/browse/LUCENE-6785) by Alan Woodward (@romseygeek), updated Oct 01 2015
Attachments: [LUCENE-6785.patch](https://apache.github.io/lucene-jira-archive/attachments/LUCENE-6785/LUCENE-6785.patch) (versions: 2), [LUCENE-6785-alt.patch](https://apache.github.io/lucene-jira-archive/attachments/LUCENE-6785/LUCENE-6785-alt.patch)

Contributor guide

Open the contributing guide

Research direction

Start by reading Query.rewrite(), Query.createWeight(), and IndexSearcher.rewrite(), then inspect the highlighting call sites mentioned in the issue. Compare the discussion and attached patches before deciding on scope. Done means the rewrite logic is handled through createWeight(), callers no longer depend on repeated rewrite(), and the relevant tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
search
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.