-
Notifications
You must be signed in to change notification settings - Fork 982
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding HumanReadableQuery with a descrition param, used for debugging print output #12816
Conversation
We use this only in KnnByte/FloatVectorQuery toString method so the benchmarker can disambiguate between different KnnFloatVectorQuery/KnnByteVectorQuery queries.
I'd rather like not to touch these queries, and introduce a brand new query that rewrites to a |
I much prefer @jpountz idea. This additional field is purely for debugging purposes. A |
Thanks for the suggestion @jpountz! I'll add a |
Created a |
lucene/core/src/java/org/apache/lucene/search/HumanReadableQuery.java
Outdated
Show resolved
Hide resolved
@Override | ||
public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) | ||
throws IOException { | ||
return in.createWeight(searcher, scoreMode, boost); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if a bonus here would be a wrapped weight that overrides explain and adds the human description to the output?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we keep it as simple as possible, return the inner query in #rewrite and throw an UnsupportedOperationException in #createWeight?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll make it as simple as possible for now, and if needed we can always extend it further.
Should we move it in |
Yes, that sounds like a better place for it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left minor comments but it looks good to me otherwise!
lucene/misc/src/java/org/apache/lucene/misc/search/HumanReadableQuery.java
Outdated
Show resolved
Hide resolved
lucene/misc/src/test/org/apache/lucene/misc/search/TestHumanReadableQuery.java
Outdated
Show resolved
Hide resolved
…leQuery.java Co-authored-by: Adrien Grand <jpountz@gmail.com>
Thanks for the feedback! Done the changes there. |
… move CHANGES.txt entry from 10.0 -> 9.9.0 on bulk backport of recent FST improvements
We use this only in KnnByte/FloatVectorQuery toString method so the benchmarker can disambiguate between different KnnFloatVectorQuery/KnnByteVectorQuery queries.
Closes #12487 and should enable VectorQuery fixes to the benchmarker in mikemccand/luceneutil#226. More details about the problem in the issues.