enhance: support iterative filter execution #37363

chasingegg · 2024-11-01T07:21:57Z

mergify · 2024-11-01T07:22:34Z

Invalid PR Title Format Detected

Your PR submission does not adhere to our required standards. To ensure clarity and consistency, please meet the following criteria:

Title Format: The PR title must begin with one of these prefixes:

feat: for introducing a new feature.
fix: for bug fixes.
enhance: for improvements to existing functionality.
test: for add tests to existing functionality.
doc: for modifying documentation.
auto: for the pull request from bot.

Description Requirement: The PR must include a non-empty description, detailing the changes and their impact.

Required Title Structure:

[Type]: [Description of the PR]

Where Type is one of feat, fix, enhance, test or doc.

Example:

enhance: improve search performance significantly

Please review and update your PR to comply with these guidelines.

mergify · 2024-11-01T07:27:16Z

@chasingegg go-sdk check failed, comment rerun go-sdk can trigger the job again.

mergify · 2024-11-01T07:32:49Z

@chasingegg cpp-unit-test check failed, comment rerun cpp-unit-test can trigger the job again.

mergify · 2024-11-01T07:36:51Z

@chasingegg E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

codecov · 2024-11-01T09:54:49Z

Codecov Report

Attention: Patch coverage is 81.86851% with 262 lines in your changes missing coverage. Please review.

Project coverage is 81.06%. Comparing base (618f0cb) to head (6881210).
Report is 36 commits behind head on master.

Files with missing lines	Patch %	Lines
internal/core/src/exec/expression/Expr.h	52.03%	59 Missing ⚠️
...nal/core/src/exec/operator/IterativeFilterNode.cpp	76.51%	31 Missing ⚠️
internal/core/src/exec/expression/CompareExpr.h	64.61%	23 Missing ⚠️
internal/core/src/exec/expression/ColumnExpr.cpp	65.78%	13 Missing ⚠️
internal/core/src/exec/operator/Utils.h	53.57%	13 Missing ⚠️
internal/core/src/common/Chunk.cpp	8.33%	11 Missing ⚠️
...rnal/core/src/exec/expression/JsonContainsExpr.cpp	96.09%	10 Missing ⚠️
internal/core/src/exec/expression/ColumnExpr.h	0.00%	8 Missing ⚠️
internal/core/src/exec/expression/UnaryExpr.cpp	92.72%	8 Missing ⚠️
internal/core/src/exec/expression/UnaryExpr.h	72.41%	8 Missing ⚠️
... and 18 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #37363      +/-   ##
==========================================
- Coverage   81.08%   81.06%   -0.02%     
==========================================
  Files        1372     1375       +3     
  Lines      191564   192216     +652     
==========================================
+ Hits       155324   155825     +501     
- Misses      30740    30901     +161     
+ Partials     5500     5490      -10

Components	Coverage Δ
Client	`74.43% <ø> (ø)`
Core	`68.93% <81.79%> (+0.08%)`	⬆️
Go	`83.22% <100.00%> (+0.01%)`	⬆️

Files with missing lines	Coverage Δ
internal/core/src/common/Chunk.h	`60.71% <100.00%> (+0.47%)`	⬆️
internal/core/src/common/QueryInfo.h	`100.00% <ø> (ø)`
internal/core/src/exec/Driver.cpp	`81.39% <100.00%> (+0.44%)`	⬆️
internal/core/src/exec/QueryContext.h	`84.61% <100.00%> (+0.30%)`	⬆️
...ternal/core/src/exec/expression/AlwaysTrueExpr.cpp	`88.23% <100.00%> (+2.52%)`	⬆️
...e/src/exec/expression/BinaryArithOpEvalRangeExpr.h	`100.00% <100.00%> (ø)`
...nternal/core/src/exec/expression/BinaryRangeExpr.h	`94.00% <100.00%> (+1.31%)`	⬆️
internal/core/src/exec/expression/CallExpr.cpp	`100.00% <100.00%> (ø)`
internal/core/src/exec/expression/EvalCtx.h	`100.00% <100.00%> (ø)`
internal/core/src/exec/expression/ExistsExpr.cpp	`89.47% <100.00%> (+2.37%)`	⬆️
... and 47 more

... and 32 files with indirect coverage changes

PwzXxm · 2024-11-04T08:16:10Z

internal/core/src/exec/QueryContext.h

