Improve error message to guide user fixing invalid parameter (#518)

Signed-off-by: yah01 <yang.cen@zilliz.com>
milvus-io · Jul 18, 2023 · bfb377d · bfb377d
1 parent 524c9ad
commit bfb377d
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 62 deletions.
diff --git a/entity/genidx/genidx.go b/entity/genidx/genidx.go
@@ -277,11 +277,11 @@ func (ip idxParam) ValidationCode() string {
 		min := strings.TrimSpace(parts[0])
 		max := strings.TrimSpace(parts[1])
 		return fmt.Sprintf(`if %s < %s {
-		return nil, errors.New("%s not valid")
+		return nil, errors.New("%s has to be in range [%s, %s]")
 	}
 	if %s > %s {
-		return nil, errors.New("%s not valid")
-	}`, ip.Name, min, ip.Name, ip.Name, max, ip.Name)
+		return nil, errors.New("%s has to be in range [%s, %s]")
+	}`, ip.Name, min, ip.Name, min, max, ip.Name, max, ip.Name, min, max)
 	default:
 		return ""
 	}

diff --git a/entity/indexes_gen.go b/entity/indexes_gen.go
diff --git a/entity/indexes_search_param_gen.go b/entity/indexes_search_param_gen.go
diff --git a/test/testcases/index_test.go b/test/testcases/index_test.go
@@ -195,26 +195,26 @@ func TestCreateIndexInvalidParams(t *testing.T) {
 	// invalid IvfFlat nlist [1, 65536]
 	_, errIvfFlatNlist := entity.NewIndexIvfFlat(entity.L2, 0)
 	_, errIvfFlatNlist2 := entity.NewIndexIvfFlat(entity.L2, 65537)
-	common.CheckErr(t, errIvfFlatNlist, false, "nlist not valid")
-	common.CheckErr(t, errIvfFlatNlist2, false, "nlist not valid")
+	common.CheckErr(t, errIvfFlatNlist, false, "nlist has to be in range [1, 65536]")
+	common.CheckErr(t, errIvfFlatNlist2, false, "nlist has to be in range [1, 65536]")
 
 	// invalid IvfSq8 nlist [1, 65536]
 	_, errIvfSq8Nlist := entity.NewIndexIvfFlat(entity.L2, 0)
 	_, errIvfSq8Nlist2 := entity.NewIndexIvfFlat(entity.L2, 65537)
-	common.CheckErr(t, errIvfSq8Nlist, false, "nlist not valid")
-	common.CheckErr(t, errIvfSq8Nlist2, false, "nlist not valid")
+	common.CheckErr(t, errIvfSq8Nlist, false, "nlist has to be in range [1, 65536]")
+	common.CheckErr(t, errIvfSq8Nlist2, false, "nlist has to be in range [1, 65536]")
 
 	// invalid IvfPq nlist [1, 65536]
 	_, errIvfPqNlist := entity.NewIndexIvfPQ(entity.L2, -1, 16, 8)
-	common.CheckErr(t, errIvfPqNlist, false, "nlist not valid")
+	common.CheckErr(t, errIvfPqNlist, false, "nlist has to be in range [1, 65536]")
 	_, errIvfPqNlist2 := entity.NewIndexIvfPQ(entity.L2, 65538, 16, 8)
-	common.CheckErr(t, errIvfPqNlist2, false, "nlist not valid")
+	common.CheckErr(t, errIvfPqNlist2, false, "nlist has to be in range [1, 65536]")
 
 	// invalid IvfPq params m dim ≡ 0 (mod m), nbits [1, 16]
 	_, errIvfPqNbits := entity.NewIndexIvfPQ(entity.L2, 128, 8, 0)
-	common.CheckErr(t, errIvfPqNbits, false, "nbits not valid")
+	common.CheckErr(t, errIvfPqNbits, false, "nbits has to be in range [1, 16]")
 	_, errIvfPqNbits2 := entity.NewIndexIvfPQ(entity.L2, 128, 8, 17)
-	common.CheckErr(t, errIvfPqNbits2, false, "nbits not valid")
+	common.CheckErr(t, errIvfPqNbits2, false, "nbits has to be in range [1, 16]")
 	// TODO unclear error message
 	idxInvalidm, _ := entity.NewIndexIvfPQ(entity.L2, 128, 7, 8)
 	errm := mc.CreateIndex(ctx, collName, common.DefaultFloatVecFieldName, idxInvalidm, false)
@@ -223,13 +223,13 @@ func TestCreateIndexInvalidParams(t *testing.T) {
 
 	// invalid Hnsw M [4, 64], efConstruction [8, 512]
 	_, errHnswM := entity.NewIndexHNSW(entity.L2, 3, 96)
-	common.CheckErr(t, errHnswM, false, "M not valid")
+	common.CheckErr(t, errHnswM, false, "M has to be in range [4, 64]")
 	_, errHnswM2 := entity.NewIndexHNSW(entity.L2, 128, 96)
-	common.CheckErr(t, errHnswM2, false, "M not valid")
+	common.CheckErr(t, errHnswM2, false, "M has to be in range [4, 64]")
 	_, errHnswEf := entity.NewIndexHNSW(entity.L2, 8, 7)
-	common.CheckErr(t, errHnswEf, false, "efConstruction not valid")
+	common.CheckErr(t, errHnswEf, false, "efConstruction has to be in range [8, 512]")
 	_, errHnswEf2 := entity.NewIndexHNSW(entity.L2, 8, 515)
-	common.CheckErr(t, errHnswEf2, false, "efConstruction not valid")
+	common.CheckErr(t, errHnswEf2, false, "efConstruction has to be in range [8, 512]")
 
 	// invalid flat metric type jaccard
 	// TODO unclear error message

diff --git a/test/testcases/search_test.go b/test/testcases/search_test.go
@@ -581,32 +581,32 @@ func TestSearchInvalidSearchParams(t *testing.T) {
 	invalidNprobe := []int{-1, 0, 65537}
 	for _, nprobe := range invalidNprobe {
 		_, errIvfFlat := entity.NewIndexIvfFlatSearchParam(nprobe)
-		common.CheckErr(t, errIvfFlat, false, "nprobe not valid")
+		common.CheckErr(t, errIvfFlat, false, "nprobe has to be in range [1, 65536]")
 	}
 
 	// ivf sq8 search param
 	for _, nprobe := range invalidNprobe {
 		_, errIvfSq8 := entity.NewIndexIvfSQ8SearchParam(nprobe)
-		common.CheckErr(t, errIvfSq8, false, "nprobe not valid")
+		common.CheckErr(t, errIvfSq8, false, "nprobe has to be in range [1, 65536]")
 	}
 
 	// ivf pq search param
 	for _, nprobe := range invalidNprobe {
 		_, errIvfPq := entity.NewIndexIvfPQSearchParam(nprobe)
-		common.CheckErr(t, errIvfPq, false, "nprobe not valid")
+		common.CheckErr(t, errIvfPq, false, "nprobe has to be in range [1, 65536]")
 	}
 
 	// hnsw search params ef [top_k, 32768]
 	invalidEfs := []int{-1, 0, 32769}
 	for _, invalidEf := range invalidEfs {
 		_, errHnsw := entity.NewIndexHNSWSearchParam(invalidEf)
-		common.CheckErr(t, errHnsw, false, "ef not valid")
+		common.CheckErr(t, errHnsw, false, "ef has to be in range [1, 32768]")
 	}
 
 	// bin ivf flat
 	for _, nprobe := range invalidNprobe {
 		_, errBinIvfFlat := entity.NewIndexBinIvfFlatSearchParam(nprobe)
-		common.CheckErr(t, errBinIvfFlat, false, "nprobe not valid")
+		common.CheckErr(t, errBinIvfFlat, false, "nprobe has to be in range [1, 65536]")
 	}
 }