Re: [indexeddb] Issues stated on the current spec

Awesome list!

> 1. Sections: 3.1.2 Object Store
> Issue Text: specify that generators are not shared between stores.
> Feedback: We prefer this approach.  We should state this in the spec and remove the issue.

Agreed. This was the intent, but it might not be spelled out.
Generators are not shared, and always produce the lowest integer
larger than the highest key that has ever been inserted in the object
store.

This ensures that generators never generate a number that has existed
in the object store.

One tricky problem is what to do if someone inserts a non-numeric key
in the object store. Such a key could be higher than any number that
the generator could generate.

I can think of three approaches:

1. Treat this the same way as inserting the number 2^64-1 into the
object store. I.e. make it impossible to generate more numbers. The
object store will have to be removed and recreated without recreating
the high-valued key.
2. Throw a exception if a record is inserted with a non-numeric key in
a object store which uses generators
3. Make generators generate numbers higher than the highest *numeric*
value ever inserted.

I'm leaning towards 3. The best implementation strategy for generators
seems to be to record any insertions with explicit keys and make sure
to update the generator current value to a higher number. To implement
3 all you'd need to do is to only do this for numeric keys.

> 2. Section: 3.1.11 The IDBDatabaseException Interface
> Issue Text: These codes are in flux and may change entirely as exception/error handling may be changing in the WebIDL spec.
> Feedback: It seems we can remove this comment and still go to last call.  We can always change these codes later.

Actually, what we should do is use the new exception strategy that
WebIDL uses. Which is that exceptions are differentiated using a
.name, which is a string, rather than a numeric .code.

> 3. Section: 3.2.3 Opening a database [IDBFactory.cmp()]
> Issue Text: This should probably take a collation parameter as well. In fact, it might make sense for this to be on the IDBDatabase, IDBObjectStore, and IDBIndex as well and do the comparison with their default collation.
> Feedback: Since we're not introducing collations in our APIs for v1, I believe we can remove this comment.

Agreed.

> 4. Section: 3.2.7 Cursor [IDBCursor.delete()]
> Issue Text: This method used to set this cursor's value to null. Do we want to keep that?
> Feedback: We believe that for the asynchronous APIs, the cursor is a cached value and therefore it can still be accessed after we call delete.  The reason is that delete is an asynchronous operation that should only affect the server value of the current record.  The impact should only be felt the next time you try to access this record from the server in any operation. We should be able to specify this on the spec and remove the issue.

Agreed.

> 5. Section: 3.3.3 Object Store [In example]
> Issue Text: The scenario above doesn't actually work well because you have to unwind to the event loop to get the version transaction to commit before you can do something else. Leaving it like this for now, as this will get sorted out when we add callbacks for transactions in the sync API (we'll have to do it for the setVersion transaction as well).
> Feedback: I believe this will be simplified with the new version of open which includes the db version.  At that point, I expect this issue to go away.

Agreed.

> 6. Section: 3.3.4 Index [in example]
> Issue Text: The scenario above doesn't actually work well because you have to unwind to the event loop to get the version transaction to commit before you can do something else. Leaving it like this for now, as this will get sorted out when we add callbacks for transactions in the sync API (we'll have to do it for the setVersion transaction as well).
> Feedback: I believe this will be simplified with the new version of open which includes the db version.  At that point, I expect this issue to go away.

Agreed.

> 7. Section: 3.3.5 Cursor [in IDBCursorSync.delete()]
> Issue Text: This method used to set this cursor's value to null. Do we want to keep that?
> Feedback: We believe that for synchronous APIs we need to ensure that the client state reflects the server state.  Since this call is synchronous, the value associated with the cursor should be set to null after the delete API is called. We should be able to specify this in the spec and remove the issue.

I don't think that even in the synchronous API we want cursor.value to
always reflect the value in the object store. For example if the
object store is modified directly, rather than through a cursor, this
wouldn't affect the value of the cursor.

If we *do* want to ensure that cursor.value always reflected the value
in the cursor, we'd have to run a database query every time
cursor.value was accessed, which in the common case (i.e. when the
object store was not modified) would be a lot of extra overhead for no
discernible difference.

So I think we should make both the synchronous and asynchronous API
not touch cursor.value when cursor.update or cursor.delete is called.

> 8. Section: 4.2 Transaction Creation steps
> Issue Text: This should be specified more precisely. Maybe with some sort of global variable locked
> Feedback: Is this not addressed in the current spec?

I agree that it is addressed.

> 9. Section: 4.8 VERSION_CHANGE transaction steps
> Issue Text: If .close() is called immediately but a transaction associated with the connection keeps running for a "long time", should we also fire a blocked event?
> Feedback: This seems like an optimization that individual User Agents can choose to make without affecting compatibility.  Because of the asynchronous nature of the APIs, this behavior seems to be unavoidable.

I mostly agree. The problem is that the current language in the spec
prohibits the UA from firing the "blocked" event if all databases call
.close() from the "versionchange" event handler. So even if a database
can't be closed for a long time due to a long-running transaction, the
UA is with the current spec language prohibited from firing the
"blocked" event.

I think we should change this somehow, but I'm not entirely clear on how.

> 10. Section: 4.10 Database deletion steps
> Issue Text: Should we allow blocked to be fired here too, if waiting takes "too long"?
> Feedback: We don't see the value of calling onblock twice because we don't provide a mechanism to cancel the db deletion transaction.  All the onblock provides web developers is a mechanism for them to notify their users that the db is pending deletion.  This doesn't seem to require more than one onblock.

The "here too" part here was intended to mean "just like in the
VERSION_CHANGE transaction steps". I.e. not to ever fire the event
twice.

The issue here is exactly the same as in the "VERSION_CHANGE
transaction steps". I.e. the language in the spec currently forbids
the "blocked" event from being fired if all handlers of the
"versionchange" event call .close(), even if they they keep a long
running transaction.

> 11. Section: 4.12 Fire an error event
> Issue Text: TODO: need to define more error handling here.
> Feedback: Not sure what else we need.

I think this is mostly an old comment.

However, there is one thing that the Firefox implementation does, that
has worked very well and that I think should be standardized. If a
"error" event is fired but not canceled, we fire a separate "error"
event on the window object as an error reporting mechanism.

This way, if there's a bug in the code which causes a invalid database
operation, or if an internal error happens, for example a file IO
error, and that error goes unhandled, then the window.onerror handler
gets triggered.

This is very similar to how if an exception goes unhandled in normal
JS execution, the window.onerror handler gets triggered. Since the
asynchronous API can't use exceptions, but instead has to use "error"
events we wouldn't normally be able to take advantage of javascripts
built-in error reporting mechanism. By making unhandled "error" events
trigger window.onerror we bridge this gap.

Another way to look at it is that if an error isn't handled when the
synchronous API is used, then window.onerror is triggered since an
exception is throw. By making a unhandled "error" event also trigger
window.onerror we align the async and the sync API better.

> 12. Section: 5.7 Cursor Iteration Operation
> Issue Text: This should only be done right before firing the success event. Not asynchronously before. Not sure how/where to express that.
> Feedback: This seems more like a note rather than an issue.  I believe we can just capture what you stated and that should be enough.

What we should do here is to say that the same "task" both sets the
new values and fires the success event. I.e. something like "Once data
has been successfully read, schedule a task which when run sets the
cursors value and fires a "success" event".

/ Jonas

Received on Saturday, 27 August 2011 02:37:38 UTC