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

map insert support return increment id #6662

Merged
merged 7 commits into from
Nov 15, 2023

Conversation

FangSqing
Copy link
Contributor

  • Do only one thing

  • Non breaking API changes

  • Tested

What did this pull request do?

This pull request aims to support returning the primary key value, when using a map (or map list) to call GORM to insert record, in auto-increment primary key case.

If the primary key value is not set, the returned map or map list will be filled with a custom column "@id".

User Case Description

The following cases are all using auto-increment primary key
Case 1:
  Input: map or map list without primary key
  Output: map or map list will be filled with "@id"
Case 2:
  Input: map or map list with primary key
  Output: map or map list will be filled with "@id"

The following cases are all using non-auto-increment primary key
Case 1:
  Input: map or map list without primary key
  Output: error, the same as before.
Case 2:
  Input: map or map list with primary key
  Output: the same as before

Last but not least, it has no impact on non-auto-increment primary keys.

if db.Statement.Schema.PrioritizedPrimaryField == nil || !db.Statement.Schema.PrioritizedPrimaryField.HasDefaultValue {
return
}

insertID, err := result.LastInsertId()
Copy link
Member

Choose a reason for hiding this comment

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

In some cases, the Dest might be a map value, but db.Statement.Schema is not nil, for example:

db.Model(&User{}).First(map[string]interface{}{})

Copy link
Member

Choose a reason for hiding this comment

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

btw, can you add some test cases for the feature, thank you.

@FangSqing FangSqing force-pushed the feat/map_increment_id branch 2 times, most recently from 65f2f63 to 54f0b22 Compare November 8, 2023 09:18
@FangSqing FangSqing force-pushed the feat/map_increment_id branch 2 times, most recently from fa6a876 to 98fb23f Compare November 14, 2023 13:24
@jinzhu jinzhu merged commit 3207ad6 into go-gorm:master Nov 15, 2023
34 checks passed
Copy link
Member

@a631807682 a631807682 left a comment

Choose a reason for hiding this comment

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

https://github.com/go-gorm/gorm/pull/6662/files#diff-7539aa7c170a85138fa67c7846b65fda95a51169bcf8637d5961ea570307d755R52

Since the detection of RETURNING does not support map, an error will occur if the driver only supports returning but not LastInsertId (the database usually only supports one).
We need to change the detection of RETURNING if we want to support this feat.

related to #6812

@sujit-baniya
Copy link

This PR creates bug for inserting rows (slice of map) in batches for Postgres

Error: LastInsertId is not supported by this driver

@FangSqing
Copy link
Contributor Author

Sorry, I’ll check it and fix it with a new PR.

@thesiti92
Copy link

I also cannot update gorm because this breaks inserting a map of values in postgres. Thanks for working on a fix!

@sujit-baniya
Copy link

For the time being, till the issue is fixed, I've been checking for the error message.

master...sujit-baniya:gorm:master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants