Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add insert ignore #39

Merged
merged 3 commits into from
Nov 11, 2019
Merged

Add insert ignore #39

merged 3 commits into from
Nov 11, 2019

Conversation

Hadesonly
Copy link

@Hadesonly Hadesonly commented Nov 10, 2019

Added REPLACE INTO and INSERT IGNORE INTO.
Almost the same with InsertInto, and easily pass the tests.

@Hadesonly
Copy link
Author

plz cr. @huandu

struct.go Outdated Show resolved Hide resolved
struct_test.go Outdated Show resolved Hide resolved
struct.go Outdated
Comment on lines 223 to 224
//`InsertIgnoreInto` is the Same with `InsertInto`, except for inserting with the existing primary key or unique key.
//When inserting with the existing primary key or unique key, the db would ignore this operation with no error.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some wording issues. All newly added docs have same issues.

  • Leave a blank after every //.
  • The comment should start with a definition of this func, e.g. ReplaceIntoForTag creates a new InsertBuilder for REPLACE with table name. We need to provide enough information to doc readers. We should not force them to check another func to understand how to use this func.
  • the Same with -> similar to (and mind the case please)
  • The sentence except for ... is not true. The responsibility of this package is only to build SQL without any execution. We need to tell readers what would happen to the SQL we build, e.g. using INSERT IGNORE INTO instead of INSERT INTO.
  • As of previous review comment, the sentence When inserting with ... should be deleted. I believe package users are DB experts and know how to use INSERT IGNORE INTO.

You can communicate with me for doc in person before submitting a new version. It would be more effective for us.

struct.go Outdated
Comment on lines 351 to 396
if s.taggedFields == nil {
return ib
}

fields, ok := s.taggedFields[tag]

if !ok {
return ib
}

vs := make([]reflect.Value, 0, len(value))

for _, item := range value {
v := reflect.ValueOf(item)
v = dereferencedValue(v)

if v.Type() == s.structType {
vs = append(vs, v)
}
}

if len(vs) == 0 {
return ib
}

cols := make([]string, 0, len(fields))
values := make([][]interface{}, len(vs))

for _, f := range fields {
cols = append(cols, f)
name := s.fieldAlias[f]

for i, v := range vs {
data := v.FieldByName(name).Interface()
values[i] = append(values[i], data)
}
}

cols = s.quoteFields(cols)
ib.Cols(cols...)

for _, value := range values {
ib.Values(value...)
}

return ib
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dup code.

@Hadesonly
Copy link
Author

solved the problems above,and plz cr again @huandu

Copy link
Owner

@huandu huandu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@huandu huandu merged commit a88ee9b into huandu:master Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants