enhance: rewrite index params for compatibility #35788
Conversation
mergify
bot
added
dco-passed
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #35788 +/- ##
==========================================
+ Coverage 72.29% 72.61% +0.32%
==========================================
Files 1238 1264 +26
Lines 148063 150710 +2647
==========================================
+ Hits 107039 109443 +2404
- Misses 36254 36383 +129
- Partials 4770 4884 +114
|
| func IsScalarIndexType(indexType IndexType) bool { | ||
| return indexType == IndexSTLSORT || indexType == IndexTRIE || indexType == IndexTrie || | ||
| indexType == IndexBitmap || indexType == IndexHybrid || indexType == IndexINVERTED || | ||
| indexType == AutoIndex |
There was a problem hiding this comment.
AutoIndex can also be VectorIndex.
There was a problem hiding this comment.
OK, remove it, actually indexparam not include autoindex, userindexpram use it
zhagnlu
force-pushed
the
rewrite_indexparam_autoindex
branch
from
8d63dae to
6a5ae9e
Compare
| @@ -209,6 +209,7 @@ func checkParams(fieldIndex *model.Index, req *indexpb.CreateIndexRequest) bool | |||
| for i, param2 := range req.GetUserIndexParams() { | |||
| if param2.Key == param1.Key && param2.Value == param1.Value { | |||
| exist = true | |||
| break | |||
There was a problem hiding this comment.
have to admit original logic is not well-written, but we should not break here, need to check all the params and make sure all the params are the same
There was a problem hiding this comment.
have to admit original logic is not well-written, but we should not break here, need to check all the params and make sure all the params are the same
acutally, outside have circle
break is just break current key, outside keys always do this
| @@ -224,14 +225,32 @@ func checkParams(fieldIndex *model.Index, req *indexpb.CreateIndexRequest) bool | |||
| } | |||
| } | |||
| exist = true | |||
| break | |||
internal/datacoord/index_meta.go
Outdated
| } | ||
| } | ||
| } | ||
| log.Debug("final request", zap.Any("create index request", req.String())) |
There was a problem hiding this comment.
use info or warn log ? help to debug
zhagnlu
force-pushed
the
rewrite_indexparam_autoindex
branch
from
6a5ae9e to
0d44817
Compare
|
/lgtm |
…ut config changed Signed-off-by: luzhang <[email protected]>
zhagnlu
force-pushed
the
rewrite_indexparam_autoindex
branch
from
0d44817 to
e860366
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: czs007, zhagnlu 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 |
#32900