ignore autoinc column on update
We are ignoring autoinc column for inserts, we could to the same for updates. Either like this:
FoobarTable.filter(_.id === someFoobar.id).update(someFoobar)
where newRow's autoinc column is automatically excluded or even like this
FoobarTable.update(someFoobar)
where it is automatically used as a discriminator (as requested in #570).
I second this, we're currently generating our own UpdateProjections to remove an auto updated timestamp column.
I vote for option 2:
FoobarTable.update(someFoobar)
What does a timestamp column have to do with autoinc? On Jan 17, 2014 12:12 PM, "brendanator" [email protected] wrote:
I second this, we're currently generating our own UpdateProjections to remove an auto updated timestamp column.
I vote for option 2:
FoobarTable.update(someFoobar)
— Reply to this email directly or view it on GitHubhttps://github.com/slick/slick/issues/601#issuecomment-32624472 .
We have a database standard where all of our tables have an auto-generated timestamp column on them. When you insert or update to a table this field will be set to the current timestamp. Therefore this could easily be described as auto-incrementing.
If you attempt to set this field in an insert or update statement it will fail with an error, therefore we've created our own projections to not set this field
N.B. We're working with an existing db schema
Maybe I've created a bit of confusion here by suggesting
FoobarTable.update(someFoobar)
I was assuming a different field to the timestamp was marked as being the primary key.
It may also be of interesting to hear that we use this timestamp column to implement optimistic locking on top of slick
Here is an instance of that problem: http://stackoverflow.com/questions/24631166/slick-update-of-field-on-sqlserver-jtds
A lot of preliminary work was done for soft inserts and insertAndUpdate support in 2.0 and 2.1. It should be possible for updates to use the same mechanism as for inserts, i.e. optionally compile to two diferent statements for forced and soft update, and use different result converters for them that match the generated column list.
Hello,
Just checked the status of this one because I saw earlier that it was scheduled for 3.2.0 but it's missing from 3.2.0-RC1. Apparently is was recently changed to be included in 3.2.1 instead.
@szeiger can you please confirm the milestone or give some insights on the current status of this improvement? At the present time, code that is designed to be ported across databases may not execute generic update() operations over a given SQLServer table that contains at least one auto-inc column.
I could try to modify the update statement for such use cases in order to remove the auto-inc column(s) from the statement itself, but that would be kind of clumsy. I really prefer to have this done by Slick itself.
@gonmarques so yes, this has been moved to 3.2.1 to get 3.2.0 out. I don't think specific work has been going on into this direction further than what was described, but @szeiger may know better.
It's still not clear - at least to me - if this issue is really an improvement or if it should have been labeled with some higher priority instead.
True fact is that Slick is generating erroneous SQL for a very specific RDBMS - SQLServer in this case - and such problematic generation could well be avoided using the metadata we have in our hands.
It may well be the very legitimate case to put the responsibility on the user. After all he/she is defining an action that instructs Slick to update all the record's columns. But on the other hand, we know before hand that auto-inc columns are probably to be left untouched - especially in this case where erroneous SQL is generated for this particular RDBMS.
We should also no forget that - as @cvogt mentions in the first comment - we are already "ignoring autoinc column for inserts". In order to keep things consistent, shouldn't this update() action also exclude auto-inc columns?
Should this be enough to force a new Release Candidate for 3.2.0? I understand that the combination of Slick + SQLServer + need for the update() action probably represents a little percentage of the overall use cases, but nevertheless it feels like something is being released with a well known issue left unresolved.
We only force a new RC if an issue is a regression. I agree that this would be great to have. Things like this generally get fixed when either
- one of the maintainers looks into it and community feedback is a guiding force here, but available time, interest and personal priority is as well. Most of us are volunteers.
- an open source contributor fixes it
- a Lightbend customer is pushing for it
Apparently, it's still missing even in 3.2.1. Is there is any hope that it'll show up in the next release?
Hi, this looks like the problem I'm currently having, I can't get the following behaviour to work correctly:
Column A with AutoIncrement and NOT NULL
Insert with Column A set, should insert with the set value Insert without Column A set, should insert with next value
If I make the column nullable (in the database), I can get it to work this way, but then I can't use it in an index
@Daxten your case is unrelated. You can ask for help on the gitter.im slick/slick channel.
I'm also looking forward to this We have to use insertOrUpdate instead of update because our PostgreSQL does not allow updating of autogenerated primary keys.
Is this difficult to implement? Could I help?
I encountered a similar problem. We have an auto-increment column that gets updated on update or upsert operations. Any plans to address this? I see that upsert and updateInsert compilers are InsertCompiler.AllColumns rather than InsertCompiler.NonAutoInc:
lazy val upsertCompiler = QueryCompiler(Phase.assignUniqueSymbols, Phase.inferTypes, new InsertCompiler(InsertCompiler.AllColumns), new JdbcInsertCodeGen(createUpsertBuilder))
lazy val updateInsertCompiler = QueryCompiler(Phase.assignUniqueSymbols, Phase.inferTypes, new InsertCompiler(InsertCompiler.AllColumns), new JdbcInsertCodeGen(createUpdateInsertBuilder))
Is this the issue?
you can checkout the latest version and use my test from this PR to check if it fixes it: https://github.com/slick/slick/pull/1930