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 > > >> > > >> > > >> > > >> >
