Skip to content

Commit

Permalink
fix: text match panics when enable_match is set be false (#38950)
Browse files Browse the repository at this point in the history
fix: #38949

---------

Signed-off-by: SpadeA-Tang <tangchenjie1210@gmail.com>
  • Loading branch information
SpadeA-Tang authored Jan 3, 2025
1 parent d7623ab commit 4245c5b
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 4 deletions.
1 change: 1 addition & 0 deletions internal/core/src/common/EasyAssert.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ enum ErrorCode {
FollyCancel = 2038,
OutOfRange = 2039,
GcpNativeError = 2040,
TextIndexNotFound = 2041,

KnowhereError = 2099
};
Expand Down
6 changes: 5 additions & 1 deletion internal/core/src/segcore/SegmentGrowingImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,11 @@ SegmentGrowingImpl::AddTexts(milvus::FieldId field_id,
int64_t offset_begin) {
std::unique_lock lock(mutex_);
auto iter = text_indexes_.find(field_id);
AssertInfo(iter != text_indexes_.end(), "text index not found");
if (iter == text_indexes_.end()) {
throw SegcoreError(
ErrorCode::TextIndexNotFound,
fmt::format("text index not found for field {}", field_id.get()));
}
iter->second->AddTexts(n, texts, texts_valid_data, offset_begin);
}

Expand Down
7 changes: 5 additions & 2 deletions internal/core/src/segcore/SegmentInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,11 @@ index::TextMatchIndex*
SegmentInternalInterface::GetTextIndex(FieldId field_id) const {
std::shared_lock lock(mutex_);
auto iter = text_indexes_.find(field_id);
AssertInfo(iter != text_indexes_.end(),
"failed to get text index, text index not found");
if (iter == text_indexes_.end()) {
throw SegcoreError(
ErrorCode::TextIndexNotFound,
fmt::format("text index not found for field {}", field_id.get()));
}
return iter->second.get();
}

Expand Down
6 changes: 5 additions & 1 deletion internal/parser/planparserv2/parser_visitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,10 @@ func (v *ParserVisitor) VisitTextMatch(ctx *parser.TextMatchContext) interface{}
if err != nil {
return err
}
columnInfo := toColumnInfo(column)
if !v.schema.IsFieldTextMatchEnabled(columnInfo.FieldId) {
return fmt.Errorf("field %v does not enable text match", columnInfo.FieldId)
}
if !typeutil.IsStringType(column.dataType) {
return fmt.Errorf("text match operation on non-string is unsupported")
}
Expand All @@ -499,7 +503,7 @@ func (v *ParserVisitor) VisitTextMatch(ctx *parser.TextMatchContext) interface{}
expr: &planpb.Expr{
Expr: &planpb.Expr_UnaryRangeExpr{
UnaryRangeExpr: &planpb.UnaryRangeExpr{
ColumnInfo: toColumnInfo(column),
ColumnInfo: columnInfo,
Op: planpb.OpType_TextMatch,
Value: NewString(queryText),
},
Expand Down
16 changes: 16 additions & 0 deletions internal/parser/planparserv2/plan_parser_v2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/milvus-io/milvus-proto/go-api/v2/commonpb"
"github.com/milvus-io/milvus-proto/go-api/v2/schemapb"
"github.com/milvus-io/milvus/internal/proto/planpb"
"github.com/milvus-io/milvus/pkg/common"
Expand Down Expand Up @@ -53,6 +54,16 @@ func newTestSchema(EnableDynamicField bool) *schemapb.CollectionSchema {
}
}

func enableMatch(schema *schemapb.CollectionSchema) {
for _, field := range schema.Fields {
if typeutil.IsStringType(field.DataType) {
field.TypeParams = append(field.TypeParams, &commonpb.KeyValuePair{
Key: "enable_match", Value: "True",
})
}
}
}

func newTestSchemaHelper(t *testing.T) *typeutil.SchemaHelper {
schema := newTestSchema(true)
schemaHelper, err := typeutil.CreateSchemaHelper(schema)
Expand Down Expand Up @@ -221,6 +232,11 @@ func TestExpr_TextMatch(t *testing.T) {
exprStrs := []string{
`text_match(VarCharField, "query")`,
}
for _, exprStr := range exprStrs {
assertInvalidExpr(t, helper, exprStr)
}

enableMatch(schema)
for _, exprStr := range exprStrs {
assertValidExpr(t, helper, exprStr)
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/util/typeutil/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,14 @@ func (helper *SchemaHelper) IsFieldLoaded(fieldID int64) bool {
return helper.loadFields.Contain(fieldID)
}

func (helper *SchemaHelper) IsFieldTextMatchEnabled(fieldId int64) bool {
sche, err := helper.GetFieldFromID(fieldId)
if err != nil {
return false
}
return CreateFieldSchemaHelper(sche).EnableMatch()
}

func (helper *SchemaHelper) getDefaultJSONField(fieldName string) (*schemapb.FieldSchema, error) {
for _, f := range helper.schema.GetFields() {
if f.DataType == schemapb.DataType_JSON && f.IsDynamic {
Expand Down

0 comments on commit 4245c5b

Please sign in to comment.