From 923a661dfee5820899033a94acae24e6ad7c250d Mon Sep 17 00:00:00 2001 From: SimFG Date: Fri, 22 Nov 2024 11:42:32 +0800 Subject: [PATCH] enhance: filter the fields instead of create a new response obj (#37845) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /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 --- internal/proxy/meta_cache.go | 27 ++++------------------ internal/proxy/meta_cache_test.go | 37 +++++++++++++++++++++++-------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/internal/proxy/meta_cache.go b/internal/proxy/meta_cache.go index 407f6aee7c..382c9380f8 100644 --- a/internal/proxy/meta_cache.go +++ b/internal/proxy/meta_cache.go @@ -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) { diff --git a/internal/proxy/meta_cache_test.go b/internal/proxy/meta_cache_test.go index 7ec54463bd..cfc3cb291c 100644 --- a/internal/proxy/meta_cache_test.go +++ b/internal/proxy/meta_cache_test.go @@ -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) } @@ -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{}, @@ -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{}, @@ -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{}, @@ -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{}, @@ -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{}, @@ -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{}, @@ -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{}, @@ -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{}, @@ -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{},