Skip to content

Commit

Permalink
enhance: remove CheckVecIndexWithDataTypeExist function in pkg and re…
Browse files Browse the repository at this point in the history
…move some cgo call (milvus-io#34102)

issue: milvus-io#22837
related pr: milvus-io#34104

Signed-off-by: cqy123456 <qianya.cheng@zilliz.com>
  • Loading branch information
cqy123456 authored and yellow-shine committed Jul 2, 2024
1 parent a86ec8e commit dceb34a
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 23 deletions.
38 changes: 38 additions & 0 deletions internal/proxy/cgo_util.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Licensed to the LF AI & Data foundation under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package proxy

/*
#cgo pkg-config: milvus_segcore
#include "segcore/check_vec_index_c.h"
#include <stdlib.h>
*/
import "C"

import (
"unsafe"

"github.com/milvus-io/milvus-proto/go-api/v2/schemapb"
)

func CheckVecIndexWithDataTypeExist(name string, dType schemapb.DataType) bool {
cIndexName := C.CString(name)
cType := uint32(dType)
defer C.free(unsafe.Pointer(cIndexName))
check := bool(C.CheckVecIndexWithDataType(cIndexName, cType))
return check
}
58 changes: 58 additions & 0 deletions internal/proxy/cgo_util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Licensed to the LF AI & Data foundation under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package proxy

import (
"testing"

"github.com/milvus-io/milvus-proto/go-api/v2/schemapb"
"github.com/milvus-io/milvus/pkg/util/indexparamcheck"
)

func Test_CheckVecIndexWithDataTypeExist(t *testing.T) {
cases := []struct {
indexType string
dataType schemapb.DataType
want bool
}{
{indexparamcheck.IndexHNSW, schemapb.DataType_FloatVector, true},
{indexparamcheck.IndexHNSW, schemapb.DataType_BinaryVector, false},
{indexparamcheck.IndexHNSW, schemapb.DataType_Float16Vector, true},

{indexparamcheck.IndexSparseWand, schemapb.DataType_SparseFloatVector, true},
{indexparamcheck.IndexSparseWand, schemapb.DataType_FloatVector, false},
{indexparamcheck.IndexSparseWand, schemapb.DataType_Float16Vector, false},

{indexparamcheck.IndexGpuBF, schemapb.DataType_FloatVector, true},
{indexparamcheck.IndexGpuBF, schemapb.DataType_Float16Vector, false},
{indexparamcheck.IndexGpuBF, schemapb.DataType_BinaryVector, false},

{indexparamcheck.IndexFaissBinIvfFlat, schemapb.DataType_BinaryVector, true},
{indexparamcheck.IndexFaissBinIvfFlat, schemapb.DataType_FloatVector, false},

{indexparamcheck.IndexDISKANN, schemapb.DataType_FloatVector, true},
{indexparamcheck.IndexDISKANN, schemapb.DataType_Float16Vector, true},
{indexparamcheck.IndexDISKANN, schemapb.DataType_BFloat16Vector, true},
{indexparamcheck.IndexDISKANN, schemapb.DataType_BinaryVector, false},
}

for _, test := range cases {
if got := CheckVecIndexWithDataTypeExist(test.indexType, test.dataType); got != test.want {
t.Errorf("CheckVecIndexWithDataTypeExist(%v, %v) = %v", test.indexType, test.dataType, test.want)
}
}
}
13 changes: 7 additions & 6 deletions internal/proxy/task_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,18 +370,19 @@ func checkTrain(field *schemapb.FieldSchema, indexParams map[string]string) erro
indexParams[common.BitmapCardinalityLimitKey] = paramtable.Get().CommonCfg.BitmapIndexCardinalityBound.GetValue()
}
}
if typeutil.IsVectorType(field.DataType) && indexType != indexparamcheck.AutoIndex {
exist := indexparamcheck.CheckVecIndexWithDataTypeExist(indexType, field.DataType)
if !exist {
return fmt.Errorf("data type %d can't build with this index %s", field.DataType, indexType)
}
}
checker, err := indexparamcheck.GetIndexCheckerMgrInstance().GetChecker(indexType)
if err != nil {
log.Warn("Failed to get index checker", zap.String(common.IndexTypeKey, indexType))
return fmt.Errorf("invalid index type: %s", indexType)
}

if typeutil.IsVectorType(field.DataType) && indexType != indexparamcheck.AutoIndex {
exist := CheckVecIndexWithDataTypeExist(indexType, field.DataType)
if !exist {
return fmt.Errorf("data type %d can't build with this index %s", field.DataType, indexType)
}
}

if !typeutil.IsSparseFloatVectorType(field.DataType) {
if err := fillDimension(field, indexParams); err != nil {
return err
Expand Down
17 changes: 0 additions & 17 deletions pkg/util/indexparamcheck/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,10 @@

package indexparamcheck

/*
#cgo pkg-config: milvus_segcore
#include "segcore/check_vec_index_c.h"
#include <stdlib.h>
*/
import "C"

import (
"fmt"
"strconv"
"unsafe"

"github.com/milvus-io/milvus-proto/go-api/v2/schemapb"
"github.com/milvus-io/milvus/pkg/util/funcutil"
)

Expand Down Expand Up @@ -78,11 +69,3 @@ func setDefaultIfNotExist(params map[string]string, key string, defaultValue str
params[key] = defaultValue
}
}

func CheckVecIndexWithDataTypeExist(name string, dType schemapb.DataType) bool {
cIndexName := C.CString(name)
cType := uint32(dType)
defer C.free(unsafe.Pointer(cIndexName))
check := bool(C.CheckVecIndexWithDataType(cIndexName, cType))
return check
}

0 comments on commit dceb34a

Please sign in to comment.