IndexedDB: Indexes should be secondarily sorted on primary key
Created attachment 115491 [details] Patch
I should probably add something to the IDBLevelDBCodingTest.cpp
Created attachment 115623 [details] Patch
Comment on attachment 115623 [details] Patch Ugh, I actually need to remove some of the logic around "sequence number"; it's unnecessary for new records.
Created attachment 115652 [details] Patch
Comment on attachment 115652 [details] Patch Should be good to go now, review at leisure.
Comment on attachment 115652 [details] Patch Looks great, thanks for fixing this! View in context: https://bugs.webkit.org/attachment.cgi?id=115652&action=review > Source/WebCore/ChangeLog:7 > + Maybe say a few words in the Changelog about how this is implemented, i.e. that we're appending the primary key to index data keys. > Source/WebKit/chromium/ChangeLog:6 > + Reviewed by NOBODY (OOPS!). I would probably add a line saying that this updates the unit test. > Source/WebCore/storage/IDBLevelDBCoding.cpp:1429 > +Vector<char> IndexDataKey::encode(int64_t databaseId, int64_t objectStoreId, int64_t indexId, const Vector<char>& encodedUserKey, const Vector<char>& encodedPrimaryKey, int64_t sequenceNumber) hmm, maybe we should change the parameter order here? So far, I think they've been in the same order that we encode them. > LayoutTests/storage/indexeddb/cursor-primary-key-order.html:95 > + curreq = evalAndLog("curreq = index.openCursor()"); not super happy about "curreq" as name.. cursorRequest, or just request, maybe?
(In reply to comment #7) > (From update of attachment 115652 [details]) > > > Source/WebCore/storage/IDBLevelDBCoding.cpp:1429 > > +Vector<char> IndexDataKey::encode(int64_t databaseId, int64_t objectStoreId, int64_t indexId, const Vector<char>& encodedUserKey, const Vector<char>& encodedPrimaryKey, int64_t sequenceNumber) > > hmm, maybe we should change the parameter order here? So far, I think they've been in the same order that we encode them. Following this change, sequenceNumber is unnecessary except for testing; it has a default in the header, so I wanted it to be last. Alternately, I could add another overload that takes sequenceNumber in the "correct" order, that's only used for testing.
(In reply to comment #8) > (In reply to comment #7) > > (From update of attachment 115652 [details] [details]) > > > > > Source/WebCore/storage/IDBLevelDBCoding.cpp:1429 > > > +Vector<char> IndexDataKey::encode(int64_t databaseId, int64_t objectStoreId, int64_t indexId, const Vector<char>& encodedUserKey, const Vector<char>& encodedPrimaryKey, int64_t sequenceNumber) > > > > hmm, maybe we should change the parameter order here? So far, I think they've been in the same order that we encode them. > > Following this change, sequenceNumber is unnecessary except for testing; it has a default in the header, so I wanted it to be last. Didn't realize there was a default in the header. Never mind, then.
Created attachment 116386 [details] Patch
tony@ can you take a look?
Comment on attachment 116386 [details] Patch Clearing flags on attachment: 116386 Committed r101142: <http://trac.webkit.org/changeset/101142>
All reviewed patches have been landed. Closing bug.