Agreed ..for returning the list of affected rows. I think then we need not
require following methods:
public int getInsertedRows();
public int getUpdatedRows();
public int getDeletedRows();

As per below code an UpdateStatus object is communicating the status of
UpdateScript's run method, not individual insert, delete or update
operations.

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();
>   }
> });

I think we need following methods :
 1. Method to state if UpdateScript executed. However, this will not
indicate any failure or success.
 2. Method(s) to indicate is execution was successful or failure.
 3. Method to communicate custom data/info/any results from run method.
I think this is becoming like Future.


On Sun, Aug 11, 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.
> > >>>>>
> > >>
> >
> >
>

Reply via email to