Skip to content

enhance: [2.6] type-aware bidirectional in/== or expression rewriting#48545

Open
zhengbuqian wants to merge 3 commits intomilvus-io:2.6from
zhengbuqian:feat/optimize-in-vs-eqor-threshold-2.6
Open

enhance: [2.6] type-aware bidirectional in/== or expression rewriting#48545
zhengbuqian wants to merge 3 commits intomilvus-io:2.6from
zhengbuqian:feat/optimize-in-vs-eqor-threshold-2.6

Conversation

@zhengbuqian
Copy link
Collaborator

issue: #45525
pr: #48544

Replace the fixed threshold (150) for merging == or into in[] with
type-specific thresholds based on benchmark data:
- INT types: use in[] when N >= 10 (== or is faster below due to simpler execution path)
- FLOAT types: use in[] when N >= 15 (float hash is more expensive)
- Other types (varchar, bool): use in[] when N >= 3

The rewriting is now bidirectional:
- == or → in[]: when shouldUseInExpr returns true (existing direction)
- in[] → == or: when shouldUseInExpr returns false (new direction, via visitTermExpr)
- not in → != and: when shouldUseInExpr returns false (new, via visitUnaryExpr)

Both directions use the same shouldUseInExpr function to ensure consistency.

Signed-off-by: Buqian Zheng <zhengbuqian@gmail.com>
@sre-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zhengbuqian

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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 sre-ci-robot requested review from czs007 and xige-16 March 26, 2026 08:57
@sre-ci-robot sre-ci-robot added the size/L Denotes a PR that changes 100-499 lines. label Mar 26, 2026
@mergify mergify bot added dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement labels Mar 26, 2026
@sre-ci-robot
Copy link
Contributor

[ci-v2-notice]
Notice: New ci-v2 system is enabled for this PR.

To rerun ci-v2 checks, comment with:

  • /ci-rerun-code-check // for ci-v2/code-check
  • /ci-rerun-build // for ci-v2/build
  • /ci-rerun-build-all // for ci-v2/build-all (multi-arch builds)
  • /ci-rerun-buildenv // for ci-v2/build-env (build milvus-env builder images)
  • /ci-rerun-ut-integration // for ci-v2/ut-integration, will rerun ci-v2/build
  • /ci-rerun-ut-go // for ci-v2/ut-go, will rerun ci-v2/build
  • /ci-rerun-ut-cpp // for ci-v2/ut-cpp
  • /ci-rerun-ut // for all ci-v2/ut-integration, ci-v2/ut-go, ci-v2/ut-cpp, will rerun ci-v2/build
  • /ci-rerun-e2e-arm // for ci-v2/e2e-arm
  • /ci-rerun-e2e-default // for ci-v2/e2e-default
  • /ci-rerun-ciloop // for ci-v2/ciloop (build + unit tests in one pipeline)
  • /ci-rerun-gosdk // for ci-v2/go-sdk (Go SDK E2E tests, AMD)
  • /ci-rerun-gosdk-arm // for ci-v2/go-sdk-arm (Go SDK E2E tests, ARM)

If you have any questions or requests, please contact @zhikunyao.

@sre-ci-robot sre-ci-robot added do-not-merge/need-merge-master-first any pr merge to release branch need to merge master first do-not-merge/need-milestone generate by v2-label-manager labels Mar 26, 2026
@sre-ci-robot
Copy link
Contributor

[INFO] PR Label Summary by Default
[FAILED] PR #48544 not merged

[WARNING] Milestone not set

You can set milestone by commenting:
/set-milestone
Example:
/set-milestone 2.5.0

Use /refresh-label to update related check and label manually

@codecov
Copy link

codecov bot commented Mar 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.48%. Comparing base (cb08db0) to head (638ef88).
⚠️ Report is 734 commits behind head on 2.6.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##              2.6   #48545       +/-   ##
===========================================
+ Coverage   76.99%   82.48%    +5.49%     
===========================================
  Files        1700      555     -1145     
  Lines      262533    88038   -174495     
===========================================
- Hits       202142    72621   -129521     
+ Misses      53550    15365    -38185     
+ Partials     6841       52     -6789     
Components Coverage Δ
Client ∅ <ø> (∅)
Core 83.36% <93.03%> (+1.15%) ⬆️
Go ∅ <ø> (∅)
Files with missing lines Coverage Δ
...rnal/core/unittest/test_expr_materialized_view.cpp 97.94% <100.00%> (ø)

... and 1584 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sre-ci-robot sre-ci-robot added the low-code-coverage add test-label from zhikun, diff coverage > 80% label Mar 26, 2026
zhengbuqian and others added 2 commits March 26, 2026 20:40
The expression rewriter splits small IN lists into OR equals based on
type-specific thresholds (integer >= 10, float >= 15, default >= 3).
Tests that expected TermExpr with fewer values than the threshold were
failing because the rewriter converted them to OR expressions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Buqian Zheng <zhengbuqian@gmail.com>
BOOL only has 2 unique values (true/false), which is always below
the IN threshold (default >= 3), so it gets split into OR equals.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Buqian Zheng <zhengbuqian@gmail.com>
@sre-ci-robot
Copy link
Contributor

[INFO] PR Label Summary by Default
[FAILED] PR #48544 not merged

[WARNING] Milestone not set

You can set milestone by commenting:
/set-milestone
Example:
/set-milestone 2.5.0

Use /refresh-label to update related check and label manually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved dco-passed DCO check passed. do-not-merge/need-merge-master-first any pr merge to release branch need to merge master first do-not-merge/need-milestone generate by v2-label-manager kind/enhancement Issues or changes related to enhancement low-code-coverage add test-label from zhikun, diff coverage > 80% size/L Denotes a PR that changes 100-499 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants