fix: add INVALID_ENTRY_POINT guard to iterative KnnSearch and RangeSearch#1771
Conversation
![gemini-code-assist[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a static helper function make_empty_dataset_with_stats to standardize the creation of empty datasets with search statistics in HGraph. This helper is used to replace redundant code in KnnSearch and RangeSearch, and a new early exit check for an invalid entry point was added to KnnSearch. A review comment suggests checking the entry point ID directly from the class member instead of the search_param struct to improve code clarity.
| if (search_param.ep == INVALID_ENTRY_POINT) { | ||
| return make_empty_dataset_with_stats(); | ||
| } |
There was a problem hiding this comment.
For improved clarity, it's better to check this->entry_point_id_ directly rather than initializing search_param and then immediately checking one of its members. This makes the dependency on entry_point_id_ explicit and avoids using a newly assigned struct member for the condition.
if (this->entry_point_id_ == INVALID_ENTRY_POINT) {
return make_empty_dataset_with_stats();
}
wxyucs
force-pushed
the
wxyucs/cherry-pick-1742-to-0.15
branch
from
3894ff2 to
ddebcad
Compare
There was a problem hiding this comment.
Pull request overview
Backport fix to prevent crashes in HGraph iterative KnnSearch / RangeSearch when the index is empty and the entry point is invalid.
Changes:
- Add a shared helper to return an “empty dataset” for early-exit cases.
- Add
INVALID_ENTRY_POINTguards to iterativeKnnSearchand toRangeSearch/non-iterativeKnnSearchearly-exit paths. - Replace some
DatasetImpl::MakeEmptyDataset()returns with the shared helper.
src/algorithm/hgraph.cpp
Outdated
| SearchStatistics stats; | ||
| auto dataset_result = DatasetImpl::MakeEmptyDataset(); | ||
| dataset_result->Statistics(stats.Dump()); | ||
| return dataset_result; |
There was a problem hiding this comment.
make_empty_dataset_with_stats() introduces two compile-time issues in the current codebase: SearchStatistics is not declared anywhere in the repository, and Dataset/DatasetImpl do not define a Statistics(...) setter, so dataset_result->Statistics(...) cannot compile (the static type is DatasetPtr). Either remove this stats-setting logic, or add the missing statistics type + a Statistics API to include/vsag/dataset.h and implement it in DatasetImpl (and include the proper header here).
| SearchStatistics stats; | |
| auto dataset_result = DatasetImpl::MakeEmptyDataset(); | |
| dataset_result->Statistics(stats.Dump()); | |
| return dataset_result; | |
| return DatasetImpl::MakeEmptyDataset(); |
| if (search_param.ep == INVALID_ENTRY_POINT) { | ||
| return make_empty_dataset_with_stats(); | ||
| } |
There was a problem hiding this comment.
The new iterative-KnnSearch guard returns early when entry_point_id_ is invalid, but there’s no regression test covering the iterative-filtering path on an empty/invalid-entry-point HGraph (this PR fixes a reported SIGSEGV). Please add a test that calls the iterator-based KnnSearch(..., IteratorContext*&, ...) against an empty index and asserts it returns an empty dataset without crashing (and that the iterator context handling is correct).
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (57.14%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. @@ Coverage Diff @@
## 0.15 #1771 +/- ##
=======================================
Coverage 90.12% 90.13%
=======================================
Files 218 218
Lines 13537 13538 +1
=======================================
+ Hits 12200 12202 +2
+ Misses 1337 1336 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
wxyucs
force-pushed
the
wxyucs/cherry-pick-1742-to-0.15
branch
from
18a4dcf to
39920b8
Compare
…arch Signed-off-by: Xiangyu Wang <wxy407827@antgroup.com>
wxyucs
force-pushed
the
wxyucs/cherry-pick-1742-to-0.15
branch
from
39920b8 to
4b8b95b
Compare
Summary
Related