Skip to content

Commit 7b5eb85

Browse files
cydraincydrain
authored
Remove redundant parameter for segcore Search() (milvus-io#19025)
Signed-off-by: yudong.cai <yudong.cai@zilliz.com> Signed-off-by: yudong.cai <yudong.cai@zilliz.com>
1 parent 765907a commit 7b5eb85

4 files changed

Lines changed: 53 additions & 68 deletions

File tree

‎internal/core/src/segcore/segment_c.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,7 @@ Search(CSegmentInterface c_segment,
6565
CSearchPlan c_plan,
6666
CPlaceholderGroup c_placeholder_group,
6767
uint64_t timestamp,
68-
CSearchResult* result,
69-
int64_t segment_id) {
68+
CSearchResult* result) {
7069
try {
7170
auto segment = (milvus::segcore::SegmentInterface*)c_segment;
7271
auto plan = (milvus::query::Plan*)c_plan;

‎internal/core/src/segcore/segment_c.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,7 @@ Search(CSegmentInterface c_segment,
4242
CSearchPlan c_plan,
4343
CPlaceholderGroup c_placeholder_group,
4444
uint64_t timestamp,
45-
CSearchResult* result,
46-
int64_t segment_id);
45+
CSearchResult* result);
4746

4847
void
4948
DeleteRetrieveResult(CRetrieveResult* retrieve_result);

‎internal/core/unittest/test_c_api.cpp‎

Lines changed: 50 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -824,11 +824,11 @@ TEST(CApiTest, SearchTest) {
824824
placeholderGroups.push_back(placeholderGroup);
825825

826826
CSearchResult search_result;
827-
auto res = Search(segment, plan, placeholderGroup, N + ts_offset, &search_result, -1);
827+
auto res = Search(segment, plan, placeholderGroup, N + ts_offset, &search_result);
828828
ASSERT_EQ(res.error_code, Success);
829829

830830
CSearchResult search_result2;
831-
auto res2 = Search(segment, plan, placeholderGroup, ts_offset, &search_result2, -1);
831+
auto res2 = Search(segment, plan, placeholderGroup, ts_offset, &search_result2);
832832
ASSERT_EQ(res2.error_code, Success);
833833

834834
DeleteSearchPlan(plan);
@@ -883,7 +883,7 @@ TEST(CApiTest, SearchTestWithExpr) {
883883
dataset.timestamps_.push_back(1);
884884

885885
CSearchResult search_result;
886-
auto res = Search(segment, plan, placeholderGroup, dataset.timestamps_[0], &search_result, -1);
886+
auto res = Search(segment, plan, placeholderGroup, dataset.timestamps_[0], &search_result);
887887
ASSERT_EQ(res.error_code, Success);
888888

889889
DeleteSearchPlan(plan);
@@ -1055,36 +1055,23 @@ TEST(CApiTest, GetRealCount) {
10551055

10561056
void
10571057
CheckSearchResultDuplicate(const std::vector<CSearchResult>& results) {
1058-
auto sr = (SearchResult*)results[0];
1059-
auto topk = sr->unity_topK_;
1060-
auto num_queries = sr->total_nq_;
1061-
1062-
// fill primary keys
1063-
std::vector<PkType> result_pks(num_queries * topk);
1064-
for (int i = 0; i < results.size(); i++) {
1065-
auto search_result = (SearchResult*)results[i];
1066-
auto size = search_result->result_offsets_.size();
1067-
if (size == 0) {
1068-
continue;
1069-
}
1070-
for (int j = 0; j < size; j++) {
1071-
auto offset = search_result->result_offsets_[j];
1072-
result_pks[offset] = search_result->primary_keys_[j];
1058+
auto nq = ((SearchResult*)results[0])->total_nq_;
1059+
1060+
std::unordered_set<PkType> pk_set;
1061+
for (int qi = 0; qi < nq; qi++) {
1062+
pk_set.clear();
1063+
for (int i = 0; i < results.size(); i++) {
1064+
auto search_result = (SearchResult*)results[i];
1065+
ASSERT_EQ(nq, search_result->total_nq_);
1066+
auto topk_beg = search_result->topk_per_nq_prefix_sum_[qi];
1067+
auto topk_end = search_result->topk_per_nq_prefix_sum_[qi + 1];
1068+
for (int ki = topk_beg; ki < topk_end; ki++) {
1069+
ASSERT_NE(search_result->seg_offsets_[ki], INVALID_SEG_OFFSET);
1070+
auto ret = pk_set.insert(search_result->primary_keys_[ki]);
1071+
ASSERT_TRUE(ret.second);
1072+
}
10731073
}
10741074
}
1075-
1076-
// check primary key duplicates
1077-
// int64_t cnt = 0;
1078-
// std::unordered_set<PkType> pk_set;
1079-
// for (int qi = 0; qi < num_queries; qi++) {
1080-
// pk_set.clear();
1081-
// for (int k = 0; k < topk; k++) {
1082-
// int64_t idx = topk * qi + k;
1083-
// pk_set.insert(result_pks[idx]);
1084-
// }
1085-
// cnt += pk_set.size();
1086-
// }
1087-
// assert(cnt == topk * num_queries);
10881075
}
10891076

10901077
TEST(CApiTest, ReudceNullResult) {
@@ -1141,7 +1128,7 @@ TEST(CApiTest, ReudceNullResult) {
11411128
auto slice_topKs = std::vector<int64_t>{1};
11421129
std::vector<CSearchResult> results;
11431130
CSearchResult res;
1144-
status = Search(segment, plan, placeholderGroup, dataset.timestamps_[0], &res, -1);
1131+
status = Search(segment, plan, placeholderGroup, dataset.timestamps_[0], &res);
11451132
assert(status.error_code == Success);
11461133
results.push_back(res);
11471134
CSearchResultDataBlobs cSearchResultData;
@@ -1218,9 +1205,9 @@ TEST(CApiTest, ReduceRemoveDuplicates) {
12181205
auto slice_topKs = std::vector<int64_t>{topK / 2, topK};
12191206
std::vector<CSearchResult> results;
12201207
CSearchResult res1, res2;
1221-
status = Search(segment, plan, placeholderGroup, dataset.timestamps_[0], &res1, -1);
1208+
status = Search(segment, plan, placeholderGroup, dataset.timestamps_[0], &res1);
12221209
assert(status.error_code == Success);
1223-
status = Search(segment, plan, placeholderGroup, dataset.timestamps_[0], &res2, -1);
1210+
status = Search(segment, plan, placeholderGroup, dataset.timestamps_[0], &res2);
12241211
assert(status.error_code == Success);
12251212
results.push_back(res1);
12261213
results.push_back(res2);
@@ -1243,11 +1230,11 @@ TEST(CApiTest, ReduceRemoveDuplicates) {
12431230
auto slice_topKs = std::vector<int64_t>{topK / 2, topK, topK};
12441231
std::vector<CSearchResult> results;
12451232
CSearchResult res1, res2, res3;
1246-
status = Search(segment, plan, placeholderGroup, dataset.timestamps_[0], &res1, -1);
1233+
status = Search(segment, plan, placeholderGroup, dataset.timestamps_[0], &res1);
12471234
assert(status.error_code == Success);
1248-
status = Search(segment, plan, placeholderGroup, dataset.timestamps_[0], &res2, -1);
1235+
status = Search(segment, plan, placeholderGroup, dataset.timestamps_[0], &res2);
12491236
assert(status.error_code == Success);
1250-
status = Search(segment, plan, placeholderGroup, dataset.timestamps_[0], &res3, -1);
1237+
status = Search(segment, plan, placeholderGroup, dataset.timestamps_[0], &res3);
12511238
assert(status.error_code == Success);
12521239
results.push_back(res1);
12531240
results.push_back(res2);
@@ -1317,9 +1304,9 @@ testReduceSearchWithExpr(int N, int topK, int num_queries) {
13171304
std::vector<CSearchResult> results;
13181305
CSearchResult res1;
13191306
CSearchResult res2;
1320-
auto res = Search(segment, plan, placeholderGroup, dataset.timestamps_[N - 1], &res1, -1);
1307+
auto res = Search(segment, plan, placeholderGroup, dataset.timestamps_[N - 1], &res1);
13211308
assert(res.error_code == Success);
1322-
res = Search(segment, plan, placeholderGroup, dataset.timestamps_[N - 1], &res2, -1);
1309+
res = Search(segment, plan, placeholderGroup, dataset.timestamps_[N - 1], &res2);
13231310
assert(res.error_code == Success);
13241311
results.push_back(res1);
13251312
results.push_back(res2);
@@ -1525,7 +1512,7 @@ TEST(CApiTest, Indexing_Without_Predicate) {
15251512
Timestamp time = 10000000;
15261513

15271514
CSearchResult c_search_result_on_smallIndex;
1528-
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex, -1);
1515+
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex);
15291516
assert(res_before_load_index.error_code == Success);
15301517

15311518
// load index to segment
@@ -1578,7 +1565,7 @@ TEST(CApiTest, Indexing_Without_Predicate) {
15781565

15791566
CSearchResult c_search_result_on_bigIndex;
15801567
auto res_after_load_index =
1581-
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex, -1);
1568+
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex);
15821569
assert(res_after_load_index.error_code == Success);
15831570

15841571
auto search_result_on_raw_index_json = SearchResultToJson(*search_result_on_raw_index);
@@ -1650,7 +1637,7 @@ TEST(CApiTest, Indexing_Expr_Without_Predicate) {
16501637
Timestamp time = 10000000;
16511638

16521639
CSearchResult c_search_result_on_smallIndex;
1653-
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex, -1);
1640+
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex);
16541641
assert(res_before_load_index.error_code == Success);
16551642

16561643
// load index to segment
@@ -1703,7 +1690,7 @@ TEST(CApiTest, Indexing_Expr_Without_Predicate) {
17031690

17041691
CSearchResult c_search_result_on_bigIndex;
17051692
auto res_after_load_index =
1706-
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex, -1);
1693+
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex);
17071694
assert(res_after_load_index.error_code == Success);
17081695

17091696
auto search_result_on_raw_index_json = SearchResultToJson(*search_result_on_raw_index);
@@ -1792,7 +1779,7 @@ TEST(CApiTest, Indexing_With_float_Predicate_Range) {
17921779
Timestamp time = 10000000;
17931780

17941781
CSearchResult c_search_result_on_smallIndex;
1795-
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex, -1);
1782+
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex);
17961783
assert(res_before_load_index.error_code == Success);
17971784

17981785
// load index to segment
@@ -1846,7 +1833,7 @@ TEST(CApiTest, Indexing_With_float_Predicate_Range) {
18461833

18471834
CSearchResult c_search_result_on_bigIndex;
18481835
auto res_after_load_index =
1849-
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex, -1);
1836+
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex);
18501837
assert(res_after_load_index.error_code == Success);
18511838

18521839
auto search_result_on_bigIndex = (SearchResult*)c_search_result_on_bigIndex;
@@ -1949,7 +1936,7 @@ TEST(CApiTest, Indexing_Expr_With_float_Predicate_Range) {
19491936
Timestamp time = 10000000;
19501937

19511938
CSearchResult c_search_result_on_smallIndex;
1952-
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex, -1);
1939+
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex);
19531940
assert(res_before_load_index.error_code == Success);
19541941

19551942
// load index to segment
@@ -2003,7 +1990,7 @@ TEST(CApiTest, Indexing_Expr_With_float_Predicate_Range) {
20031990

20041991
CSearchResult c_search_result_on_bigIndex;
20051992
auto res_after_load_index =
2006-
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex, -1);
1993+
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex);
20071994
assert(res_after_load_index.error_code == Success);
20081995

20091996
auto search_result_on_bigIndex = (SearchResult*)c_search_result_on_bigIndex;
@@ -2090,7 +2077,7 @@ TEST(CApiTest, Indexing_With_float_Predicate_Term) {
20902077
Timestamp time = 10000000;
20912078

20922079
CSearchResult c_search_result_on_smallIndex;
2093-
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex, -1);
2080+
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex);
20942081
assert(res_before_load_index.error_code == Success);
20952082

20962083
// load index to segment
@@ -2144,7 +2131,7 @@ TEST(CApiTest, Indexing_With_float_Predicate_Term) {
21442131

21452132
CSearchResult c_search_result_on_bigIndex;
21462133
auto res_after_load_index =
2147-
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex, -1);
2134+
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex);
21482135
assert(res_after_load_index.error_code == Success);
21492136

21502137
auto search_result_on_bigIndex = (SearchResult*)c_search_result_on_bigIndex;
@@ -2240,7 +2227,7 @@ TEST(CApiTest, Indexing_Expr_With_float_Predicate_Term) {
22402227
Timestamp time = 10000000;
22412228

22422229
CSearchResult c_search_result_on_smallIndex;
2243-
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex, -1);
2230+
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex);
22442231
assert(res_before_load_index.error_code == Success);
22452232

22462233
// load index to segment
@@ -2294,7 +2281,7 @@ TEST(CApiTest, Indexing_Expr_With_float_Predicate_Term) {
22942281

22952282
CSearchResult c_search_result_on_bigIndex;
22962283
auto res_after_load_index =
2297-
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex, -1);
2284+
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex);
22982285
assert(res_after_load_index.error_code == Success);
22992286

