RESOLVED FIXED 137154
IndexedDB is deleting data when a PK is shared amongst two objectStores 
https://bugs.webkit.org/show_bug.cgi?id=137154
Summary IndexedDB is deleting data when a PK is shared amongst two objectStores
Raymond Camden
Reported 2014-09-26 13:16:51 PDT
Imagine you have one indexeddb instance and two objectstores: people and notes If you add data to people with an assigned PK of 1, and then do the same to note, the data in people is erased. If you switch to using auto number PKs then the same thing happens (since they both end up having the same ID). Primary keys are supposed to be unique per objectStore, not the entire database. Here is a blog entry showing examples of this issue: http://www.raymondcamden.com/2014/9/25/IndexedDB-on-iOS-8--Broken-Bad#cFF30B25B-F01B-05AB-E8041ED50386A176
Attachments
Patch v1 for EWS (9.39 KB, patch)
2014-10-30 09:36 PDT, Brady Eidson 		no flags
DetailsFormatted DiffDiff
Patch v2 - New test (13.76 KB, patch)
2014-10-30 12:27 PDT, Brady Eidson 		jer.noble: review+
DetailsFormatted DiffDiff
Patch for landing. (14.01 KB, patch)
2014-10-30 13:17 PDT, Brady Eidson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2014-09-26 17:58:26 PDT
<rdar://problem/18476324>
Brady Eidson
Comment 2 2014-09-27 12:06:59 PDT
Actually <rdar://problem/18479306>
Brady Eidson
Comment 3 2014-10-30 09:32:27 PDT
The problem is the schema: "CREATE TABLE Records (objectStoreID INTEGER NOT NULL ON CONFLICT FAIL, key TEXT COLLATE IDBKEY NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT REPLACE, value NOT NULL ON CONFLICT FAIL)")); Which needs to be: "CREATE TABLE Records (objectStoreID INTEGER NOT NULL ON CONFLICT FAIL, key TEXT COLLATE IDBKEY NOT NULL ON CONFLICT FAIL, value NOT NULL ON CONFLICT FAIL)")); With the addition of an index to make objectstore/key pairs unique: "CREATE UNIQUE INDEX IF NOT EXISTS RecordsIndex ON Records (objectStoreID, key);" That in itself is straightforward. Unfortunately since the bad schema is out there, we also need a migration path. That was a little weird to get right, but I have now gotten it right. Patch forthcoming.
Brady Eidson
Comment 4 2014-10-30 09:36:40 PDT
Created attachment 240678 [details] Patch v1 for EWS I'm running tests locally to see which have been fixed, but I also want the bot's opinion (because my machine never agrees with them anyways)
Brady Eidson
Comment 5 2014-10-30 12:27:38 PDT
Created attachment 240686 [details] Patch v2 - New test
Jer Noble
Comment 6 2014-10-30 12:49:50 PDT
Comment on attachment 240686 [details] Patch v2 - New test View in context: https://bugs.webkit.org/attachment.cgi?id=240686&action=review r=me, with test nits. > LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore.html:3 > +<body> > +<div id="logger"></div> > +<script> What, no <!DOCTYPE>? > LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore.html:11 > +function log(msg) { > + document.getElementById("logger").innerHTML += msg + "<br>"; > +} I'm pretty sure we've already got mechanisms for this laying around in LayoutTest .js files somewhere. No biggie though. > LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore.html:19 > + thisDB.createObjectStore("people", {keyPath:"id"}); nit: spaces-not-tabs (here and elsewhere) > LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore.html:34 > +openRequest.onerror = function(e) { > + log("openRequest error - " + e); > +} Do you want to notifyDone() here too? Just to avoid a test timeout in the case of an unrecoverable error. > LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore.html:43 > + var transaction = db.transaction(["people"],"readwrite"); nit: space-after-comma > LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore.html:51 > + name:"Person", > + id:sharedID nit: space-after-colon > LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore.html:69 > + note:"Note", > + uid:sharedID Ditto. > LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore.html:98 > + var transaction = db.transaction(["people"],"readwrite"); nit: space-after-comma > LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore.html:107 > + log("Error getting person - ", e); notifyDone()? > LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore.html:119 > + var transaction2 = db.transaction(["notes"],"readwrite"); > + nit: space-after-comma > LayoutTests/storage/indexeddb/primary-key-unique-to-objectstore.html:134 > + for (n in e.target.result) { > + log(n); > + } Is result an array? Could you do: "e.target.result.forEach(log);"? > Source/WebKit2/DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.cpp:64 > +static const String v2RecordsTableSchema(const String& tableName) ... > Source/WebKit2/DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.cpp:74 > +static const String& v2RecordsTableSchema() These two methods having the same name (with different parameters) is a little weird. I mean, this second method is literally for the table named "Records", but the first isn't. I can't think of anything better for the first name though. v2RecordsTableLikeSchemaWithName()?
Brady Eidson
Comment 7 2014-10-30 13:17:46 PDT
Created attachment 240689 [details] Patch for landing.
WebKit Commit Bot
Comment 8 2014-10-30 14:00:55 PDT
Comment on attachment 240689 [details] Patch for landing. Clearing flags on attachment: 240689 Committed r175378: <http://trac.webkit.org/changeset/175378>
WebKit Commit Bot
Comment 9 2014-10-30 14:00:58 PDT
All reviewed patches have been landed. Closing bug.
Raymond Camden
Comment 10 2015-03-10 03:27:21 PDT
For folks who view this bug, note that it is still not released as far as I know (tested iOS 8.2).
Brady Eidson
Comment 11 2015-03-11 12:07:08 PDT
(In reply to comment #10) > For folks who view this bug, note that it is still not released as far as I > know (tested iOS 8.2). This bug tracker is about the WebKit open source project. It would be good feedback to have if the bug still reproduces in WebKit nightlies or trunk builds.
Raymond Camden
Comment 12 2015-03-11 13:10:13 PDT
I'm sorry - it was pointed out to me later. I didn't mean to clutter the issue.
Nolan Lawson
Comment 13 2015-06-05 08:23:19 PDT
I've written a live test case: http://bl.ocks.org/nolanlawson/0ee229ca3bf4b650286e I'm still seeing this issue in desktop Safari 8.0.7 as well as Safari on iOS 8.3. The reported user agent string says "AppleWebKit/600.7.5", so I'm surprised it's still happening.
Brady Eidson
Comment 14 2015-06-05 08:36:32 PDT
(In reply to comment #13) > I've written a live test case: > http://bl.ocks.org/nolanlawson/0ee229ca3bf4b650286e > > I'm still seeing this issue in desktop Safari 8.0.7 as well as Safari on iOS > 8.3. The reported user agent string says "AppleWebKit/600.7.5", so I'm > surprised it's still happening. Hi Nolan, When you're referring to Safari 8.0.7 and iOS 8.3, you're referring to products that Apple releases. Their configuration and scheduling are outside the purview of the WebKit open source project. What would be useful info would be whether or not this still reproduces for you using a WebKit nightly. Have you tried that?
Nolan Lawson
Comment 15 2015-06-05 09:29:39 PDT
Hi Brady, I just tried Webkit r185238, and my test case passes. :) Thanks a bunch! Looking forward to when this lands in an Apple release.
Note You need to log in before you can comment on or make changes to this bug.