Bug 52576 - IndexedDB: Support auto-increment keys
Summary: IndexedDB: Support auto-increment keys
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Hans Wennborg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-17 07:28 PST by Hans Wennborg
Modified: 2011-01-19 08:24 PST (History)
3 users (show)

See Also:


Attachments
Patch (19.02 KB, patch)
2011-01-17 07:50 PST, Hans Wennborg
no flags Details | Formatted Diff | Diff
Patch (22.13 KB, patch)
2011-01-19 07:27 PST, Hans Wennborg
jorlow: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hans Wennborg 2011-01-17 07:28:21 PST
IndexedDB: Support auto-increment keys
Comment 1 Hans Wennborg 2011-01-17 07:50:13 PST
Created attachment 79175 [details]
Patch
Comment 2 Jeremy Orlow 2011-01-17 08:09:39 PST
Let's cache the last autonumber.  It's even OK if we leak some numbers as long as it always increases.

Also, I believe that the expected behavior is that if you have a keypath and autoIncrement value, the latter should get inserted into the former...even if it already exists.  I don't remember in detail and maybe this should be brought up on list and/or changed.  Also, such behavior can be punted to a later patch if non-trivial...but we should catch the error and raise an exception if so.
Comment 3 Hans Wennborg 2011-01-19 07:27:16 PST
(In reply to comment #2)
> Let's cache the last autonumber.  It's even OK if we leak some numbers as long as it always increases.
Done.

> Also, I believe that the expected behavior is that if you have a keypath and autoIncrement value, the latter should get inserted into the former...even if it already exists.  I don't remember in detail and maybe this should be brought up on list and/or changed.  Also, such behavior can be punted to a later patch if non-trivial...but we should catch the error and raise an exception if so.

You're right, I was reading an old version of the spec. And injecting the auto incremented value into an object is non-trivial, so I'm punting on that.

Re-writing the big if-else block at the start of IDBObjectStoreBackendImpl::putInternal to hopefully make it a bit clearer.
Comment 4 Hans Wennborg 2011-01-19 07:27:40 PST
Created attachment 79424 [details]
Patch
Comment 5 WebKit Review Bot 2011-01-19 07:29:59 PST
Attachment 79424 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 2

perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
	LANGUAGE = (unset),
	LC_ALL = (unset),
	LANG = "en_US.US-ASCII"
    are supported and installed on your system.
perl: warning: Falling back to the standard locale ("C").
Updating OpenSource
perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
	LANGUAGE = (unset),
	LC_ALL = (unset),
	LANG = "en_US.US-ASCII"
    are supported and installed on your system.
perl: warning: Falling back to the standard locale ("C").
RA layer request failed: OPTIONS of 'http://svn.webkit.org/repository/webkit': could not connect to server (http://svn.webkit.org) at /usr/lib/git-core/git-svn line 2295

Died at Tools/Scripts/update-webkit line 132.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Jeremy Orlow 2011-01-19 08:07:13 PST
Comment on attachment 79424 [details]
Patch

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

r=me

> Source/WebCore/storage/IDBObjectStoreBackendImpl.cpp:220
> +            callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::UNKNOWN_ERR, "Adding data to object stores with auto increment and in-line keys not currently supported."));

"Not yet supported"?
Comment 7 Hans Wennborg 2011-01-19 08:24:28 PST
Committed r76126: <http://trac.webkit.org/changeset/76126>