Re: [indexeddb] Do we need to support keyPaths with an empty string?

On Tue, Jan 24, 2012 at 12:07 PM, Israel Hilerio <israelh@microsoft.com> wrote:
> On Tuesday, January 24, 2012 2:46 AM Jonas Sicking wrote:
>> On Fri, Jan 20, 2012 at 3:38 PM, Israel Hilerio <israelh@microsoft.com>
>> wrote:
>> > On Friday, January 20, 2012 2:31 PM, Jonas Sicking wrote:
>> >> On Fri, Jan 20, 2012 at 12:23 PM, ben turner <bent.mozilla@gmail.com>
>> wrote:
>> >> > Mozilla is fine with removing the special |keyPath:""| behavior.
>> >> > Please note that this will also mean that step 1 of the algorithm
>> >> > here
>> >> >
>> >> >
>> >> > http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#dfn-step
>> >> > s-f or-extracting-a-key-from-a-value-using-a-key-path
>> >> >
>> >> > will need to change.
>> >> >
>> >> > We do want to continue to allow set behavior without specifying the
>> >> > key twice, though, so we would propose adding an additional option
>> >> > to createObjectStore to accomplish this:
>> >> >
>> >> >  // Old way:
>> >> >  var set = db.createObjectStore("mySet", { keyPath:"" });
>> >> >  set.put(keyValue);
>> >> >
>> >> >  // New way:
>> >> >  var set = db.createObjectStore("mySet", { isSet: true });
>> >> >  set.put(keyValue);
>> >> >
>> >> > (We are not in love with "isSet", better names are highly
>> >> > encouraged!)
>> >> >
>> >> > What do you all think? This would allow us to continue to support
>> >> > nice set behavior without making the empty string "magic".
>> >>
>> >> I actually think that the current behavior that we have is pretty
>> >> consistent. Any time you give the keyPath property a string we create
>> >> an objectStore with a keyPath. And any time you have an objectStore
>> >> with a keyPath you are not allowed to pass an explicit key since the
>> >> key is gotten from the keyPath. There's no special handling of empty
>> strings happening.
>> >>
>> >> But I do agree that it can be somewhat confusing to tell
>> >> ""/null/undefined apart since they are all falsy. In particular, an
>> >> expression like
>> >>
>> >> if (myObjectStore.keyPath) {
>> >>   ...
>> >> }
>> >>
>> >> doesn't work to test if an objectStore has a keyPath or not. You
>> >> instead need to check
>> >>
>> >> if (myObjectStore.keyPath != null) {
>> >>   ...
>> >> }
>> >>
>> >> or
>> >>
>> >> if (typeof myObjectStore.keyPath == "string") {
>> >>   ...
>> >> }
>> >>
>> >> Hence the "isSet" suggestion.
>> >>
>> >> Though I also realized after talking to Ben that empty keyPaths show
>> >> up in indexes too. Consider creating a objectStore which maps peoples
>> >> names to email addresses. Then you can create an index when does the
>> >> opposite mapping, or which ensures that email addresses are unique:
>> >>
>> >> var store = db.createObjectStore("people"); var index =
>> >> store.createIndex("reverse", "", { unique: true });
>> >> store.add("john.doe@email.com", "John Doe");
>> >> store.add("mike@smith.org", "Mike Smith");
>> >>
>> >> store.get("John Doe").onsuccess = function(e) {
>> >>   alert("John's email is " + e.target.result); }
>> >>index.getKey("mike@smith.org").onsuccess = function(e) {
>> >>   alert("mike@smith.org is owned by " + e.target.result); }
>> >>
>> >> Are people proposing we remove empty keyPaths here too?
>> >>
>> >> / Jonas
>> >
>> > Yes, I'm proposing removing empty string KeyPaths all together to avoid
>> confusion.
>> > I would like to know how often you expect developers to follow this
>> > pattern instead of using objects.  Our believe is that objects will be
>> > the main value stored in object stores instead of single values.
>> >
>> > Supporting keyPath with empty strings brings up all kinds of side effects.
>> For example:
>> >
>> > var store = db.createObjectStore("people"); var index =
>> > store.createIndex("reverse", "", { unique: true });
>> > store.add({email: "john.doe@email.com"}, "John Doe");
>> > store.add({email: "mike@smith.org"},"Mike Smith");
>> >
>> > What should happen in this case, do we throw an exception?
>>
>> This doesn't seem any different from
>>
>> var store = db.createObjectStore("people"); var index =
>> store.createIndex("reverse", "x", { unique: true }); store.add({ x: {email:
>> "john.doe@email.com"} }, "John Doe"); store.add({ x: {email:
>> "mike@smith.org"} },"Mike Smith");
>>
>> IIRC we decided a while ago that indexes do not add constraints. I.e.
>> that if the keyPath for an index doesn't yield a valid key, then the index
>> simply doesn't get an entry pointing to newly stored value.
>>
>> So I don't really see that empty keyPaths bring up any special cases.
>> The only special case we have in Firefox for empty keyPaths (apart from the
>> keyPath evaluation code itself) is the code that throws an exception if you try
>> to create an objectStore with an empty keyPath and a key generator.
>>
>> > Having some type of flag seems more promising for object stores.
>> > However, we still need to figure out how to deal with Indexes on sets, do
>> we pass another flag to support the indexes on sets?  If we do that, then
>> what do we do with the keyPath parameter to an index.
>> > It seems we're overloading the functionality of these methods to support
>> different patterns.
>>
>> Indeed, supporting the same use cases but using something other than
>> empty key paths gets pretty messy for indexes. If we want to keep supporting
>> these use cases (which I personally do), then I think using empty key paths is
>> the cleanest solution.
>>
>> Really the only downside that I see is the somewhat non-intuitive
>> "objectStore.keyPath != null" check. But I'm not convinced that this is
>> something that people will run in to a lot given that the main use-case that I
>> can think of is generic code which visualize a indexedDB database for
>> developers.
>>
>> / Jonas
>
> After going over your explanation, uses cases, and examples, I agree with you
> that supporting an empty string keypath on indexes makes sense and is less
> confusing than any alternatives :-)
>
> So the only thing left to do is to remove the support for empty string keyPaths from the
> createObjectStore method and introduce some type of flag to specify set behavior/
> support.  As I understand it, the set pattern will constrains the put/add methods to only valid keys.
> Those keys will be used as both values and keys.  When specifying this flag, no other
> keyValue can be specified as part of the put/add method.  Is this correct?
>
> What about changing the flag name to be {useValuesAsKeys: true}? This seems to
> capture the essence of the behavior we want to prescribe.

I would prefer to keep empty-string keyPaths supported on
objectStores. I agree that it introduces some risk of confusion as
discussed earlier. But it adds a nice feature (sets) while keeping
keyPath syntax consistent between indexes and objectStores.

But I can live with something like a useValuesAsKeys property if that
is the solution that everyone else prefer. However what would
IDBObjectStore.keyPath return for such an objectStore? I.e. are we
really getting rid of the problem since objectStore.keyPath would
presumably return something false-y, but the objectStore behaves as if
it has a keyPath since you can't pass an explicit key to it.

/ Jonas

Received on Wednesday, 25 January 2012 09:48:26 UTC