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

multi insert #386

Closed
wants to merge 1 commit into from
Closed

multi insert #386

wants to merge 1 commit into from

Conversation

taxio
Copy link

@taxio taxio commented Sep 8, 2018

Hi @aarondl
I'm very sorry for confusing you. I did clean up #374 .

I fixed getting default value.
Maybe, I think it works correctly with psql and mysql.

If you accept this change, I will fix for other DBs as well. (sqlite3, cockroach...)

refs #364

{{- $colTitled := $colName | titleCase -}}
ids := []interface{}{}
for i := 0; i < len(o); i++ {
id := {{$col.Type}}(lastID)+{{$col.Type}}(i)
Copy link
Contributor

Choose a reason for hiding this comment

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

As I known, the step is controlled by a variable auto_increment_increment for MySQL.

https://dev.mysql.com/doc/refman/5.7/en/replication-options-master.html#sysvar_auto_increment_increment

Of course, it would be 1 (default) for most cases.

Copy link

@mbyio mbyio Jan 27, 2019

Choose a reason for hiding this comment

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

I haven't contributed before, so my apologies if this is off-base.

In MySQL, there is no general way to correctly fetch generated auto incrementing IDs when you insert more than one row in the same statement. There are ways to do it if some MySQL options have the right settings: you need to know the value of auto_increment_increment (so you can skip numbers accordingly), and you need to have innodb_autoinc_lock_mode set to 1 or 0 (so the IDs being generated are consecutive; note that the default in MySQL 8 is 2, or non-consecutive, for performance reasons). You could also write lock the table to guarantee you're the only one generating IDs, but that's certainly not what most people would expect to happen.

I (and probably other people too) have to do bulk inserts for performance reasons. In AWS (and probably most cloud providers), the minimum latency for executing a single query is ~1 ms. So if you insert 10,000 rows using individual insert statements, it will take at least 10 seconds, and it will monopolize DB resources. If you do it as a bulk insert, it will take only a handful of milliseconds.

Because bulk insert support is very useful (and I suspect most people will just implement it themselves anyway if it isn't provided by the library), I suggest implementing it with the caveat that, in MySQL, you won't be able to get auto generated IDs back if any were generated.

Copy link
Member

Choose a reason for hiding this comment

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

Well - this is now made easier now that each database driver can create extra functions that differ from the core completely. Perhaps we could use that for bulk insert? Though I'm still on the fence about it in general. It's just such a departure from what makes sqlboiler useful. If you're not doing the "get the id back" part, and binding other default values back into the struct, why use it at all for insertion?

{{if not .Dialect.UseLastInsertID -}}
retQuery = " RETURNING {{.LQ}}" + strings.Join(returnColumns, "{{.RQ}},{{.LQ}}") + "{{.RQ}} "
{{else -}}
retQuery = fmt.Sprintf("SELECT {{.LQ}}%s{{.RQ}} FROM {{$schemaTable}} WHERE {{.LQ}}%s{{.RQ}} IN %s",
Copy link
Contributor

Choose a reason for hiding this comment

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

select ... in ... cannot make the same order between your primary key and return record.

Therefore, i think you have to add a ORDER BY PK clause to keep the order.

{{end}}
{{end}}
{{- end}}
nzDefaults := queries.NonZeroDefaultSet({{$alias.DownSingular}}ColumnsWithDefault, row)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure that this can work. Since you have to give the columns up-front (the list of the columns you want to insert), but if you're using boil.Infer it can change the columns that are being inserted and returned.

For example I have a user struct with a name, and a money amount that's 0 by default. And I want to insert two users using this method:

user1 := &models.User{Name: "a"}
user2 := &models.User{Name: "b", Money: 100}
slice := models.UserSlice{user1, user2}

err := slice.InsertAll(..., boil.Infer)

In this case, the insert and return statements for user1 must look like:

insert into users (name) values ('a');
select id, money from users where id = LAST_INSERT_ID();

And user2:

insert into users (name, money) values ('a', 100);
select id from users where id = LAST_INSERT_ID();

Keep in mind I'm just doing the return query for illustrative purposes, in reality we get the last insert id from Result.LastInsertId. But the point is that the columns can change depending on what values the structs have, so how are we to make this method in this way? I think if anything it would have to follow in the footsteps of UpdateAll which takes a list of columns that it needs to update. Or maybe we just limit it so that boil.Infer is somehow unusable here? Not sure. We need to figure out what to do here though.

@sachnk
Copy link

sachnk commented Jun 7, 2019

Hey --

We're looking for multi-row insert functionality. Did this ever make it in?

@aarondl
Copy link
Member

aarondl commented Jun 7, 2019

Nope

@aarondl
Copy link
Member

aarondl commented Aug 26, 2019

Stale

@aarondl aarondl closed this Aug 26, 2019
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