23002287
auto search_result_on_bigIndex = (SearchResult*)c_search_result_on_bigIndex;
@@ -2382,7 +2369,7 @@ TEST(CApiTest, Indexing_With_binary_Predicate_Range) {
23822369
Timestamp time = 10000000;
23832370

23842371
CSearchResult c_search_result_on_smallIndex;
2385-
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex, -1);
2372+
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex);
23862373
assert(res_before_load_index.error_code == Success);
23872374

23882375
// load index to segment
@@ -2437,7 +2424,7 @@ TEST(CApiTest, Indexing_With_binary_Predicate_Range) {
24372424

24382425
CSearchResult c_search_result_on_bigIndex;
24392426
auto res_after_load_index =
2440-
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex, -1);
2427+
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex);
24412428
assert(res_after_load_index.error_code == Success);
24422429

24432430
auto search_result_on_bigIndex = (SearchResult*)c_search_result_on_bigIndex;
@@ -2538,7 +2525,7 @@ TEST(CApiTest, Indexing_Expr_With_binary_Predicate_Range) {
25382525
Timestamp time = 10000000;
25392526

25402527
CSearchResult c_search_result_on_smallIndex;
2541-
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex, -1);
2528+
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex);
25422529
ASSERT_TRUE(res_before_load_index.error_code == Success) << res_before_load_index.error_msg;
25432530

