Re: DataStore.deleteByQuery()

[Scott Stults <ss...@opensourceconnections.com>]

I do like the idea of making this a property of the DataStore. For one thing, when you're using multiple types of DataStores in a single app it's sort of a reminder to the developer of what the semantics are. For another, it's a great reminder to implementers of new DataStores that their own semantics aren't necessarily shared with others. I've banged my knee on this a couple of times :)


-Scott

On Aug 31, 2013, at 2:34 PM, Renato Marroquín Mogrovejo <re...@gmail.com> wrote:

> Sorry to take so freaking long to answer this.
> I was thinking on Henry's solution i.e. hard code the default parameter in
> an "abstract implementation" of the specific tests, and say that if someone
> wants to modify these tests, then it should do it without updating the hard
> coded value. This is only for testing purposes of course. This being said,
> we could even create two tests, one for inclusive and another one for
> exclusive keys.
> 
> 
> Renato M.
> 
> 
> 2013/8/31 Henry Saputra <he...@gmail.com>
> 
>> We could just hard coded the default in the DataStore implementation. This
>> should not be changed via properties value I believe.
>> 
>> - Henry
>> 
>> 
>> On Fri, Aug 23, 2013 at 10:41 AM, Apostolis Giannakidis <
>> ap.giannakidis@gmail.com> wrote:
>> 
>>> Sure,
>>> but based on which value will we set the parameter? It should be a
>>> datastore specific value; i.e. each data store should set its own value
>> for
>>> this parameter. This is why I proposed to put it in the gora.properties
>>> file, as this will be configurable for each data store. This will serve
>> as
>>> the default case of the range.
>>> Additionally, we can provide extended API methods (as Renato suggested)
>>> that accept an extra argument that will override the default case. What
>> do
>>> you think?
>>> 
>>> Apos
>>> 
>>> 
>>> On Fri, Aug 23, 2013 at 6:33 PM, Renato Marroquín Mogrovejo <
>>> renatoj.marroquin@gmail.com> wrote:
>>> 
>>>> We'd have to set the new parameter before we start that specific test.
>>> Does
>>>> it make sense?
>>>> 
>>>> 
>>>> Renato M.
>>>> 
>>>> 
>>>> 2013/8/22 Apostolis Giannakidis <ap...@gmail.com>
>>>> 
>>>>> Oracle can have both inclusive and exclusive ranges.
>>>>> However, I still have not understood how the test cast will work, if
>> we
>>>>> follow Renato's suggestion to add another parameter to the API
>> method.
>>>>> 
>>>>> Regards,
>>>>> Apos
>>>>> 
>>>>> 
>>>>> On Fri, Aug 23, 2013 at 2:16 AM, Henry Saputra <
>>> henry.saputra@gmail.com
>>>>>> wrote:
>>>>> 
>>>>>> +1
>>>>>> 
>>>>>> Looks like HBase is the one exception? Oracle and Accumulo seem to
>> be
>>>>>> inclusive, and I believe Cassandra also inclusive.
>>>>>> 
>>>>>> In the DataStoreTestUtil, we could probably check the type of data
>>>> store
>>>>>> before executing the delete to pass the right flag.
>>>>>> 
>>>>>> - Henry
>>>>>> 
>>>>>> 
>>>>>> On Thu, Aug 22, 2013 at 3:54 PM, Renato Marroquín Mogrovejo <
>>>>>> renatoj.marroquin@gmail.com> wrote:
>>>>>> 
>>>>>>> @Henry, you are right mate (:
>>>>>>> @Apos, you are right as well mate (: but IMHO we could set this
>>> value
>>>>>> with
>>>>>>> a default value (the one that most data stores use) and run the
>>> tests
>>>>>> like
>>>>>>> this i.e. modifying the tests to make this run well, maybe we
>> could
>>>>> even
>>>>>>> set this new parameter when starting the test so it runs
>> smoothly.
>>>>>>> 
>>>>>>> 
>>>>>>> Renato M.
>>>>>>> 
>>>>>>> 
>>>>>>> 2013/8/22 Apostolis Giannakidis <ap...@gmail.com>
>>>>>>> 
>>>>>>>> Hi Henry,
>>>>>>>> 
>>>>>>>> As far as I have understood Renato's proposal, that's correct.
>>>>>>>> But, now that I think of it, if we follow Renato's suggestion,
>>> then
>>>>> how
>>>>>>>> will the test case[1] know if it should include the key or not
>> in
>>>> its
>>>>>>>> checks?
>>>>>>>> 
>>>>>>>> [1]
>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> https://github.com/apache/gora/blob/trunk/gora-core/src/test/java/org/apache/gora/store/DataStoreTestUtil.java#L747
>>>>>>>> 
>>>>>>>> 
>>>>>>>> On Thu, Aug 22, 2013 at 8:08 PM, Henry Saputra <
>>>>>> henry.saputra@gmail.com
>>>>>>>>> wrote:
>>>>>>>> 
>>>>>>>>> HI Renato,
>>>>>>>>> 
>>>>>>>>> If the API change include a new parameter to indicate
>> inclusive
>>>> vs
>>>>>>>>> exclusive then Gora do not have to decide anything and just
>>>>> delegate
>>>>>>> the
>>>>>>>>> new parameter to the corresponding datastore?
>>>>>>>>> 
>>>>>>>>> - Henry
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> On Thu, Aug 22, 2013 at 10:43 AM, Renato Marroquín Mogrovejo
>> <
>>>>>>>>> renatoj.marroquin@gmail.com> wrote:
>>>>>>>>> 
>>>>>>>>>> Hi all,
>>>>>>>>>> 
>>>>>>>>>> I think we could just add an extra parameter to the query
>>> API,
>>>> so
>>>>>>> users
>>>>>>>>> can
>>>>>>>>>> decide programmatically whether they want to use the
>> deletes
>>> as
>>>>>>>> inclusive
>>>>>>>>>> or exclusive, and they could do this while programming with
>>>>> Gora's
>>>>>>> API.
>>>>>>>>> And
>>>>>>>>>> we could decide to use a default value for the option that
>>> most
>>>>>> data
>>>>>>>>> stores
>>>>>>>>>> support. What do you think?
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> Renato M.
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> 2013/8/18 Apostolis Giannakidis <ap...@gmail.com>
>>>>>>>>>> 
>>>>>>>>>>> Yes, I can also do both inclusive and exclusive ranges in
>>>>> Oracle
>>>>>>>> NoSQL.
>>>>>>>>>> So
>>>>>>>>>>> it remains to be decided by the Gora API.
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> On Sun, Aug 18, 2013 at 4:06 AM, Scott Stults <
>>>>>>>>>>> sstults@opensourceconnections.com> wrote:
>>>>>>>>>>> 
>>>>>>>>>>>> Thanks for the reply, Apos. Seeing as how this test is
>> in
>>>>> flux
>>>>>> I
>>>>>>>>> won't
>>>>>>>>>>>> worry too much about it now. FWIW, I could do inclusive
>>> or
>>>>>>>> exclusive
>>>>>>>>>>> ranges
>>>>>>>>>>>> with Lucene.
>>>>>>>>>>>> 
>>>>>>>>>>>> -Scott
>>>>>>>>>>>> 
>>>>>>>>>>>> On Aug 17, 2013, at 9:52 PM, Apostolis Giannakidis <
>>>>>>>>>>>> ap.giannakidis@gmail.com> wrote:
>>>>>>>>>>>> 
>>>>>>>>>>>>> Hello Scott,
>>>>>>>>>>>>> 
>>>>>>>>>>>>> The issue that you just spotted is the same issue
>> that
>>> I
>>>>> also
>>>>>>>>>>>>> coincidentally spotted a week ago.
>>>>>>>>>>>>> Keith Turner first identified the issue and
>> documented
>>> it
>>>>> in
>>>>>>>> Jira.
>>>>>>>>>>> Please
>>>>>>>>>>>>> see GORA-66.
>>>>>>>>>>>>> https://issues.apache.org/jira/browse/GORA-66
>>>>>>>>>>>>> 
>>>>>>>>>>>>> This is also a blocking issue for me, as it does not
>>>> allow
>>>>> me
>>>>>>> to
>>>>>>>>>>> complete
>>>>>>>>>>>>> the implementation of deleteByQuery(). Personally, I
>>>>> @Ignored
>>>>>>>> this
>>>>>>>>>> test
>>>>>>>>>>>>> case until GORA-66 is resolved. I saw that the same
>> was
>>>>> done
>>>>>> in
>>>>>>>>>>> Accumulo
>>>>>>>>>>>>> datastore.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> I hope this helps,
>>>>>>>>>>>>> Apos
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>> On Sat, Aug 17, 2013 at 8:11 PM, Scott Stults <
>>>>>>>>>>>>> sstults@opensourceconnections.com> wrote:
>>>>>>>>>>>>> 
>>>>>>>>>>>>>> All,
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> I'm having a little trouble getting my head around
>>>>>>>>> deleteByQuery().
>>>>>>>>>>> The
>>>>>>>>>>>>>> javadoc in the interface indicates that any object
>>> that
>>>>>>> matches
>>>>>>>>> the
>>>>>>>>>>>> query
>>>>>>>>>>>>>> should get deleted. The unit test
>>>>>>>>>>>>>> DataStoreTestUtil.testDeleteByQueryFields() expects
>>> the
>>>>>> object
>>>>>>>> to
>>>>>>>>>>> still
>>>>>>>>>>>>>> exist with the queried-for fields cleared. To me it
>>>> seems
>>>>>> like
>>>>>>>> the
>>>>>>>>>>> test
>>>>>>>>>>>> is
>>>>>>>>>>>>>> for an update, rather than a delete.
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Are my semantics all mixed up?
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> -Scott
>>>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>>