Skip to content
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

Add explanation to runtime field query #63429

Merged
merged 3 commits into from
Oct 8, 2020

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Oct 7, 2020

This adds a detail to the output of explain for runtime fields
queries that makes it clear that their score is entirely based on their
boost. We don't have any tf/idf/norms/whatever to do any scoring - we
just score boost.

This adds a `detail` to the output of `explain` for runtime fields
queries that makes it clear that their score is entirely based on their
boost. We don't have any tf/idf/norms/whatever to do any scoring - we
just score `boost`.
@nik9000 nik9000 added >bug :Search/Search Search-related issues that do not fall into other categories v8.0.0 v7.11.0 labels Oct 7, 2020
@nik9000 nik9000 requested a review from javanna October 7, 2020 16:55
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Search)

@elasticmachine elasticmachine added the Team:Search Meta label for search team label Oct 7, 2020
- match: {hits.hits.0._explanation.details.0.value: 1.0}
- match: {hits.hits.0._explanation.details.0.description: 'runtime field query scoring boost'}
- match: {hits.hits.0._explanation.details.0.details: []}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went with fairly simple integration tests for this instead of unit tests because it felt easier. I could go either way, but I think this gets the job done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thats fine with me, maybe a small unit test for the lucene query would be also useful?

Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for working on this @nik9000 I left a couple of minor comments

day_of_week: Monday
- match: {hits.hits.0._explanation.description: day_of_week:Monday}
- match: {hits.hits.0._explanation.details.0.value: 1.0}
- match: {hits.hits.0._explanation.details.0.description: 'runtime field query scoring boost'}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am being picky about the description: the boost is provided with a query right? I am wondering if the boost itself has anything special, specifically related to runtime fields. It feels to me that the boost is just a boost, what is special is that with runtime fields it's the only factor in scoring calculation. What I am getting to is that we could make the detail even clearer, probably. Thinking out loud: should we have two items in details, one that is a constant, that explains that the field is a runtime fields, and the other one that is the actual query boost?

Maybe we should have a couple more tests around providing a boost with the query, and customizing the score with a script?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your proposal for the details makes sense to me. I'll make that change. I'll add a test with a boost certainly. What are you thinking as far as scripting changing the score? I think we usually do that by wrapping the query in a script score, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we usually do that by wrapping the query in a script score, right?

yes that is what I meant

- match: {hits.hits.0._explanation.details.0.value: 1.0}
- match: {hits.hits.0._explanation.details.0.description: 'runtime field query scoring boost'}
- match: {hits.hits.0._explanation.details.0.details: []}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thats fine with me, maybe a small unit test for the lucene query would be also useful?

@nik9000 nik9000 requested a review from javanna October 8, 2020 14:55
@nik9000 nik9000 merged commit 6dfb376 into elastic:master Oct 8, 2020
nik9000 added a commit to nik9000/elasticsearch that referenced this pull request Oct 8, 2020
This adds a `detail` to the output of `explain` for runtime fields
queries that makes it clear that their score is entirely based on their
boost. We don't have any tf/idf/norms/whatever to do any scoring - we
just score `boost`.
nik9000 added a commit that referenced this pull request Oct 8, 2020
This adds a `detail` to the output of `explain` for runtime fields
queries that makes it clear that their score is entirely based on their
boost. We don't have any tf/idf/norms/whatever to do any scoring - we
just score `boost`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v7.11.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants