Bug 145993 - IDB: Records table migration doesn't work with all versions of SQLite
Summary: IDB: Records table migration doesn't work with all versions of SQLite
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-06-15 16:45 PDT by Brady Eidson
Modified: 2022-02-28 04:09 PST (History)
4 users (show)

See Also:


Attachments
Patch v1 (3.99 KB, patch)
2015-06-15 16:52 PDT, Brady Eidson
darin: review+
Details | Formatted Diff | Diff
Patch for landing (4.55 KB, patch)
2015-06-16 10:45 PDT, Brady Eidson
beidson: commit-queue-
Details | Formatted Diff | Diff
Patch for landing v2 (4.55 KB, patch)
2015-06-16 10:51 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2015-06-15 16:45:38 PDT
IDB: Records table migration doesn't work with all versions of SQLite

Getting the current Records table schema from the sqlite_master table does not necessarily return the exact SQL used to create the table.

Some SQLites will return a quoted table name in the returned SQL, and others will have the table name be unquoted.

In the createOrMigrateRecordsTableIfNecessary() function we should check for both the quoted and unquoted schemas.
Comment 1 Brady Eidson 2015-06-15 16:52:06 PDT
Created attachment 254907 [details]
Patch v1
Comment 2 Jon Lee 2015-06-15 17:19:02 PDT
Comment on attachment 254907 [details]
Patch v1

Provisional r=me
Comment 3 Jon Lee 2015-06-15 17:20:51 PDT
CC'ing some folks who might be able to give a real r+.
Comment 4 Darin Adler 2015-06-16 09:39:40 PDT
Comment on attachment 254907 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=254907&action=review

> Source/WebKit2/DatabaseProcess/IndexedDB/sqlite/UniqueIDBDatabaseBackingStoreSQLite.cpp:63
> +    StringBuilder builder;
> +    builder.appendLiteral("CREATE TABLE ");
> +    builder.append(tableName);
> +    builder.appendLiteral(" (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)");
> +    return builder.toString();

This is something that would be more efficient with just the "+" operator or makeString rather than a StringBuilder.
Comment 5 Brady Eidson 2015-06-16 10:45:38 PDT
Created attachment 254957 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2015-06-16 10:49:03 PDT
Comment on attachment 254957 [details]
Patch for landing

Rejecting attachment 254957 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 254957, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

/Volumes/Data/EWS/WebKit/Source/WebKit2/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.appspot.com/results/6073680211738624
Comment 7 Brady Eidson 2015-06-16 10:50:43 PDT
(In reply to comment #6)
> Comment on attachment 254957 [details]
> Patch for landing
> 
> Rejecting attachment 254957 [details] from commit-queue.
> 
> Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch',
> '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02',
> 'validate-changelog', '--check-oops', '--non-interactive', 254957,
> '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
> 
> /Volumes/Data/EWS/WebKit/Source/WebKit2/ChangeLog neither lists a valid
> reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case
> insensitive).
> 

Wrong. Thanks, tools.
Comment 8 Brady Eidson 2015-06-16 10:51:52 PDT
Created attachment 254958 [details]
Patch for landing v2
Comment 9 Brady Eidson 2015-06-16 11:27:41 PDT
rdar://problem/17357002
Comment 10 WebKit Commit Bot 2015-06-16 11:42:01 PDT
Comment on attachment 254958 [details]
Patch for landing v2

Clearing flags on attachment: 254958

Committed r185599: <http://trac.webkit.org/changeset/185599>