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.

Sign up for GitHub

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

Jump to bottom

KnnVectorQueryBuilder support for allowUnmappedFields #107047

Merged
merged 13 commits into from
Jul 10, 2024
Merged

KnnVectorQueryBuilder support for allowUnmappedFields #107047

merged 13 commits into from
Jul 10, 2024

Conversation

ghostspiders
Copy link
Contributor

KnnVectorQueryBuilder does not respect the index.query.parse.allow_unmapped_fields index setting. update KnnVectorQueryBuilder support for allowUnmappedFields.
#106846

@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label v8.14.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Apr 3, 2024
@gwbrown gwbrown added :Search Foundations/Mapping Index mappings, including merging and defining field types and removed needs:triage Requires assignment of a team area label labels Apr 8, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Search Meta label for search team label Apr 8, 2024
@benwtrent benwtrent self-assigned this Apr 9, 2024
@benwtrent benwtrent added the >bug label Apr 9, 2024
@benwtrent
Copy link
Member

I think the change looks good. Gonna get CI to test. Thank you @ghostspiders !

@benwtrent
Copy link
Member

buildkite test this please

@benwtrent
Copy link
Member

buildkite test this please

@benwtrent
Copy link
Member

@elasticmachine update branch

@benwtrent
Copy link
Member

buildkite test this please

@benwtrent
Copy link
Member

@ghostspiders

There are multiple failures around the yaml tests:

./gradlew ':qa:mixed-cluster:v8.13.3#mixedClusterTest' -Dtests.class="org.elasticsearch.backwards.MixedClusterClientYamlTestSuiteIT" -Dtests.method="test {p0=search.vectors/45_knn_search_byte/Test nonexistent field}" -Dtests.seed=A52D9437EA192750 -Dtests.bwc=true -Dtests.locale=it-IT -Dtests.timezone=Africa/Mbabane -Druntime.java=21

is one example.

I am thinking that those tests need to adjusted with a version skip to ensure that they are only ran on older versions where we would fail on a non-existent field directly.

@ghostspiders
Copy link
Contributor Author

ghostspiders commented Apr 16, 2024 •
edited
Loading

@benwtrent I passed this yaml tests
`./gradlew ':qa:mixed-cluster:v8.9.2#mixedClusterTest' -Dtests.class="org.elasticsearch.backwards.MixedClusterClientYamlTestSuiteIT" -Dtests.method="test {p0=search.vectors/45_knn_search_byte/Test nonexistent field}" -Dtests.bwc=true -Druntime.java=21

@benwtrent
Copy link
Member

buildkite test this please

@benwtrent
Copy link
Member

@elasticmachine update branch

@benwtrent
Copy link
Member

buildkite test this please

@benwtrent
Copy link
Member

buildkite test this please

@benwtrent
Copy link
Member

buildkite test this please

@benwtrent
Copy link
Member

@elasticmachine update branch

@benwtrent
Copy link
Member

buildkite test this please

@joegallo joegallo mentioned this pull request May 10, 2024
Closed
@elasticsearchmachine elasticsearchmachine added v8.16.0 Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch and removed v8.15.0 Team:Search Meta label for search team labels Jul 4, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-foundations (Team:Search Foundations)

@benwtrent
Copy link
Member

buildkite test this please

benwtrent
benwtrent reviewed Jul 8, 2024
View reviewed changes
Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

gotta update for 8.16

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.vectors/40_knn_search.yml Outdated Show resolved Hide resolved
rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.vectors/40_knn_search.yml Outdated Show resolved Hide resolved
...api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.vectors/45_knn_search_byte.yml Outdated Show resolved Hide resolved
...api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.vectors/45_knn_search_byte.yml Outdated Show resolved Hide resolved
@benwtrent
Copy link
Member

buildkite test this please

@benwtrent
Copy link
Member

buildkite test this please

@benwtrent
Copy link
Member

run elasticsearch-ci/part-3

@benwtrent benwtrent merged commit 3bd192c into elastic:main Jul 10, 2024
17 checks passed
@benwtrent benwtrent mentioned this pull request Jul 10, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benwtrent benwtrent benwtrent approved these changes

Assignees

@benwtrent benwtrent

Labels
>bug external-contributor Pull request authored by a developer outside the Elasticsearch team :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch v8.16.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ghostspiders @elasticsearchmachine @benwtrent @gwbrown @elasticmachine