RE: [IndexedDB] IDBObjectStore.delete should accept a KeyRange

On Wednesday, October 26, 2011 9:35 AM, Joshua Bell wrote:
>On Tue, Oct 25, 2011 at 4:50 PM, Israel Hilerio <israelh@microsoft.com> wrote:
>>On Monday, October 24, 2011 7:40 PM, Jonas Sicking wrote:
>>>
>>> While I was there it did occur to me that the fact that the .delete 
>>> function "returns" (through request.result in the async API) 
>>> true/false depending on if any records were removed or not might be bad for performance.
>>>
>>> I suspect that this highly depends on the implementation and that in 
>>> some implementations knowing if records were deleted will be free 
>>> and in others it will be as costly as a .count() and then a 
>>> .delete(). In yet others it could depend on if a range, rather than 
>>> a key, was used, or if the objectStore has indexes which might need updating.
>>>
>>> Ultimately I don't have a strong preference either way, though it 
>>> seems unfortunate to slow down implementations for what likely is a rare use case.
>>>
>>> Let me know what you think.
>>>
>>> / Jonas
>>>

>>To clarify, removing the return value from the sync call would change its return signature to void.  In this case, >>successfully returning from the IDBObjectStore.delete call would mean that the information was successfully >>deleted, correct?  If the information was not successfully deleted, would we throw an exception?
>>
>>In the async case, we would keep the same return value of IDBRequest for IDBObjectStore.delete.  The only change is >>that the request.result would be null, correct?  If no information is deleted or if part of the keyRange data is deleted, >>should we throw an error event?  It seems reasonable to me.

>When you write "If no information is deleted ... should we throw an error event?" do you mean (1) there was no >matching key so the delete was a no-op, or (2) there was a matching key but an internal error occurred preventing the >delete? I ask because the second clause, "if part of the keyRange data is deleted, should we throw an error event?" >doesn't make sense to me in interpretation (1) since I'd expect sparse ranges in many cases.

I was originally referring to (1) and (2).  However, after discussing this with a couple of folks we believe that the better approach would be to:
* continue to return true or false in the result.  This will take care of (1) and the successful deletion of all records.
* publish an error event if (2).  What I meant by (2) is that if there was a successful set of matches that were able to be returned by the keyRange, we should guarantee the deletion of all the matches or none.

However, (2) brings up a bigger issue.  We are basically saying that if we support deletion of keyRanges we are guaranteeing that the batch operation will all happen or none of it will happen.  This implies some type of inner-transaction associated only with the delete operation, which could also be rolledback as part of the outer-transaction.  Otherwise, you could potentially preventDefault on any record that failed to be deleted and have your database in some type of inconsistent state.  Was that the intent?

>In the async case, interpretation (1) matches Chrome's current behavior: success w/ null result if something was >deleted, error if there was nothing to delete. But I was about to land a patch to match the spec: success w/ true/false, >so this thread is timely.

Our current implementation matches the spec, so we are okay keeping it the way it is in the spec.  In either case, we need to figure out what to do when the partial deletion of keyRange records takes place.

>I agree with Jonas that returning any indication of whether data was deleted could be costly depending on >implementation. But returning success+null vs. error is just as costly as success+true vs. success+false, so I'd prefer >that if we do return an indication, we do so using the boolean approach.

>To Jonas' question: although I suspect that in most cases there will be indexes and the delete operation will internally >produce the answer anyway, since script can execute a count() then delete() we probably shouldn't penalize >delete()>and thus have it always return success+null. As I mentioned, Chrome doesn't currently match the spec in this >regard  so we don't have users dependent on the spec'd behavior.

Israel

Received on Wednesday, 26 October 2011 18:45:01 UTC