Closed
Bug 386366
Opened 17 years ago
Closed 17 years ago
Add convenience method for getting the database schema version
Categories
(Toolkit :: Storage, enhancement)
Toolkit
Storage
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha8
People
(Reporter: sdwilsh, Assigned: sdwilsh)
References
Details
Attachments
(1 file, 1 obsolete file)
6.17 KB,
patch
|
moco
:
review+
|
Details | Diff | Splinter Review |
Useful for all of our consumers.
Assignee | ||
Comment 1•17 years ago
|
||
Attachment #270360 -
Flags: review?(sspitzer)
Comment 2•17 years ago
|
||
1) from http://www.sqlite.org/pragma.html "The pragmas schema_version and user_version are used to set or get the value of the schema-version and user-version, respectively. Both the schema-version and the user-version are 32-bit signed integers stored in the database header." So, in the idl: attribute unsigned long schemaVersion; that should not be unsigned, right? 2) + *version = 0; + PRBool hasResult; + if (NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) + *version = stmt->AsInt64(0); Should that be AsInt32(), not AsInt64() 3) for your tests, should verify that you can set the version to -1, too? perhaps also test setting the version if it is already set (go from 1 to 2).
Updated•17 years ago
|
Attachment #270360 -
Flags: review?(sspitzer)
Assignee | ||
Comment 3•17 years ago
|
||
(In reply to comment #2) > that should not be unsigned, right? ah, yes. Good catch. > Should that be AsInt32(), not AsInt64() Now, yes, but before we needed AsInt64 to get the full range of a PRUint32 (or almost) > for your tests, should verify that you can set the version to -1, too? perhaps > also test setting the version if it is already set (go from 1 to 2). I can do that
Assignee | ||
Comment 4•17 years ago
|
||
Attachment #270360 -
Attachment is obsolete: true
Attachment #270365 -
Flags: review?(sspitzer)
Comment 5•17 years ago
|
||
should we be returning an error when the schema is not set? see http://lxr.mozilla.org/seamonkey/source/toolkit/components/places/src/nsNavHistory.cpp#523 and cool, you test both the negative schema and the modification of a set schema (going from 1 to -1) with just one additional test.
Assignee | ||
Comment 6•17 years ago
|
||
(In reply to comment #5) > should we be returning an error when the schema is not set? Well, in the idl I say that we return 0 if it isn't set, which is what happens anyway in my experience using SQLite Database Browser (and makes sense based on their docs saying that is always stored with the database). Consumers can always do an NS_ENSURE_TRUE on the result.
Comment 7•17 years ago
|
||
Comment on attachment 270365 [details] [diff] [review] v1.1 r=sspitzer, thanks for answering my questions. I'm getting 0 back when the schema is not set as well, but also, I like return 0 in either case (not set or 0.)
Attachment #270365 -
Flags: review?(sspitzer) → review+
Comment 8•17 years ago
|
||
storage/public/mozIStorageConnection.idl 1.9 storage/src/mozStorageConnection.cpp 1.22 storage/test/unit/test_storage_connection.js 1.2
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Assignee | ||
Updated•17 years ago
|
Keywords: dev-doc-needed
Comment 9•17 years ago
|
||
Removing doc needed keyword in favor of a new bug that covers the fact that in general we actually need proper documentation for Storage; that bug (bug 399535) is set as blocked by this one.
Keywords: dev-doc-needed
You need to log in
before you can comment on or make changes to this bug.
Description
•