You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "bjacobowitz (via GitHub)" <gi...@apache.org> on 2025/04/02 16:33:48 UTC
[I] Monitor TermFilteredPresearcher does not return stored query if it contains filter field [lucene]
bjacobowitz opened a new issue, #14427:
URL: https://github.com/apache/lucene/issues/14427
### Description
`TermFilteredPresearcher` may fail to return stored queries if those queries contain the filter field in the query itself (not just in the metadata).
When building the presearcher query ([here](https://github.com/apache/lucene/blob/ddef6177122d564bf2319fa72b3327ee53ad7019/lucene/monitor/src/java/org/apache/lucene/monitor/TermFilteredPresearcher.java#L99-L152)) , `TermFilteredPresearcher` removes filter field tokens from the main clause of the presearcher query ([via acceptor here](https://github.com/apache/lucene/blob/ddef6177122d564bf2319fa72b3327ee53ad7019/lucene/monitor/src/java/org/apache/lucene/monitor/TermFilteredPresearcher.java#L115-L130)) , presumably under the assumption that they will be added back to the query in a separate clause for filter fields, ANDed with the first clause ([here](https://github.com/apache/lucene/blob/ddef6177122d564bf2319fa72b3327ee53ad7019/lucene/monitor/src/java/org/apache/lucene/monitor/TermFilteredPresearcher.java#L138-L146)).
We end up with a presearcher query that is something like
```
+(some_other_field_in_query_index: value) #(+(my_filter_field:value))
```
However, if an indexed query contains that filter field, and if that field was the only indexed field for the query's associated document in the query index, the first of the AND'd clauses cannot match (because the filter field was omitted), so the overall AND'd presearcher query cannot match, and the presearcher fails to return the query.
A user can work around this by using an additional dedicated field for the filter field (i.e. adding it on both the query metadata and the document), but this seems like an _easy_ trap to fall into.
**My question here: is this intentional?** Is the idea of a "filter field" that it appears in documents and in MonitorQuery metadata but must not appear in a itself query? I'm aware of another issue about the intended behavior of Monitor filter fields (https://github.com/apache/lucene/issues/11607), so I'm unsure.
If intentional, I think we should document that more directly. If unintentional, we might consider removing the check on filter fields ([here](https://github.com/apache/lucene/blob/ddef6177122d564bf2319fa72b3327ee53ad7019/lucene/monitor/src/java/org/apache/lucene/monitor/TermFilteredPresearcher.java#L121)) when building the first part of the presearcher query.
I've set up a test project [here](https://github.com/bjacobowitz/sturdy-chainsaw/tree/main) to demonstrate the problem (specifically in [this test file](https://github.com/bjacobowitz/sturdy-chainsaw/blob/8e24641e2af3e9d5f19de31e78480048f19f5dd4/src/test/java/MonitorTest.java), where [`testWithEmptyMetadata`](https://github.com/bjacobowitz/sturdy-chainsaw/blob/8e24641e2af3e9d5f19de31e78480048f19f5dd4/src/test/java/MonitorTest.java#L22C10-L22C31) works but [`testWithMetadata`](https://github.com/bjacobowitz/sturdy-chainsaw/blob/8e24641e2af3e9d5f19de31e78480048f19f5dd4/src/test/java/MonitorTest.java#L52) fails).
### Version and environment details
Tested with Lucene 10.1.0 on macOS Sequoia 15.3.2 (but I think the problem has been around since at least Lucene version 8, if not earlier).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [I] Monitor TermFilteredPresearcher does not return stored query if it contains filter field [lucene]
Posted by "bjacobowitz (via GitHub)" <gi...@apache.org>.
bjacobowitz commented on issue #14427:
URL: https://github.com/apache/lucene/issues/14427#issuecomment-2851859048
On further reflection / investigation, I think updating the documentation is the way to go here.
I tried out permitting the filter fields on both sides of the presearcher query (in the terms clause and the filter clause) and that fixes the correctness issue, but it comes at a potentially unacceptable performance cost. See the attached patch [here](https://github.com/user-attachments/files/20041277/0001-Support-filter-fields-appearing-in-Lucene-Monitor-qu.patch) (or [this commit](https://github.com/bjacobowitz/lucene/commit/adf256bca6f0b3903f8d92ab9d844cbadac90257)) for the implementation I tried out.
With this (inadvisable) change, if we allow the filter field on both sides, we can end up with a presearcher query like this:
```
+((field:(test) language:(en)) __anytokenfield:__ANYTOKEN__) #(+(language:en))
```
This presearcher query would still match a stored query involving `field:test` and the filter field `language:en`, but by including the language on both sides the presearcher may return _any_ query that includes `language:en` for matching. That runs the risk of entirely subverting the optimization of only running queries involving `field:test`.
The magnitude of the performance cost here depends on how specific the filter field's values are. With a somewhat specific filter field, where relatively few queries share the same value, inadvertently running all queries with the same value incurs a relatively low cost, but in the case where the filter field is something common like `language`, where many queries will share the same value for the filter field, the cost of running those extra queries would be high, and is probably not acceptable.
There is a larger question around "correctness" when a filter field appears in the query itself, because the query could contradict the filter field's value from metadata! For example, if the query's filter field in metadata indicates "+language:en" and the query itself has a clause "-language:en", what is the correct behavior? Hard to say. Allowing that field to float freely in the stored query doesn't really make sense and should be avoided, but I wouldn't say this is obvious to the user.
I think it would be ideal to block the monitor from storing a query which contains a filter field in the query itself, to prevent the user from stumbling into errors like I did, but to do that we would potentially need to throw a new exception when storing queries and it would run the risk of breaking existing users with a new error at runtime, so I don't think it's such a clean option.
In the absence of a more direct solution here, I think we should update the documentation around filter fields, so that anyone newly learning about them will also learn that they should not appear in the query. I will open a PR to do this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org