/diesel

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

PG upsert + batch insert + empty vec = panic! #797

Closed
sgrif opened this Issue Mar 13, 2017 · 0 comments

Comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
1 participant
@sgrif
@sgrif
Member

sgrif commented Mar 13, 2017

To reproduce:

let new_users: Vec<NewUser> = Vec::new();
insert(&new_users.on_conflict_do_nothing()).into(users::table).execute(&conn)

@sgrif sgrif referenced this issue Mar 13, 2017

Merged

Do less work in the inner loop of `assign_commits` #82

sgrif added a commit that referenced this issue Sep 16, 2017

@sgrif
rm `IntoInsertStatement` 
This gets rid of `IntoInsertStatement`, and turns `BatchInsertStatement`
into the only visible type for constructing queries. In its
implementations of `LoadDsl` and `ExecuteDsl`, it ends up constructing
`InsertStatement` which is what actually performs the execution.

The names are now super poor and misleading. The reason I've opted not
to rename them in this commit is that I actually think I'm going to end
up getting rid of `BatchInsertStatement` entirely, and only have
`InsertStatement`. If we don't end up going that route, I'm going to
rename `InsertStatement` to `ExecutableInsertStatement` and rename
`BatchInsertStatement` to just `InsertStatement`.

Aside from simplifying our codebase, this change also makes sure that
all insert statements constructed will include the 0-row checking case.
I think we can eventually just move this into the `QueryFragment` impl
by doing `SELECT returning_clause FROM table WHERE 1=0`. This would mean
the only thing we have to special case is SQLite batch insert.

Fixes #797
Loading status checks…
51d6480

@sgrif sgrif referenced this issue Sep 16, 2017

Closed

rm `IntoInsertStatement` #1173

@sgrif sgrif closed this in 349aa4f Sep 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment