Hi, I think the PK is not enough, you have to add the WHERE constraint on every UNIQUE column (with OR). See the example in my previous mail.
Regards, Ivan <[email protected]> ezt írta (időpont: 2016. aug. 19., P, 14:02): > Hello Ivan, > > thanks for testing. I think you're right, this is a dangerous behaviour. > > In my new commit I added that every column from SET thats also part of > the primary key of the first table of the command is added as WHERE. > In my first tests it works. The implementation might still be a little > bit hacky. > > What do you tink? > > - jan > > Zitat von ivan nemeth <[email protected]>: > > > Hi, > > > > I think the problem is more complicated than just naming the method. > > > > Take my previous example, with a small change, and define a UNIQUE > > constraint on the NAME column. > > > > ID NAME > > ---------------- > > 1 A > > 2 B > > > > After executing this command: > > > > DBCommand cmd = db.createCommand(); > > cmd.set(T.ID.to <http://db.upsert_test.id.to/>(3)); > > cmd.set(T.NAME.to <http://db.upsert_test.name.to/>("A")); > > db.executeInsertOrUpdate(cmd, conn); > > > > the result will be: > > > > ID NAME > > ---------------- > > 3 A > > 2 B > > > > To achieve the same result with the fallback method, one has to add the > > following constraint > > > > cmd.where(T.ID.is(3).or(T.NAME.is("A"))); > > > > ------------ > > > > Although it would be really a great feature, I see some problems with the > > implementation: > > > > 1. I think the main goal of the DBCommand API is that the same code > should > > do the same thing on every driver. In the example above if you omit the > > constraint, MySql implementetion will be ok, but the fallback won't. > > > > 2. To achieve the same result, it is required that the coder adds a > > constraint which OR's *the PK and all UNIQUE columns* with the > > corresponding value in the update statement (this is what MySql actually > > does according to the documentation). This may lead a lot of errors if > > someone forgets to add a constraint, tests it on MySql and than moves to > > other driver. > > > > 3. For me the MySql approach that takes into account not only the PK > > conflict, but all UQ conflicts, is a bit strange. I don't know how other > > coders think about it, but for me in OOP a record is identified in the > > table with it's PK. So in the example above I would expect insertOrUpdate > > to throw a duplicate key exception, and not updating the PK of the first > > row. I think this is inconsistent with an object oriented approach, as in > > this case an object disappears from the db after a simple update or > insert > > operation. Maybe this behaviour is self-evident for MySql programmers, > but > > for me it's not. > > > > > > > > > > Regards, > > Ivan > > > > > > > > > > Rainer Döbele <[email protected]> ezt írta (időpont: 2016. aug. 12., P, > > 11:56): > > > >> IMO updateOrInsert is what we want, just from the plain understanding of > >> the name. > >> > >> Rainer > >> > >> > >> > from: [email protected] [mailto:[email protected]] > >> > to: [email protected] > >> > subject: Re: Proposal: Extending DBCommand with "insertOrUpdate" > >> > ("UPSERT") > >> > > >> > Hello Ivan, > >> > > >> > thanks for testing. > >> > > >> > I think youre right - its a little bit confusing. For me it was always > >> clear I need > >> > to add a WHERE-clause because, I wanted to do an update, if that > "fails" > >> (as > >> > in "nothing updated"), an INSERT. MySQL uses an INSERT as initial > >> statement, > >> > so I called the new methods "insertOrUpdate" > >> > instead of "updateOrInsert". Maybe thats the point. Do you think its > more > >> > clear that you have to define a WHERE, if the method was called > >> > "updateOrInsert"? > >> > > >> > - jan > >> > > >> > Am 2016-08-11 19:43, schrieb ivan nemeth: > >> > > Hi Jan, > >> > > > >> > > I've tested it on MySql and I have one problem with it. > >> > > > >> > > 1. The insertOrUpdate doesn't do the same as the fallback method in > >> > > case of an update and if I don't define any where conditions. > >> > > > >> > > I have a table, with columns ID (PK) and NAME and the following > >> > > initial values. > >> > > > >> > > ID NAME > >> > > ---------------- > >> > > 1 A > >> > > 2 B > >> > > > >> > > *1. InsertOrUpdate* > >> > > > >> > > > >> > > DBCommand cmd = db.createCommand(); > >> > > cmd.set(T.ID.to <http://db.UPSERT_TEST.ID.to>(2)); > >> > > cmd.set(T.NAME.to <http://db.UPSERT_TEST.NAME.to>("C")); > >> > > db.executeInsertOrUpdate(cmd, conn); > >> > > > >> > > After executing it, the table looks OK: > >> > > > >> > > ID NAME > >> > > ---------------- > >> > > 1 A > >> > > 2 C > >> > > > >> > > *2. Fallback => try update, then insert* > >> > > > >> > > cmd.set(T.ID.to <http://db.UPSERT_TEST.ID.to>(2)); > >> > > cmd.set(T.NAME.to <http://db.UPSERT_TEST.NAME.to>("C")); > >> > > int count = db.executeUpdate(cmd, conn); if (count < 1) { count += > >> > > db.executeInsert(cmd, conn); } > >> > > > >> > > This throws a Duplicate key exception in the executeUpdate() method, > >> > > because it tries to update ALL records (there is no update condition > >> > > defined). > >> > > If I add the condition cmd.where(T.ID.i > >> > > <http://db.upsert_test.name.to/>s(2), > >> > > it's OK, but I think it's problematic that insertOrUpdate behaves > >> > > differently with different drivers. > >> > > (Maybe the where conditions on the PK and UQ constraints should be > >> > > added to the update statement automatically in the fallback method?) > >> > > > >> > > 2. MSSQL implementation > >> > > > >> > > On MSSQL I see much harder to implement because you have to make > >> > some > >> > > kind of join with a temporary table on the PKs and UQs. (MERGE) > >> > > > >> > > ----------- > >> > > > >> > > Regards, > >> > > Ivan > >> > > > >> > > > >> > > > >> > > <[email protected]> ezt írta (időpont: 2016. aug. 9., K, 7:57): > >> > > > >> > >> I pushed it to the EMPIREDB-247 branch. > >> > >> > >> > >> Ivan: can you test if that implementation works for you usecase, > too? > >> > >> > >> > >> - jan > >> > >> > >> > >> Zitat von [email protected]: > >> > >> > >> > >> > Hello Rainer, > >> > >> > > >> > >> > here is an example of a MERGE INTO from Stackoverflow: > >> > >> > http://stackoverflow.com/a/2692441. I dont have access to a > Oracle > >> > >> > DB so I cant test it. > >> > >> > > >> > >> > I created a ticket and will create a branch in the git > repository. > >> > >> > > >> > >> > - jan > >> > >> > > >> > >> > Zitat von Rainer Döbele <[email protected]>: > >> > >> > > >> > >> >> Hi Jan, > >> > >> >> > >> > >> >> I appreciate the idea. > >> > >> >> What worries me a bit is that we only know how to implement it > for > >> > >> >> 2 > >> > >> DBMS. > >> > >> >> Ideally we would have broader support or workarounds for other > >> > >> >> systems instead of throwing an Exception. > >> > >> >> > >> > >> >> Perhaps we should at least give a client the opportunity to > check > >> > >> >> beforehand if this features is available. > >> > >> >> This can be done by extending the enum DBDriverFeature. > >> > >> >> > >> > >> >> Have you already thought what your Oracle version would look > like? > >> > >> >> > >> > >> >> But if you have thought it through carefully and still think it > is > >> > >> >> a good idea, then you are welcome to go ahead, create a ticket > and > >> > >> >> implement it. > >> > >> >> > >> > >> >> Regards, > >> > >> >> Rainer > >> > >> >> > >> > >> >> > >> > >> >>> -----Ursprüngliche Nachricht----- > >> > >> >>> Von: [email protected] [mailto:[email protected]] > >> > >> >>> Gesendet: Montag, 8. August 2016 13:01 > >> > >> >>> An: [email protected] > >> > >> >>> Betreff: Proposal: Extending DBCommand with "insertOrUpdate" > >> > >> >>> ("UPSERT") > >> > >> >>> > >> > >> >>> Hello, > >> > >> >>> > >> > >> >>> I'm currently writing a few sync jobs. The naive approach was > to > >> > >> >>> run an UPDATE first and an INSERT with same DBCommand object if > >> > >> >>> nothing was updated. This works but is slow. > >> > >> >>> > >> > >> >>> I figured there is a INSERT ... ON DUPLICATE KEY UPDATE Syntax > in > >> > >> >>> MySQL > >> > >> >>> (http://dev.mysql.com/doc/refman/5.5/en/insert-on- > >> > duplicate.html) > >> > >> >>> which combines INSERT and UPDATE in one single statement. Using > >> > >> >>> this > >> > >> I'm > >> > >> >>> able to perform a single batch satement instead of single > >> statements. > >> > >> In my > >> > >> >>> first try it saved 70 % of running time. > >> > >> >>> > >> > >> >>> My implementation is pretty simple, its just > >> > >> >>> > >> > >> >>> public synchronized String getInsertOrUpdate() { > >> > >> >>> StringBuilder buf = new StringBuilder(getInsert()); > >> > >> >>> buf.append(" ON DUPLICATE KEY UPDATE "); > >> > >> >>> long context = CTX_NAME | CTX_VALUE; > >> > >> >>> addListExpr(buf, set, context, ", "); > >> > >> >>> return buf.toString(); > >> > >> >>> } > >> > >> >>> > >> > >> >>> in DBCommandMySQL, but to add this in my DBSQLScript I have to > >> > >> >>> cast my DBCommand to DBCommandMySQL every time. > >> > >> >>> > >> > >> >>> I think we should add a method to do this in DBCommand with a > >> > >> >>> default implementation that throws a NotSupportedException. > Same > >> > >> >>> in DBDatabase (executeInserOrUpdate(...). IMO this is a good > idea > >> > >> >>> because its > >> > >> possible in > >> > >> >>> Oracle (using MERGE) and at least Postgres > >> > >> >>> (https://wiki.postgresql.org/wiki/UPSERT) - and is very > useful. > >> > >> >>> > >> > >> >>> Opinions? > >> > >> >>> > >> > >> >>> - jan > >> > >> > >> > >> > >> > >> > >> > >> > >> > > > >
