Skip to content

Issue #2284: Added Integration Tests for mixed vector documents #2818

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
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Issue #2284: Added Integration Tests for mixed vector documents #2818

wants to merge 2 commits into from

Conversation

night-owl-1709
Copy link
Contributor

@night-owl-1709 night-owl-1709 commented Aug 3, 2025 •
edited
Loading

Description

This PR adds comprehensive integration tests to validate OpenSearch's ability to handle documents with mixed vector and non-vector fields across multiple engines (FAISS, Lucene) and query types (k-NN, script scoring, filters, etc). The implementation focuses on functional validation and performance testing with optimised test code and robust error handling.

Components:

  • MixedVectorDocumentIT.java: Comprehensive functional and performance validations with enhanced error handling and logging

MixedVectorDocumentIT.java – Integration Tests

Feature/Test Description
Mixed Document Handling Inserts both vector and non-vector docs; ensures proper query handling with unique document IDs
k-NN Search Validates results only from vector-bearing docs across engines with comprehensive result validation
Script Score Query Uses cosine similarity in script scoring with performance validation and proper error handling
Filtered k-NN Combines term filters with vector scoring queries with category-based validation
Exists Query Ensures only documents with vector field are counted with precise count validation
Terms Aggregation Checks categorical aggregation from vector-indexed documents with error handling
_forcemerge Ensures correct behaviour after segments are merged with comprehensive validation
Engine Variants Runs across FAISS and Lucene engines with engine-optimised HNSW parameters
Space Types Validates L2 and Cosine Similarity (excludes Inner Product for Lucene compatibility)
Disk-Based Index FAISS tests include force-merge optimisation with proper settings management
Batch Processing Efficient batch document insertion with unique ID ranges to prevent conflicts

Query Types Covered

  • knn queries with multiple k values and result validation
  • script_score with cosine similarity and score verification
  • exists field validation with count assertions
  • term + bool filtered queries with category validation
  • terms_aggregation on categorical fields with proper error handling

Engines & Configuration

Engine Space Types Notes
FAISS L2, Cosine Dense vector, disk-based, force-merge testing with optimised parameters
Lucene L2, Cosine Native vector scoring with memory optimisation (excludes Inner Product for compatibility)

Related Issues

Implements #2284

Check List

  • New functionality includes testing
  • New functionality has been documented
  • API changes companion pull request (N/A - test-only changes)
  • Commits are signed per the DCO using --signoff
  • CHANGELOG.md updated with test additions
  • Public documentation issue/PR (N/A - internal testing improvements)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

For more information on following the Developer Certificate of Origin and signing off your commits, please check here.

@kotwanikunal
Copy link
Member

@night-owl-1709 Thanks for adding in these tests. Looks like there are some CI failures which are related to the new tests: https://productionresultssa3.blob.core.windows.net/actions-results/9b41f7c3-9b55-49d9-a3bd-a68e10de9b41/workflow-job-run-6fdb6202-e42f-5733-908b-057e4d234ac3/logs/job/job-logs.txt?rsct=text%2Fplain&se=2025-08-04T22%3A37%3A18Z&sig=EfPWjtqDwcsUulX2%2BV740umx8BRBBziXfcDhjBkBdN8%3D&ske=2025-08-05T09%3A59%3A04Z&skoid=ca7593d4-ee42-46cd-af88-8b886a2f84eb&sks=b&skt=2025-08-04T21%3A59%3A04Z&sktid=398a6654-997b-47e9-b12b-9515b896b4de&skv=2025-05-05&sp=r&spr=https&sr=b&st=2025-08-04T22%3A27%3A13Z&sv=2025-05-05

Search for REPRODUCE with in the above logs to help with repro steps.

@night-owl-1709 night-owl-1709 force-pushed the mixedvectorITFIx branch 2 times, most recently from c2fced7 to 2dc3ebc Compare August 5, 2025 13:02
@night-owl-1709 night-owl-1709 changed the title Issue #2284: Added IT and BWC tests for mixed vector documents Issue #2284: Added Integration Tests for mixed vector documents Aug 5, 2025
kotwanikunal
kotwanikunal approved these changes Aug 6, 2025
View reviewed changes
Copy link
Member