+    std::optional<int64_t>
+    get_iterator_batch_size() {
+        return milvus::index::GetValueFromConfig<int64_t>(
+            search_info_.search_params_, "batch_size");


Is it possible to rename it as conflicting to E2E Iterator parameters?

PwzXxm · 2024-11-04T08:29:48Z

internal/core/src/exec/expression/Expr.h

-            MoveCursorForIndex();
-            if (segment_->HasFieldData(field_id_)) {
+        // when we specify input, do not maintain states
+        if (has_offset_input_) {


Why it does not need to move cursor here when input is specified? Not the other way around as in internal/core/src/exec/expression/LogicalBinaryExpr.h per say?

PwzXxm · 2024-11-04T08:33:15Z

internal/core/src/exec/operator/FilterNode.cpp

+                      size_t hi,
+                      float dist) {
+    while (lo < hi) {
+        size_t mid = (lo + hi) >> 1;


Potential overflow, use size_t mid = lo + ((hi - lo) >> 1)

PwzXxm · 2024-11-04T08:37:47Z

internal/core/src/exec/operator/FilterNode.cpp

+    double scalar_cost =
+        std::chrono::duration<double, std::micro>(scalar_end - scalar_start)
+            .count();
+    monitor::internal_core_search_latency_postfilter.Observe(scalar_cost);


Great to see adding metrics as well!

PwzXxm · 2024-11-04T08:44:07Z

internal/core/src/exec/operator/FilterNode.cpp

+                auto col_vec_size = col_vec->size();
+                TargetBitmapView bitsetview(col_vec->GetRawData(),
+                                            col_vec_size);
+                Assert(bitsetview.size() <= batch_size);


Why this must be met? Is it possible that user gives a small batch_size?

PwzXxm · 2024-11-04T08:56:29Z

internal/core/src/exec/operator/FilterNode.cpp

+}
+
+RowVectorPtr
+PhyFilterNode::GetOutput() {


Does it support RangeSearch?

mergify · 2024-11-05T11:00:58Z

@chasingegg cpp-unit-test check failed, comment rerun cpp-unit-test can trigger the job again.

zhagnlu · 2024-11-13T03:28:29Z

internal/core/src/query/PlanProto.cpp

+            plan_node->search_info_.group_by_field_id_ == std::nullopt) {
+            plannode = std::make_shared<milvus::plan::MvccNode>(
+                milvus::plan::GetNextPlanNodeId());
+            sources = std::vector<milvus::plan::PlanNodePtr>{plannode};
+            plannode = std::make_shared<milvus::plan::VectorSearchNode>(
+                milvus::plan::GetNextPlanNodeId(), sources);
+            sources = std::vector<milvus::plan::PlanNodePtr>{plannode};
+
+            // add filter nodes after vector search node
+            auto expr = ParseExprs(anns_proto.predicates());
+            plannode = std::make_shared<plan::FilterNode>(
+                milvus::plan::GetNextPlanNodeId(), expr, sources);
+            sources = std::vector<milvus::plan::PlanNodePtr>{plannode};


wrapper as a function, same as below else

zhagnlu · 2024-11-13T03:34:25Z

internal/core/src/query/PlanProto.cpp

+            !search_info.search_params_.contains(RADIUS)) {
+            search_info.post_filter_execution =
+                search_info.search_params_[POST_FILTER];
+        }


Is that means user decide to whether using post filter or not ? Could decided by other method like stats info etc

zhagnlu · 2024-11-13T03:58:43Z

internal/core/src/exec/operator/FilterNode.h

+// FilterNode will accept offsets array and execute over these and generate result valid offsets
+namespace milvus {
+namespace exec {
+class PhyFilterNode : public Operator {


PhyPosterFilterBitsNode may more accurate. PhyFilterNode should not related with vector search node, not just return bits, it is more pure concept. When we support project function, add PhyFilterNode is better，same is FilterNode.h

zhagnlu · 2024-11-13T06:47:26Z

internal/core/src/exec/expression/EvalCtx.h

@@ -28,17 +28,26 @@ namespace milvus {
 namespace exec {

 class ExprSet;
+
+using OffsetVector = FixedVector<int64_t>;


int32_t is enough

mergify · 2024-11-14T04:03:42Z

@chasingegg cpp-unit-test check failed, comment rerun cpp-unit-test can trigger the job again.

chasingegg · 2024-11-14T04:18:39Z

rerun cpp-unit-test

mergify · 2024-12-05T01:22:09Z

@chasingegg Thanks for your contribution. Please submit with DCO, see the contributing guide https://github.com/milvus-io/milvus/blob/master/CONTRIBUTING.md#developer-certificate-of-origin-dco.

Signed-off-by: chasingegg <[email protected]>

mergify · 2024-12-05T05:07:28Z

@chasingegg go-sdk check failed, comment rerun go-sdk can trigger the job again.

chasingegg · 2024-12-05T05:16:27Z

rerun go-sdk

mergify · 2024-12-05T05:55:03Z

@chasingegg go-sdk check failed, comment rerun go-sdk can trigger the job again.

chasingegg · 2024-12-05T07:05:25Z

rerun go-sdk

chasingegg · 2024-12-05T07:59:13Z

change some search_params formats and use the name hints

chasingegg · 2024-12-11T03:00:15Z

/unhold

czs007 · 2024-12-11T03:32:23Z

/approve
/lgtm

sre-ci-robot · 2024-12-11T03:32:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chasingegg, czs007

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [czs007]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sre-ci-robot added area/compilation size/XXL Denotes a PR that changes 1000+ lines. labels Nov 1, 2024

sre-ci-robot requested review from godchen0212 and xiaocai2333 November 1, 2024 07:22

mergify bot added dco-passed DCO check passed. do-not-merge/invalid-pr-format labels Nov 1, 2024

chasingegg changed the title ~~Support post filter~~ enhance: support post filter execution Nov 1, 2024

mergify bot added kind/enhancement Issues or changes related to enhancement and removed do-not-merge/invalid-pr-format labels Nov 1, 2024

PwzXxm reviewed Nov 4, 2024

View reviewed changes

chasingegg force-pushed the support-post-filter branch from 6a7ea43 to 83d7a5e Compare November 5, 2024 10:23

chasingegg force-pushed the support-post-filter branch 3 times, most recently from 909b13b to 82aa236 Compare November 8, 2024 10:14

mergify bot added the ci-passed label Nov 8, 2024

zhagnlu reviewed Nov 13, 2024

View reviewed changes

chasingegg force-pushed the support-post-filter branch from 82aa236 to 8dcc2db Compare November 14, 2024 03:50

mergify bot removed the ci-passed label Nov 14, 2024

mergify bot added the ci-passed label Nov 14, 2024

chasingegg force-pushed the support-post-filter branch from 8dcc2db to 3a9224e Compare November 14, 2024 09:07

mergify bot added ci-passed and removed ci-passed labels Nov 14, 2024

mergify bot added the ci-passed label Dec 2, 2024

chasingegg force-pushed the support-post-filter branch from 0af05ee to 8662f77 Compare December 5, 2024 01:21

mergify bot added needs-dco DCO is missing in this pull request. dco-passed DCO check passed. and removed dco-passed DCO check passed. ci-passed needs-dco DCO is missing in this pull request. labels Dec 5, 2024

chasingegg mentioned this pull request Dec 5, 2024

enhance: support hints param milvus-io/pymilvus#2408

Merged

chasingegg added 4 commits December 5, 2024 11:59

Support post filter

63d7d04

Signed-off-by: chasingegg <[email protected]>

update

987b61b

Signed-off-by: chasingegg <[email protected]>

update

b30afcc

Signed-off-by: chasingegg <[email protected]>

update

90340c3

Signed-off-by: chasingegg <[email protected]>

chasingegg force-pushed the support-post-filter branch from 8662f77 to 90340c3 Compare December 5, 2024 04:11

update

6881210

Signed-off-by: chasingegg <[email protected]>

mergify bot added the ci-passed label Dec 5, 2024

sre-ci-robot added the do-not-merge/hold label Dec 5, 2024

sre-ci-robot removed the do-not-merge/hold label Dec 11, 2024

sre-ci-robot assigned czs007 Dec 11, 2024

sre-ci-robot added the lgtm label Dec 11, 2024

sre-ci-robot merged commit 994fc54 into milvus-io:master Dec 11, 2024
20 checks passed

chasingegg deleted the support-post-filter branch December 11, 2024 03:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enhance: support iterative filter execution #37363

enhance: support iterative filter execution #37363

chasingegg commented Nov 1, 2024

mergify bot commented Nov 1, 2024

mergify bot commented Nov 1, 2024

mergify bot commented Nov 1, 2024

mergify bot commented Nov 1, 2024

codecov bot commented Nov 1, 2024 •

edited

Loading

PwzXxm Nov 4, 2024

PwzXxm Nov 4, 2024

PwzXxm Nov 4, 2024

PwzXxm Nov 4, 2024

PwzXxm Nov 4, 2024

PwzXxm Nov 4, 2024

mergify bot commented Nov 5, 2024

zhagnlu Nov 13, 2024

zhagnlu Nov 13, 2024

zhagnlu Nov 13, 2024

zhagnlu Nov 13, 2024

mergify bot commented Nov 14, 2024

chasingegg commented Nov 14, 2024

mergify bot commented Dec 5, 2024

mergify bot commented Dec 5, 2024

chasingegg commented Dec 5, 2024

mergify bot commented Dec 5, 2024

chasingegg commented Dec 5, 2024

chasingegg commented Dec 5, 2024 •

edited

Loading

chasingegg commented Dec 11, 2024

czs007 commented Dec 11, 2024

sre-ci-robot commented Dec 11, 2024

enhance: support iterative filter execution #37363

enhance: support iterative filter execution #37363

Conversation

chasingegg commented Nov 1, 2024

mergify bot commented Nov 1, 2024

mergify bot commented Nov 1, 2024

mergify bot commented Nov 1, 2024

mergify bot commented Nov 1, 2024

codecov bot commented Nov 1, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Nov 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Nov 14, 2024

chasingegg commented Nov 14, 2024

mergify bot commented Dec 5, 2024

mergify bot commented Dec 5, 2024

chasingegg commented Dec 5, 2024

mergify bot commented Dec 5, 2024

chasingegg commented Dec 5, 2024

chasingegg commented Dec 5, 2024 • edited Loading

chasingegg commented Dec 11, 2024

czs007 commented Dec 11, 2024

sre-ci-robot commented Dec 11, 2024

codecov bot commented Nov 1, 2024 •

edited

Loading

chasingegg commented Dec 5, 2024 •

edited

Loading