Rohit has indicated to me that he does not have time to finish his work on this right now. So if anyone else is looking for an interesting API-change proposal to work on, here is one. Or else we'll just put it back into the "pending" pool I guess.
2013/8/27 Kasper Sørensen <[email protected]> > Hi Rohit, > > Quick feedback based on the summary (will dive into the patch code later). > > I don't think we should change the interface of the UpdateScript > interface. This will break compile compatibility of any consuming > application. And it also puts the responsibility to gather the right > numbers on the user of the framework, not the framework itself. Rather > the execute() methods on the statements should update an internal data > structure that keeps track of the number of inserted/updated/deleted > records. The ability to maintain such a data structure is provided in > that the update callback is provided by the DataContext. The update > callback can in other words have much richer functionality than what > is exposed through it's interface. Hope it makes sense? > > Kasper > > 2013/8/26 Rohit <[email protected]>: > > Hi Kasper, > > > > I've worked on code to provide UpdateStatus. > > Just to be sure that I'm on right track I'm attaching a patch of > > intermediate code changes. However they cover up following modules > > completely: > > couchdb, csv and core. > > > > Please let me know your thoughts on this implementation ? > > If this is ok then I'll continue with the same approach to remaining > > packages. > > > > Summary of changes: > > > > Edited UpdateableDataContext's executeUpdate to return UpdateStatus. Also > > changed UpdateScript interface run method to return UpdateStatus because > > executeUpdate calls run method internally. > > Changed Row Insert/Update/Delete Builder "execute" method to return > number > > to affected rows. This info is then saved in UpdateStatus inside run > method. > > > > Thanks, > > Rohit > > > > > > > > On 21-Aug-2013, at 11:06 PM, Rohit <[email protected]> wrote: > > > > Hey Kasper, > > > > I'm out of t. Will be working on this over this weekend. > > > > Thanks, > > Rohit. > > > > On 21-Aug-2013, at 6:17 PM, Kasper Sørensen < > [email protected]> > > wrote: > > > > Hi Rohit, > > > > It's been a little while now and I just wanted to check if you're > > still on this task? IMO we gotta make some progress soon if we want to > > include it in a release. Let it be heard if you're facing issues. > > > > Regards, > > Kasper > > > > 2013/8/12 Kasper Sørensen <[email protected]>: > > > > Absolutely, that would be a wonderful start! :-) > > > > 2013/8/12 Rohit <[email protected]>: > > > > Agreed. > > > > In order to start contributing to the Metamodel. > > As a first step can I provide you a patch of this, > > then you can guide further ? > > > > Thanks, > > Rohit > > On 11-Aug-2013, at 3:30 PM, Kasper Sørensen < > [email protected]> > > wrote: > > > > The main development branch is 'master'. The .executeUpdate(...) method > is > > in the subinterface UpdateableDataContext. > > > > My point is that you can provide an update script with many operations in > > one go. Take this example: > > > > UpdateStatus status = dataContext.executeUpdate(new UpdateScript() { > > > > public void run(UpdateCallback cb) { > > cb.insertInto(table).value(...).execute(); > > cb.insertInto(table).value(...).execute(); > > cb.update(table).where(...).value(...).execute(); > > cb.deleteFrom(table).where(...).execute(); > > } > > }); > > > > > > > > In this example we know that 2 records have been inserted. Some amount of > > records have been updated (that may or may not be reported by the backing > > datastore) and some amount of records have been deleted (that may or may > > not have been reported by the backing datastore). > > > > Getting a list of affected records would be very complicated IMO, and > would > > only be consistent if done at the time of the transaction. That means > that > > if we even figure out how to do it, we also have to do it eagerly, which > is > > obviously not a good thing to do since I think getting that list is a > > feature that would not apply to most use cases. > > > > The idea about doing this in a multithreaded/nonblocking fashion is > > interesting. It would definately have to be an option in my opinion, > since > > I think the interface right now indicates that the executeUpdate method > > will return succesfully when the update is done (ie. it throws those > > exceptions that may arise during the update, and an unblocking variant > > would not be able to do that). > > > > > > 2013/8/11 Rohit <[email protected]> > > > > > > My point is why do we need 3 methods to provide the status ? We can use > > one method to provide the status (Returns a boolean if update operation a > > done or not) and other to provide a list of affected rows/objects keys, > > this would be less memory intensive. > > And in case of deletion we can throw a custom exception stating " since > > this is a delete operation we cannot provide affected rows". > > > > public interface UpdateStatus { > > public <List or Collection> getAffectedRowsKeys(); or use > > > > getRowsKeys(); or getKeys() throws MetaModelException; > > public Boolean isDone(); > > > > } > > > > > > > > I see following remote branches in git repo: > > remotes/origin/HEAD -> origin/master > > remotes/origin/hbase-module > > remotes/origin/master > > > > Which is the current development branch as I couldn't find executeUpdate > > in DataContext ? > > > > On 11-Aug-2013, at 1:04 PM, Kasper Sørensen < > > [email protected]> wrote: > > > > I agree on the reasoning here, but getting a list of affected rows may > > prove to be both quite difficult (from consistency point of view it > > needs to be created immediately after some update - how can that be > > implemented in an effecient manner (lazy, since you wouldn't always > > want to get this list, but then consistency is gone), and how to > > guarantee that those rows are even available (e.g. after a DELETE > > statement in a JDBC database, you cannot get the rows anymore)), and > > too memory intensive (updates can be anything from a single record to > > millions of records). > > > > That's why I think the three methods (maybe with an additional fourth > > method, returning the total num. of affected rows) is the best initial > > set of methods. > > > > 2013/8/11 Rohit <[email protected]>: > > > > Having an abstraction for communicating status would be a cleaner > > > > approach. As it allows us to extend the functionality if required in > future. > > > > For Example: > > Lets say we are executing executeUpdate in multithreaded env. then we > > > > can provide methods to check the status of the update. (running or done). > > > > > > Also rather than having methods returning int I think we should return > > > > a collection of Affected Rows. > > > > And rather than having multiple methods for insert/delete/update just > > > > have a single method returning the affected rows. > > > > > > I think making so will make the API simpler yet functional. > > > > public interface UpdateStatus { > > public <List or Collection> getAffectedRows(); or just use getRows() > > } > > > > > > > > -RS > > > > On 10-Aug-2013, at 10:09 PM, Kasper Sørensen < > > > > [email protected]> wrote: > > > > > > My experience with returning something like an > > int/String/other-simple-type is that it seems to fit the purpose at > > first, but then someone asks "couldn't we also get the status of > > [something], too?" ... and then you regret not encapsulating the > > return value so that you could just add another getter on it. > > > > 2013/8/10 Henry Saputra <[email protected]>: > > > > We could just make the executeUpdate just return number of elements > > affected similar to relational databases. > > > > It would be simpler and serve similar purpose. > > > > - Henry > > > > > > On Fri, Aug 9, 2013 at 12:15 AM, Kasper Sørensen < > > [email protected]> wrote: > > > > Allow me to also bump this issue. > > > > Since this would be a change that would not be compile-time > > incompatible with previous versions, I suggest we introduce at least > > the return type of the method now. Since everyone moving the Apache > > MetaModel from the old metamodel would anyways have to recompile, > > > > they > > > > would not feel the runtime incompatibility of void vs. > > > > SomeReturnType. > > > > > > By now I suggest we just keep a very simple returned interface which > > we can always choose to expand (implemented internally, not by > > consumers). For instance something like: > > > > public interface UpdateStatus { > > public int getInsertedRows(); > > public int getUpdatedRows(); > > public int getDeletedRows(); > > } > > > > This should be quite easy to implement once and reuse for most of the > > modules. For some datastores it might not be information that we can > > retrieve, so the interface should also specify an extraordinary rule; > > e.g. "if -1 is returned it means that it is not known how many rows > > where inserted/updated/deleted" > > > > Kasper > > > > 2013/7/24 Kasper Sørensen <[email protected]>: > > > > In the current API design of MetaModel, the > > > > DataContext.executeUpdate(...) > > > > method is a void method. This was initially chosen because not all > > implementations have the capability to report anything about a > > > > particular > > > > update. But some do, for instance the no. of inserted, updated or > > > > deleted > > > > records from a JDBC call. It would be nice to expose this > > > > information > > > > when > > > > available. > > > > My suggestion for this would be to let the > > > > DataContext.executeUpdate(...) > > > > method return an object with this information. All methods on the > > > > new > > > > object > > > > type would be optionally returning null, if no information is > > > > available. > > > > But > > > > when available, we can at least expose it this way. > > > > The change wouldn't have a major impact, since any project using > > > > MetaModel > > > > would already need to recompile because of the namespace change to > > org.apache.metamodel. And the change would be compile-time > > > > compatible > > > > with > > > > having a void return type. > > > > > > > > > > > > > > > > > > >
