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

[BUG] Mapping accepts both model id and dimension #2318

Open
VijayanB opened this issue Dec 11, 2024 · 2 comments
Open

[BUG] Mapping accepts both model id and dimension #2318

VijayanB opened this issue Dec 11, 2024 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@VijayanB
Copy link
Member

VijayanB commented Dec 11, 2024

What is the bug?
I can create knn index with knn field type contains both model id and dimension.

How can one reproduce the bug?
Steps to reproduce the behavior:

 curl -XPUT "http://localhost:9200/my-knn-index-1" -H 'Content-Type: application/json' -d'                                                                                                                                                                                     
{              
  "settings": {
    "index": {      
      "knn":true     
    }                        
  },                                             
  "mappings": {    
    "properties": {          
      "my_vector1": {        
        "type": "knn_vector", "model_id": "1234",
        "dimension": 2         
      }}}}                                                                                                                                                                                                                                                                                  
{"acknowledged":true,"shards_acknowledged":true,"index":"my-knn-index-1"}%  

What is the expected behavior?
Should raise an exception that either model id or dimension should be provided. When model id is provided, k-NN refer dimension from training index instead of user defined dimension in mapping request body.

What is your host/environment?
N/A

Do you have any screenshots?
N/A

Do you have any additional context?
N/A

@VijayanB VijayanB added bug Something isn't working untriaged labels Dec 11, 2024
@VijayanB
Copy link
Member Author

While adding validation we should also add version check to make sure that validation is applicable only for indices that are created after validation check is added. Without version check, when index is upgraded from old version, it will not work as expected

@jmazanec15 jmazanec15 moved this from Backlog to Backlog (Hot) in Vector Search RoadMap Jan 8, 2025
@markwu-sde
Copy link
Contributor

markwu-sde commented Jan 17, 2025

I'll take this up. Running some initial changes on my local to make sure I have the right idea:

wumr@88665a07f65b ~ % curl -XPUT "http://localhost:9200/my-knn-index-1" -H 'Content-Type: application/json' -d'                                                                                                                                                                                    
{
  "settings": {
    "index": {
      "knn":true
    }
  },
  "mappings": {
    "properties": {
      "my_vector1": {
        "type": "knn_vector", "model_id": "1234",
        "dimension": 2
      }}}}'
{"error":{"root_cause":[{"type":"mapper_parsing_exception","reason":"Failed to parse mapping [_doc]: Dimension and model can not be both specified in the mapping"}],"type":"mapper_parsing_exception","reason":"Failed to parse mapping [_doc]: Dimension and model can not be both specified in the mapping","caused_by":{"type":"illegal_argument_exception","reason":"Dimension and model can not be both specified in the mapping"}},"status":400}%   

There's also a version check in close proximity to the validation that we can look into reusing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog (Hot)
Development

No branches or pull requests

3 participants