The Indexed DB spec allows key paths for object stores and indexes to be either strings or Arrays of strings. When evaluating a key path which is an Array against object to produce a key, the result is a key which is an Array of the evaluation of the strings as key paths against the object. (This is most useful when creating indexes, as you can can create a compound key.) Some restrictions apply: Array-type key paths cannot be used for object stores with key generators (i.e. autoIncrement: true).
Created attachment 138691 [details] Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment on attachment 138691 [details] Patch Whoops, this should have applied to a different bug. The ChangeLog confused webkit-patch upload.
Created attachment 139532 [details] Patch
dgrogan@, alecflett@ - can you do an initial pass on this?
Comment on attachment 139532 [details] Patch Attachment 139532 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12589380
Comment on attachment 139532 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139532&action=review > Source/WebCore/ChangeLog:8 > + String for holding/conveying key paths. Implements spec logic for evaluating Is it possible to split the string->IDBKeyPath conversion from the array-keypath logic so that this patch isn't as big?
(In reply to comment #7) > (From update of attachment 139532 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=139532&action=review > > > Source/WebCore/ChangeLog:8 > > + String for holding/conveying key paths. Implements spec logic for evaluating > > Is it possible to split the string->IDBKeyPath conversion from the array-keypath logic so that this patch isn't as big? Yeah... it's not a huge win in reducing complexity - mostly just drops the tests. (The actual logic change in this whole less than 20 lines!). Looks like the string->IDBKeyPath conversion patch would be about 100k (vs. the current 137k). Stay tuned.
Created attachment 143147 [details] Patch
Okie dokie, this is the actual functional change described in the spec. dgrogan@ - can you take a look? haraken@ - binding/v8 changes should be non-controversial. There's a FIXME in IDBObjectStore.cpp to work around http://webkit.org/b/84217 - it's definitely non-optimal, but I don't want to tackle the whole WebIDL overloading spec yet.
Comment on attachment 143147 [details] Patch r+ for the V8 change.
tony@ - r? on the overall patch?
Comment on attachment 143147 [details] Patch Clearing flags on attachment: 143147 Committed r118011: <http://trac.webkit.org/changeset/118011>
All reviewed patches have been landed. Closing bug.
Comment on attachment 143147 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143147&action=review > LayoutTests/storage/indexeddb/resources/keypath-arrays.js:34 > + evalAndExpectException("db.createObjectStore('store-with-generator', {keyPath: ['a', 'b'], autoIncrement: true})", "DOMException.INVALID_ACCESS_ERR"); We can't support this because we can't use an array as a property, i.e. we can't assign obj.[1,2] = generatedKey ? > LayoutTests/storage/indexeddb/resources/keypath-arrays.js:35 > + evalAndExpectException("store.createIndex('index-multientry', ['e', 'f'], {multiEntry: true})", "DOMException.NOT_SUPPORTED_ERR"); We can't do array keypaths and multiEntry==true because this would be meaningless? index = store.createIndex('index-multientry', ['e', 'f'], {multiEntry: true}); store.put({a: 1, b: 2, c: 3, d: 4, e: 5, f: [6, 7]});
(In reply to comment #15) > (From update of attachment 143147 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=143147&action=review > > > LayoutTests/storage/indexeddb/resources/keypath-arrays.js:34 > > + evalAndExpectException("db.createObjectStore('store-with-generator', {keyPath: ['a', 'b'], autoIncrement: true})", "DOMException.INVALID_ACCESS_ERR"); > > We can't support this because we can't use an array as a property, i.e. we can't assign obj.[1,2] = generatedKey ? Correct. This was raised as a spec issue and the spec was updated to explicitly forbid this: "If the optionalParameters parameter is specified, and autoIncrement is set to true, and the keyPath parameter is specified to the empty string, or specified to an Array, this function must throw a InvalidAccessError exception." > > LayoutTests/storage/indexeddb/resources/keypath-arrays.js:35 > > + evalAndExpectException("store.createIndex('index-multientry', ['e', 'f'], {multiEntry: true})", "DOMException.NOT_SUPPORTED_ERR"); > > We can't do array keypaths and multiEntry==true because this would be meaningless? > > index = store.createIndex('index-multientry', ['e', 'f'], {multiEntry: true}); > store.put({a: 1, b: 2, c: 3, d: 4, e: 5, f: [6, 7]}); The spec says: "If keyPath is and [sic] Array and the multiEntry property in the optionalParameters is true, then a DOMException of type NotSupportedError must be thrown." I actually find this restriction a bit odd, since it logically could work - the array-type keypath would generate an array-type key, which if multiEntry were true would generate multiple index entries.
(In reply to comment #16) > (In reply to comment #15) > > (From update of attachment 143147 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=143147&action=review > > > LayoutTests/storage/indexeddb/resources/keypath-arrays.js:35 > > > + evalAndExpectException("store.createIndex('index-multientry', ['e', 'f'], {multiEntry: true})", "DOMException.NOT_SUPPORTED_ERR"); > > > > We can't do array keypaths and multiEntry==true because this would be meaningless? > > > > index = store.createIndex('index-multientry', ['e', 'f'], {multiEntry: true}); > > store.put({a: 1, b: 2, c: 3, d: 4, e: 5, f: [6, 7]}); > > The spec says: "If keyPath is and [sic] Array and the multiEntry property in the optionalParameters is true, then a DOMException of type NotSupportedError must be thrown." > > I actually find this restriction a bit odd, since it logically could work - the array-type keypath would generate an array-type key, which if multiEntry were true would generate multiple index entries. What keys would be in the index in the example from my last comment? [5,6] and [5,7]? That could get out of control, but could also be useful, if the specified keypaths yielded long arrays.
(In reply to comment #17) > > > index = store.createIndex('index-multientry', ['e', 'f'], {multiEntry: true}); > > > store.put({a: 1, b: 2, c: 3, d: 4, e: 5, f: [6, 7]}); > > What keys would be in the index in the example from my last comment? [5,6] and [5,7]? That could get out of control, but could also be useful, if the specified keypaths yielded long arrays. Following the spec for evaluating the key path: input key path: ['e', 'f'] input value: {a: 1, b: 2, c: 3, d: 4, e: 5, f: [6, 7]} output key: [5, [6, 7]] ... which would yield index entries for: index key: 5 index key: [6, 7] So no explosion, but not necessarily all that useful.