Re: [Bug 11257] New: Should IDBCursor.update be able to create a new entry?

On Fri, Nov 12, 2010 at 9:26 AM, Jonas Sicking <jonas@sicking.cc> wrote:

> On Thu, Nov 11, 2010 at 9:12 PM, Jeremy Orlow <jorlow@chromium.org> wrote:
> > On Fri, Nov 12, 2010 at 12:11 AM, Jonas Sicking <jonas@sicking.cc>
> wrote:
> >>
> >> On Thu, Nov 11, 2010 at 11:51 AM, Jeremy Orlow <jorlow@chromium.org>
> >> wrote:
> >> > On Thu, Nov 11, 2010 at 8:46 PM, Jonas Sicking <jonas@sicking.cc>
> wrote:
> >> >>
> >> >> On Thu, Nov 11, 2010 at 5:11 AM, Jeremy Orlow <jorlow@chromium.org>
> >> >> wrote:
> >> >> > On Mon, Nov 8, 2010 at 2:12 PM, <bugzilla@jessica.w3.org> wrote:
> >> >> >>
> >> >> >> http://www.w3.org/Bugs/Public/show_bug.cgi?id=11257
> >> >> >>
> >> >> >>           Summary: Should IDBCursor.update be able to create a new
> >> >> >> entry?
> >> >> >>           Product: WebAppsWG
> >> >> >>           Version: unspecified
> >> >> >>          Platform: PC
> >> >> >>        OS/Version: All
> >> >> >>            Status: NEW
> >> >> >>          Severity: normal
> >> >> >>          Priority: P2
> >> >> >>         Component: Indexed Database API
> >> >> >>        AssignedTo: dave.null@w3.org
> >> >> >>        ReportedBy: jonas@sicking.cc
> >> >> >>         QAContact: member-webapi-cvs@w3.org
> >> >> >>                CC: mike@w3.org, public-webapps@w3.org
> >> >> >>
> >> >> >>
> >> >> >> What should happen in the following case:
> >> >> >>
> >> >> >> db.transaction(["foo"]).objectStore("foo").openCursor().onsuccess
> =
> >> >> >> function(e)
> >> >> >> {
> >> >> >>  var cursor = e.result;
> >> >> >>  if (!cursor)
> >> >> >>    return;
> >> >> >>
> >> >> >>  cursor.delete();
> >> >> >>  cursor.update({ id: 1234, value: "Benny" });
> >> >> >> }
> >> >> >>
> >> >> >>
> >> >> >> This situation can of course arrive in more subtle ways:
> >> >> >>
> >> >> >> os = db.transaction(["foo"]).objectStore("foo");
> >> >> >> os.openCursor().onsuccess = function(e) {
> >> >> >>  var cursor = e.result;
> >> >> >>  if (!cursor)
> >> >> >>    return;
> >> >> >>
> >> >> >>  cursor.update({ id: 1234, value: "Benny" });
> >> >> >> }
> >> >> >> os.delete(1234);
> >> >> >>
> >> >> >>
> >> >> >> As specified, IDBCursor.update behaves just like
> IDBObjectStore.put
> >> >> >> and
> >> >> >> just
> >> >> >> creates a new entry, but this might be somewhat unexpected
> behavior.
> >> >> >
> >> >> > Let's just remove update and delete from IDBCursor and be done with
> >> >> > it.
> >> >>
> >> >> The problem is that you can't always get to the key of the
> objectStore
> >> >> entry to delete/update. Specifically if the objectStore uses
> >> >> out-of-line keys the cursor doesn't expose those.
> >> >
> >> > Why not fix this use case then?  I.e. change the cursor to return
> >> > .indexKey,
> >> > .primaryKey, .value (or something like that).  If we did this, we
> could
> >> > even
> >> > get rid of the different between object cursors and key cursors (which
> >> > overload the .value to mean the primary key, which is quite
> confusing).
> >>
> >> I would be ok with exposing some new property which exposes the
> >> objectstore key.
> >
> > And thus get rid of the openKeyCursor and getKey?  This would make the
> spec
> > a bit more complicated (we'd need to have 2 IDBCursor objects, one
> > that inherits from the other), but seems much simpler to use.  Shall I
> file
> > a bug?
>
> I think openKeyCursor is still needed. There are performance
> advantages since you save a btree lookup for each iterated entry, and
> you also don't need to spend time reading and possibly deserializing a
> potentially large object if all that's needed, for example when doing
> a join, is the key.
>
> All I was suggesting is that we have
>
> interface IDBCursor {
>  ...
>  readonly attribute any objectStoreKey;
>  readonly attribute any key;
>  readonly attribute any value;
>  ...
> }
>
> When using a cursor from IDBIndex.openCursor() all three properties
> are set. When using a cursor from IDBIndex.openKeyCursor() only
> objectStoreKey and key are set. When using a cursor from
> IDBObjectStore.openCursor() key and value are set and either we make
> objectStoreKey "undefined" or return the same thing as key.
>
> Or, we make IDBObjectStore.openCursor() return a cursor which doesn't
> have a objectStoreKey property.
>

Either way is fine with me.  I think the latter is probably slightly better
and worth the extra implementation hassle.


> Or we leave things the way they are now.
>
> I'm fine with either of these solutions.
>
> I'm not sure how getKey is affected by all this?


Oops.  It isn't.


>  >> But I still think that .update and .delete are useful and logical API
> >> which has very little implementation cost.
> >
> > I still don't understand why you think low implementation is important at
> > all when talking about these APIs.  If something is so insanely complex
> that
> > implementors would likely not implement it, then I can understand
> bringing
> > it up, but otherwise I think most people on this list can agree that it
> > should cary _very_ little weight when deciding whether API surface area
> is
> > worth it.
>
> For things that are mere convenience APIs I do think that cost of
> implementation matters more. Though the type of cost I'm worried about
> isn't so much the amount of code that has to be written by
> implementations now, but rather how much risk it introduces in terms
> of future constraints on the spec and future costs for how
> implementations could be written in the future.
>

Backing ourselves into a corner that limits options in the future
is definitely one of my biggest concerns when we're adding these APIs.  I
still don't see why cost to the implementation is terribly important in all
of this though.

But I think you're probably right that the risk we're adding with .delete()
and .update() is minimal. Especially since all of its behavior is stuff we
need to spec for .put anyway.

J