Repository: milvus-io/milvus-sdk-go
Pull request #472 (2023-05-26 05:31:58)

Ignore AutoID pk when converting rows to columns (#470)
/kind improvement

sre-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/milvus-io/milvus-sdk-go/pull/472#" title="Author self-approved">congqixia</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=milvus-io%2Fmilvus-sdk-go).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/milvus-io/milvus-sdk-go/blob/master/OWNERS)~~ [congqixia]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/milvus-io/milvus-sdk-go/pull/472?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=milvus-io) Report
> Merging [#472](https://app.codecov.io/gh/milvus-io/milvus-sdk-go/pull/472?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=milvus-io) (5f79242) into [master](https://app.codecov.io/gh/milvus-io/milvus-sdk-go/commit/02d910f41be4519a625b2a4c1a6c822d5d3191e4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=milvus-io) (02d910f) will **increase** coverage by `0.02%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://app.codecov.io/gh/milvus-io/milvus-sdk-go/pull/472/graphs/tree.svg?width=650&height=150&src=pr&token=WJK43PV390&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=milvus-io)](https://app.codecov.io/gh/milvus-io/milvus-sdk-go/pull/472?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=milvus-io)

```diff
@@            Coverage Diff             @@
##           master     #472      +/-   ##
==========================================
+ Coverage   84.06%   84.09%   +0.02%     
==========================================
  Files          36       36              
  Lines        3314     3319       +5     
==========================================
+ Hits         2786     2791       +5     
  Misses        425      425              
  Partials      103      103              
```


| [Impacted Files](https://app.codecov.io/gh/milvus-io/milvus-sdk-go/pull/472?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=milvus-io) | Coverage Δ | |
|---|---|---|
| [entity/rows.go](https://app.codecov.io/gh/milvus-io/milvus-sdk-go/pull/472?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=milvus-io#diff-ZW50aXR5L3Jvd3MuZ28=) | `87.04% <100.00%> (+0.26%)` | :arrow_up: |

yah01: /lgtm

Patch:
diff --git a/entity/rows.go b/entity/rows.go
index 244a271..b074fc0 100644
--- a/entity/rows.go
+++ b/entity/rows.go
@@ -265,6 +265,10 @@ func RowsToColumns(rows []Row, schemas ...*Schema) ([]Column, error) {
 
 	nameColumns := make(map[string]Column)
 	for _, field := range sch.Fields {
+		// skip auto id pk field
+		if field.PrimaryKey && field.AutoID {
+			continue
+		}
 		switch field.DataType {
 		case FieldTypeBool:
 			data := make([]bool, 0, rowsLen)
@@ -342,9 +346,16 @@ func RowsToColumns(rows []Row, schemas ...*Schema) ([]Column, error) {
 		}
 
 		for idx, field := range sch.Fields {
+			// skip dynamic field if visible
 			if isDynamic && field.IsDynamic {
 				continue
 			}
+			// skip auto id pk field
+			if field.PrimaryKey && field.AutoID {
+				// remove pk field from candidates set, avoid adding it into dynamic column
+				delete(set, field.Name)
+				continue
+			}
 			column := nameColumns[field.Name]
 
 			candi, ok := set[field.Name]


Test patch:
diff --git a/entity/rows_test.go b/entity/rows_test.go
index 0abe34b..a9e0333 100644
--- a/entity/rows_test.go
+++ b/entity/rows_test.go
@@ -212,27 +212,42 @@ type ValidStructWithNamedTag struct {
 	Vector [16]float32 `milvus:"name:vector"`
 }
 
-func TestRowsToColumns(t *testing.T) {
-	t.Run("valid cases", func(t *testing.T) {
+type RowsSuite struct {
+	suite.Suite
+}
+
+func (s *RowsSuite) TestRowsToColumns() {
+	s.Run("valid_cases", func() {
 
 		columns, err := RowsToColumns([]Row{&ValidStruct{}})
-		assert.Nil(t, err)
-		assert.Equal(t, 10, len(columns))
+		s.Nil(err)
+		s.Equal(10, len(columns))
 
 		columns, err = RowsToColumns([]Row{&ValidStruct2{}})
-		assert.Nil(t, err)
-		assert.Equal(t, 3, len(columns))
+		s.Nil(err)
+		s.Equal(3, len(columns))
+	})
 
+	s.Run("auto_id_pk", func() {
+		type AutoPK struct {
+			RowBase
+			ID     int64     `milvus:"primary_key;auto_id"`
+			Vector []float32 `milvus:"dim:32"`
+		}
+		columns, err := RowsToColumns([]Row{&AutoPK{}})
+		s.Nil(err)
+		s.Require().Equal(1, len(columns))
+		s.Equal("Vector", columns[0].Name())
 	})
 
-	t.Run("invalid cases", func(t *testing.T) {
+	s.Run("invalid_cases", func() {
 		// empty input
 		_, err := RowsToColumns([]Row{})
-		assert.NotNil(t, err)
+		s.NotNil(err)
 
 		// incompatible rows
 		_, err = RowsToColumns([]Row{&ValidStruct{}, &ValidStruct2{}})
-		assert.NotNil(t, err)
+		s.NotNil(err)
 
 		// schema & row not compatible
 		_, err = RowsToColumns([]Row{&ValidStruct{}}, &Schema{
@@ -243,14 +258,10 @@ func TestRowsToColumns(t *testing.T) {
 				},
 			},
 		})
-		assert.NotNil(t, err)
+		s.NotNil(err)
 	})
 }
 
-type RowsSuite struct {
-	suite.Suite
-}
-
 func (s *RowsSuite) TestDynamicSchema() {
 	s.Run("all_fallback_dynamic", func() {
 		columns, err := RowsToColumns([]Row{&ValidStruct{}},