metamodel-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rohit <rohit0...@gmail.com>
Subject Re: [DISCUSS] get back update status after invoking executeUpdate(...)
Date Mon, 26 Aug 2013 16:34:24 GMT
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 <rohit0286@gmail.com> 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 <i.am.kasper.sorensen@gmail.com> 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 <i.am.kasper.sorensen@gmail.com>:
>>> Absolutely, that would be a wonderful start! :-)
>>> 
>>> 2013/8/12 Rohit <rohit0286@gmail.com>:
>>>> 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 <i.am.kasper.sorensen@gmail.com>
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 <rohit0286@gmail.com>
>>>>> 
>>>>>> 
>>>>>> 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 <
>>>>>> i.am.kasper.sorensen@gmail.com> 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 <rohit0286@gmail.com>:
>>>>>>>> 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 <
>>>>>> i.am.kasper.sorensen@gmail.com> 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 <henry.saputra@gmail.com>:
>>>>>>>>>> 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
<
>>>>>>>>>> i.am.kasper.sorensen@gmail.com> 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 <i.am.kasper.sorensen@gmail.com>:
>>>>>>>>>>>> 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.
>>>>>>>>>>> 
>>>>>>>> 
>>>>>> 
>>>>>> 
>>>> 
> 


Mime
View raw message