Skip to content

Commit

Permalink
enhance: filter the fields instead of create a new response obj (milv…
Browse files Browse the repository at this point in the history 
…us-io#37845)

/kind improvement

Here you only need to filter out the system fields, and you don’t need
to recreate a response, because recreating the response will cause this
part to be easily missed when adding fields later.

Signed-off-by: SimFG <[email protected]>
  • Loading branch information
@SimFG @JsDove
SimFG authored and JsDove committed Nov 26, 2024
1 parent a3a94ab commit b0c4938
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 32 deletions.
27 changes: 4 additions & 23 deletions internal/proxy/meta_cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -706,33 +706,14 @@ func (m *MetaCache) describeCollection(ctx context.Context, database, collection
if err != nil {
return nil, err
}
resp := &milvuspb.DescribeCollectionResponse{
Status: coll.Status,
Schema: &schemapb.CollectionSchema{
Name: coll.Schema.Name,
Description: coll.Schema.Description,
AutoID: coll.Schema.AutoID,
Fields: make([]*schemapb.FieldSchema, 0),
Functions: make([]*schemapb.FunctionSchema, 0),
EnableDynamicField: coll.Schema.EnableDynamicField,
},
CollectionID: coll.CollectionID,
VirtualChannelNames: coll.VirtualChannelNames,
PhysicalChannelNames: coll.PhysicalChannelNames,
CreatedTimestamp: coll.CreatedTimestamp,
CreatedUtcTimestamp: coll.CreatedUtcTimestamp,
ConsistencyLevel: coll.ConsistencyLevel,
DbName: coll.GetDbName(),
Properties: coll.Properties,
}
userFields := make([]*schemapb.FieldSchema, 0)
for _, field := range coll.Schema.Fields {
if field.FieldID >= common.StartOfUserFieldID {
resp.Schema.Fields = append(resp.Schema.Fields, field)
userFields = append(userFields, field)
}
}

resp.Schema.Functions = append(resp.Schema.Functions, coll.Schema.Functions...)
return resp, nil
coll.Schema.Fields = userFields
return coll, nil
}

func (m *MetaCache) showPartitions(ctx context.Context, dbName string, collectionName string, collectionID UniqueID) (*milvuspb.ShowPartitionsResponse, error) {
Expand Down
37 changes: 28 additions & 9 deletions internal/proxy/meta_cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,25 @@ type MockRootCoordClientInterface struct {
listPolicy func(ctx context.Context, in *internalpb.ListPolicyRequest) (*internalpb.ListPolicyResponse, error)
}

func EqualSchema(t *testing.T, expect, actual *schemapb.CollectionSchema) {
assert.Equal(t, expect.AutoID, actual.AutoID)
assert.Equal(t, expect.Description, actual.Description)
assert.Equal(t, expect.Name, actual.Name)
assert.Equal(t, expect.EnableDynamicField, actual.EnableDynamicField)
assert.Equal(t, len(expect.Fields), len(actual.Fields))
for i := range expect.Fields {
assert.Equal(t, expect.Fields[i], actual.Fields[i])
}
assert.Equal(t, len(expect.Functions), len(actual.Functions))
for i := range expect.Functions {
assert.Equal(t, expect.Functions[i], actual.Functions[i])
}
assert.Equal(t, len(expect.Properties), len(actual.Properties))
for i := range expect.Properties {
assert.Equal(t, expect.Properties[i], actual.Properties[i])
}
}

func (m *MockRootCoordClientInterface) IncAccessCount() {
atomic.AddInt32(&m.AccessCount, 1)
}
Expand Down Expand Up @@ -212,7 +231,7 @@ func TestMetaCache_GetCollection(t *testing.T) {
schema, err := globalMetaCache.GetCollectionSchema(ctx, dbName, "collection1")
assert.Equal(t, rootCoord.GetAccessCount(), 1)
assert.NoError(t, err)
assert.Equal(t, schema.CollectionSchema, &schemapb.CollectionSchema{
EqualSchema(t, schema.CollectionSchema, &schemapb.CollectionSchema{
AutoID: true,
Fields: []*schemapb.FieldSchema{},
Functions: []*schemapb.FunctionSchema{},
Expand All @@ -225,7 +244,7 @@ func TestMetaCache_GetCollection(t *testing.T) {
schema, err = globalMetaCache.GetCollectionSchema(ctx, dbName, "collection2")
assert.Equal(t, rootCoord.GetAccessCount(), 2)
assert.NoError(t, err)
assert.Equal(t, schema.CollectionSchema, &schemapb.CollectionSchema{
EqualSchema(t, schema.CollectionSchema, &schemapb.CollectionSchema{
AutoID: true,
Fields: []*schemapb.FieldSchema{},
Functions: []*schemapb.FunctionSchema{},
Expand All @@ -240,7 +259,7 @@ func TestMetaCache_GetCollection(t *testing.T) {
schema, err = globalMetaCache.GetCollectionSchema(ctx, dbName, "collection1")
assert.Equal(t, rootCoord.GetAccessCount(), 2)
assert.NoError(t, err)
assert.Equal(t, schema.CollectionSchema, &schemapb.CollectionSchema{
EqualSchema(t, schema.CollectionSchema, &schemapb.CollectionSchema{
AutoID: true,
Fields: []*schemapb.FieldSchema{},
Functions: []*schemapb.FunctionSchema{},
Expand Down Expand Up @@ -300,7 +319,7 @@ func TestMetaCache_GetCollectionName(t *testing.T) {
schema, err := globalMetaCache.GetCollectionSchema(ctx, dbName, "collection1")
assert.Equal(t, rootCoord.GetAccessCount(), 1)
assert.NoError(t, err)
assert.Equal(t, schema.CollectionSchema, &schemapb.CollectionSchema{
EqualSchema(t, schema.CollectionSchema, &schemapb.CollectionSchema{
AutoID: true,
Fields: []*schemapb.FieldSchema{},
Functions: []*schemapb.FunctionSchema{},
Expand All @@ -313,7 +332,7 @@ func TestMetaCache_GetCollectionName(t *testing.T) {
schema, err = globalMetaCache.GetCollectionSchema(ctx, dbName, "collection2")
assert.Equal(t, rootCoord.GetAccessCount(), 2)
assert.NoError(t, err)
assert.Equal(t, schema.CollectionSchema, &schemapb.CollectionSchema{
EqualSchema(t, schema.CollectionSchema, &schemapb.CollectionSchema{
AutoID: true,
Fields: []*schemapb.FieldSchema{},
Functions: []*schemapb.FunctionSchema{},
Expand All @@ -328,7 +347,7 @@ func TestMetaCache_GetCollectionName(t *testing.T) {
schema, err = globalMetaCache.GetCollectionSchema(ctx, dbName, "collection1")
assert.Equal(t, rootCoord.GetAccessCount(), 2)
assert.NoError(t, err)
assert.Equal(t, schema.CollectionSchema, &schemapb.CollectionSchema{
EqualSchema(t, schema.CollectionSchema, &schemapb.CollectionSchema{
AutoID: true,
Fields: []*schemapb.FieldSchema{},
Functions: []*schemapb.FunctionSchema{},
Expand All @@ -354,7 +373,7 @@ func TestMetaCache_GetCollectionFailure(t *testing.T) {

schema, err = globalMetaCache.GetCollectionSchema(ctx, dbName, "collection1")
assert.NoError(t, err)
assert.Equal(t, schema.CollectionSchema, &schemapb.CollectionSchema{
EqualSchema(t, schema.CollectionSchema, &schemapb.CollectionSchema{
AutoID: true,
Fields: []*schemapb.FieldSchema{},
Functions: []*schemapb.FunctionSchema{},
Expand All @@ -364,7 +383,7 @@ func TestMetaCache_GetCollectionFailure(t *testing.T) {
rootCoord.Error = true
// should be cached with no error
assert.NoError(t, err)
assert.Equal(t, schema.CollectionSchema, &schemapb.CollectionSchema{
EqualSchema(t, schema.CollectionSchema, &schemapb.CollectionSchema{
AutoID: true,
Fields: []*schemapb.FieldSchema{},
Functions: []*schemapb.FunctionSchema{},
Expand Down Expand Up @@ -429,7 +448,7 @@ func TestMetaCache_ConcurrentTest1(t *testing.T) {
// GetCollectionSchema will never fail
schema, err := globalMetaCache.GetCollectionSchema(ctx, dbName, "collection1")
assert.NoError(t, err)
assert.Equal(t, schema.CollectionSchema, &schemapb.CollectionSchema{
EqualSchema(t, schema.CollectionSchema, &schemapb.CollectionSchema{
AutoID: true,
Fields: []*schemapb.FieldSchema{},
Functions: []*schemapb.FunctionSchema{},
Expand Down

0 comments on commit b0c4938

Please sign in to comment.