25442531
// load index to segment
@@ -2593,7 +2580,7 @@ TEST(CApiTest, Indexing_Expr_With_binary_Predicate_Range) {
25932580

25942581
CSearchResult c_search_result_on_bigIndex;
25952582
auto res_after_load_index =
2596-
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex, -1);
2583+
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex);
25972584
assert(res_after_load_index.error_code == Success);
25982585

25992586
auto search_result_on_bigIndex = (SearchResult*)c_search_result_on_bigIndex;
@@ -2681,7 +2668,7 @@ TEST(CApiTest, Indexing_With_binary_Predicate_Term) {
26812668
Timestamp time = 10000000;
26822669

26832670
CSearchResult c_search_result_on_smallIndex;
2684-
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex, -1);
2671+
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex);
26852672
assert(res_before_load_index.error_code == Success);
26862673

26872674
// load index to segment
@@ -2736,7 +2723,7 @@ TEST(CApiTest, Indexing_With_binary_Predicate_Term) {
27362723

27372724
CSearchResult c_search_result_on_bigIndex;
27382725
auto res_after_load_index =
2739-
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex, -1);
2726+
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex);
27402727
assert(res_after_load_index.error_code == Success);
27412728

27422729
std::vector<CSearchResult> results;
@@ -2847,7 +2834,7 @@ TEST(CApiTest, Indexing_Expr_With_binary_Predicate_Term) {
28472834
Timestamp time = 10000000;
28482835

28492836
CSearchResult c_search_result_on_smallIndex;
2850-
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex, -1);
2837+
auto res_before_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_smallIndex);
28512838
assert(res_before_load_index.error_code == Success);
28522839

