fix: skip dim check for non-vector fields in PreCheck (#41287) #41289
Conversation
|
Welcome @Hoyaspark! It looks like this is your first PR to milvus-io/milvus 🎉 |
|
@Hoyaspark 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. |
mergify
bot
added
needs-dco
Hoyaspark
force-pushed
the
fix/41287-skip-dim-check-non-vector
branch
from
1661385 to
f86c099
Compare
mergify
bot
added
dco-passed
|
@Hoyaspark cpp-unit-test check failed, comment |
Signed-off-by: 박상호 <[email protected]> Signed-off-by: Sangho Park <[email protected]>
Hoyaspark
force-pushed
the
fix/41287-skip-dim-check-non-vector
branch
from
f86c099 to
4022527
Compare
Signed-off-by: Sangho Park <[email protected]>
|
@Hoyaspark go-sdk check failed, comment |
Signed-off-by: Sangho Park <[email protected]>
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (50.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #41289 +/- ##
===========================================
+ Coverage 72.32% 80.61% +8.28%
===========================================
Files 310 1476 +1166
Lines 28670 210123 +181453
===========================================
+ Hits 20736 169391 +148655
- Misses 7934 34602 +26668
- Partials 0 6130 +6130
🚀 New features to boost your workflow:
|
internal/datacoord/task_index.go
Outdated
| // don't return, maybe field is scalar field or sparseFloatVector | ||
| // Extract dim only for vector types to avoid unnecessary warnings | ||
| dim := -1 | ||
| if typeutil.IsVectorType(field.GetDataType()) { |
There was a problem hiding this comment.
SparseFloatVector doesn't have a dim parameter either. It is recommended to use typeutil.IsFixDimVectorType(field.GetDataType).
There was a problem hiding this comment.
Thanks for the suggestion!
Addressed in a5417fb by using typeutil.IsFixDimVectorType instead of IsVectorType.
Signed-off-by: Sangho Park <[email protected]>
|
/lgtm @Hoyaspark Can you pick it to 2.5 branch? |
sre-ci-robot
added
size/XXL
|
@Hoyaspark Please associate the related pr of master to the body of your Pull Request. (eg. “pr: #”) |
sre-ci-robot
added
size/S
|
I mean cherry-pick it into a new PR based on the 2.5 branch. |
|
@xiaocai2333 I’ve created a new PR based on the 2.5 branch: #41329 |
|
@Hoyaspark E2e jenkins job failed, comment |
1 similar comment
|
@Hoyaspark E2e jenkins job failed, comment |
|
/run-cpu-e2e |
|
@Hoyaspark E2e jenkins job failed, comment |
|
/run-cpu-e2e |
…41329) issue: #41287 pr: #41289 Signed-off-by: Sangho Park <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: czs007, Hoyaspark 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:
Approvers can indicate their approval by writing |
What this PR does
This PR fixes an issue where the
PreCheckfunction in DataCoord logs unnecessary warningswhen attempting to retrieve 'dim' from non-vector fields.
The change adds a check to only call
GetDimFromParamswhen the field type is a vector type.Related issue
Fixes #41287