-
Notifications
You must be signed in to change notification settings - Fork 159
Adds nested knn search example #1718
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
Conversation
🔍 Preview links for changed docs: 🔔 The preview site may take up to 3 minutes to finish building. These links will become live once it completes. |
solutions/search/vector/knn.md
Outdated
} | ||
``` | ||
|
||
### Customize nested kNN search |
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'm wondering if it would be better to make this guide its own subpage. This is quite complex and may make the page significantly longer – something to consider.
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.
Good idea for a follow-up to rethink how we can break down this entire page into more manageable sub-pages —cc @kderusso
I'm pretty sure it just organically became one long append-only page because we didn't have sufficient nesting capacity in the old asciidoc system :)
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.
Thanks for your suggestions! I’ve created an issue to address this later and improve the readability of the page.
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.
Thanks for adding this document! I've added a few suggestions.
solutions/search/vector/knn.md
Outdated
|
||
### Customize nested kNN search | ||
|
||
You can combine nested kNN search with top-level dense vector fields, filters, and `inner_hits` to support more advanced use cases. |
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.
Reading the ticket closely, I wonder if we could simplify this a bit. I think the use case of "create both at once" is interesting but my take on reading this ticket is more, "we have examples of how to create regular fields but there are no good examples in the same document on how to use nested fields." It feels like only examples using nested
would provide more simple examples that could be used as building blocks. Since most people who are interested in using nested fields are doing so for chunking purposes, it may make their use case a bit more clear. WDYT?
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.
Thank you, I totally agree with your take! I updated the PR to focus only on the nested use case and removed the mixed example to keep it simpler and more relevant. What do you think about 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.
LGTM, thanks for iterating!
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.
👍
📸 Preview
This PR introduces a new example that demonstrates how to combine top-level and nested
dense_vector
fields in a single index.Related issue: https://github.com/elastic/developer-docs-team/issues/229