@kotwanikunal kotwanikunal left a comment

Choose a reason for hiding this comment

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

LGTM.

navneet1v
navneet1v previously approved these changes Aug 7, 2025
View reviewed changes
@navneet1v
Copy link
Collaborator

@night-owl-1709 can you please resolve the conflicts

@night-owl-1709 night-owl-1709 force-pushed the mixedvectorITFIx branch 3 times, most recently from 39bce47 to dcca8a7 Compare August 10, 2025 16:16
@night-owl-1709
Copy link
Contributor Author

@night-owl-1709 can you please resolve the conflicts

@navneet1v done

navneet1v
navneet1v reviewed Aug 11, 2025
View reviewed changes
@night-owl-1709 night-owl-1709 force-pushed the mixedvectorITFIx branch 2 times, most recently from 2d6344e to 9b0a3b0 Compare August 12, 2025 04:37
@night-owl-1709
Copy link
Contributor Author

@navneet1v addressed your comment

shatejas
shatejas approved these changes Aug 20, 2025
View reviewed changes
Copy link
Collaborator

@shatejas shatejas left a comment

Choose a reason for hiding this comment

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

Looks good overall

src/test/java/org/opensearch/knn/integ/MixedVectorDocumentIT.java
assertTrue("Should return search results for " + indexName, response.contains(HITS_PATTERN));

// Verify non-vector document (doc 10) is not in k-NN results
assertFalse("Non-vector document should not appear in k-NN results for " + indexName, response.contains("\"_id\":\"10\""));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Checking just one document is a bit of a weak assert, you might want to collect ids in a set and find the difference to always be empty.

@navneet1v
Copy link
Collaborator

@night-owl-1709 can we fix comments from @shatejas and also rebase the code.

navneet1v
navneet1v reviewed Aug 27, 2025
View reviewed changes
src/test/java/org/opensearch/knn/integ/MixedVectorDocumentIT.java
);
}

private void addDocument(String indexName, int docId, XContentBuilder doc) throws IOException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be in the base class

src/test/java/org/opensearch/knn/integ/MixedVectorDocumentIT.java
);
}

private Settings createIndexSettings(KNNEngine engine, SpaceType spaceType, boolean diskBased) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

see if this in the base class or not

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@navneet1v navneet1v navneet1v left review comments

@shatejas shatejas shatejas approved these changes

@heemin32 heemin32 Awaiting requested review from heemin32 heemin32 is a code owner

@VijayanB VijayanB Awaiting requested review from VijayanB VijayanB is a code owner

@vamshin vamshin Awaiting requested review from vamshin vamshin is a code owner

@jmazanec15 jmazanec15 Awaiting requested review from jmazanec15 jmazanec15 is a code owner

@naveentatikonda naveentatikonda Awaiting requested review from naveentatikonda naveentatikonda is a code owner

@junqiu-lei junqiu-lei Awaiting requested review from junqiu-lei junqiu-lei is a code owner

@martin-gaievski martin-gaievski Awaiting requested review from martin-gaievski martin-gaievski is a code owner

@ryanbogan ryanbogan Awaiting requested review from ryanbogan ryanbogan is a code owner

@luyuncheng luyuncheng Awaiting requested review from luyuncheng luyuncheng is a code owner

@0ctopus13prime 0ctopus13prime Awaiting requested review from 0ctopus13prime 0ctopus13prime is a code owner

@Vikasht34 Vikasht34 Awaiting requested review from Vikasht34 Vikasht34 is a code owner

+1 more reviewer

@kotwanikunal kotwanikunal kotwanikunal approved these changes

Reviewers whose approvals may not affect merge requirements

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@night-owl-1709 @kotwanikunal @navneet1v @shatejas