28532840
// load index to segment
@@ -2902,7 +2889,7 @@ TEST(CApiTest, Indexing_Expr_With_binary_Predicate_Term) {
29022889

29032890
CSearchResult c_search_result_on_bigIndex;
29042891
auto res_after_load_index =
2905-
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex, -1);
2892+
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex);
29062893
assert(res_after_load_index.error_code == Success);
29072894

29082895
std::vector<CSearchResult> results;
@@ -3115,7 +3102,7 @@ TEST(CApiTest, SealedSegment_search_float_Predicate_Range) {
31153102

31163103
CSearchResult c_search_result_on_bigIndex;
31173104
auto res_after_load_index =
3118-
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex, -1);
3105+
Search(sealed_segment.get(), plan, placeholderGroup, time, &c_search_result_on_bigIndex);
31193106
assert(res_after_load_index.error_code == Success);
31203107

31213108
auto search_result_on_bigIndex = (SearchResult*)c_search_result_on_bigIndex;
@@ -3228,12 +3215,12 @@ TEST(CApiTest, SealedSegment_search_without_predicates) {
32283215
std::vector<CPlaceholderGroup> placeholderGroups;
32293216
placeholderGroups.push_back(placeholderGroup);
32303217
CSearchResult search_result;
3231-
auto res = Search(segment, plan, placeholderGroup, N + ts_offset, &search_result, -1);
3218+
auto res = Search(segment, plan, placeholderGroup, N + ts_offset, &search_result);
32323219
std::cout << res.error_msg << std::endl;
32333220
ASSERT_EQ(res.error_code, Success);
32343221

32353222
CSearchResult search_result2;
3236-
auto res2 = Search(segment, plan, placeholderGroup, ts_offset, &search_result2, -1);
3223+
auto res2 = Search(segment, plan, placeholderGroup, ts_offset, &search_result2);
32373224
ASSERT_EQ(res2.error_code, Success);
32383225

32393226
DeleteSearchPlan(plan);
@@ -3408,7 +3395,7 @@ TEST(CApiTest, SealedSegment_search_float_With_Expr_Predicate_Range) {
34083395
}
34093396

34103397
CSearchResult c_search_result_on_bigIndex;
3411-
auto res_after_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_bigIndex, -1);
3398+
auto res_after_load_index = Search(segment, plan, placeholderGroup, time, &c_search_result_on_bigIndex);
34123399
assert(res_after_load_index.error_code == Success);
34133400

34143401
auto search_result_on_bigIndex = (SearchResult*)c_search_result_on_bigIndex;

‎internal/querynode/segment.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ func (s *Segment) search(searchReq *searchRequest) (*SearchResult, error) {
336336
s.pool.Submit(func() (interface{}, error) {
337337
tr := timerecord.NewTimeRecorder("cgoSearch")
338338
status = C.Search(s.segmentPtr, searchReq.plan.cSearchPlan, searchReq.cPlaceholderGroup,
339-
C.uint64_t(searchReq.timestamp), &searchResult.cSearchResult, C.int64_t(s.segmentID))
339+
C.uint64_t(searchReq.timestamp), &searchResult.cSearchResult)
340340
metrics.QueryNodeSQSegmentLatencyInCore.WithLabelValues(fmt.Sprint(Params.QueryNodeCfg.GetNodeID()), metrics.SearchLabel).Observe(float64(tr.ElapseSpan().Milliseconds()))
341341
return nil, nil
342342
}).Await()

0 commit comments

Comments
 (0)