milvus-io / milvus

A cloud-native vector database, storage for next generation AI applications
https://milvus.io
Apache License 2.0
31k stars 2.95k forks source link

[Bug]: range search get unclear error message #26503

Closed shanghaikid closed 1 year ago

shanghaikid commented 1 year ago

Is there an existing issue for this?

Environment

- Milvus version:
- Deployment mode(standalone or cluster):
- MQ type(rocksmq, pulsar or kafka):    
- SDK version(e.g. pymilvus v2.0.0rc2):
- OS(Ubuntu or CentOS): 
- CPU/Memory: 
- GPU: 
- Others:

Current Behavior

it(`Exec simple search with range filter should success`, async () => {
    const limit = 8;
    const res = await milvusClient.search({
      collection_name: COLLECTION_NAME,
      filter: 'height < 10000',
      vector: [1, 2, 3, 4],
      limit: limit,
      params: { nprobe: 1024, radius: 10, range_filter: 20 },
    });

    console.log('range ', res);

    expect(res.status.error_code).toEqual(ErrorCode.SUCCESS);
    res.results.forEach(r => {
      expect(Number(r.height)).toBeLessThan(10000);
    });
  });
range  {
      status: {
        error_code: 'UnexpectedError',
        reason: 'attempt #0: fail to Search, QueryNode ID=1, reason=worker(1) query failed: UnexpectedError: Assert "Assert "range_filter < radius" at /go/src/github.com/milvus-io/milvus/internal/core/src/common/RangeSearchHelper.cpp:126\n' +
          ' => range_filter must be less than radius for L2/HAMMING/JACCARD" at /go/src/github.com/milvus-io/milvus/internal/core/src/query/SearchBruteForce.cpp:103: channel=by-dev-rootcoord-dml_5_443700359105080880v0: fail to search/query shard leader: attempt #1: no available shard delegator found: service unavailable: fail to search on all shard leaders',
        code: 0
      },
      results: []
    }

I don't understand the error message.

=> range_filter must be less than radius for L2/HAMMING/JACCARD" what is the correct radius?

Expected Behavior

No response

Steps To Reproduce

No response

Milvus Log

No response

Anything else?

No response

yanliang567 commented 1 year ago

/assign @jiaoew1991 /unassign

jiaoew1991 commented 1 year ago

@shanghaikid For L2/HAMMING/JACCARD, the range_filter parameter needs to be smaller than the range. In this case, where range equals 10, the range_filter should be less than 10.

shanghaikid commented 1 year ago

@shanghaikid For L2/HAMMING/JACCARD, the range_filter parameter needs to be smaller than the range. In this case, where range equals 10, the range_filter should be less than 10.

I think it should return an empty result if the range_filter is not set correctly.

jiaoew1991 commented 1 year ago

@shanghaikid For L2/HAMMING/JACCARD, the range_filter parameter needs to be smaller than the range. In this case, where range equals 10, the range_filter should be less than 10.

I think it should return an empty result if the range_filter is not set correctly.

For L2/HAMMING/JACCARD, the range_filter should be smaller than the range. However, for other metrics, the range should be larger than the range_filter. If an empty value is returned, it can easily lead to misuse here. If an empty value is returned, users may mistakenly think that there is no problem with their parameters and that there is simply no data available.

shanghaikid commented 1 year ago

@shanghaikid For L2/HAMMING/JACCARD, the range_filter parameter needs to be smaller than the range. In this case, where range equals 10, the range_filter should be less than 10.

I think it should return an empty result if the range_filter is not set correctly.

For L2/HAMMING/JACCARD, the range_filter should be smaller than the range. However, for other metrics, the range should be larger than the range_filter. If an empty value is returned, it can easily lead to misuse here. If an empty value is returned, users may mistakenly think that there is no problem with their parameters and that there is simply no data available.

In database world, if you write a incorrect sql, it should return an error, if you write a correct sql but with a wrong where condition, it just return empty. should we follow this?

jiaoew1991 commented 1 year ago

it should be considered as a incorrect sql 🤔

shanghaikid commented 1 year ago

@xiaofan-luan @yanliang567 what do you guys think?

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Rotten issues close after 30d of inactivity. Reopen the issue with /reopen.