403376 - mozIStorageConnection::schemaVersion misleadingly returns PRAGMA user_version

Status: RESOLVED WONTFIX

(Core :: SQLite and Embedded Database Bindings, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

But 386366 adds a property to mozIStorageConnection for getting the user_version pragma, but it misleadingly calls it "schemaVersion", which makes it sound like it retrieves the schema_version pragma.

It seems like the property should be called "userVersion" instead, since that matches the SQLite documentation, and even though Places and the content prefs service both use the pragma to store their schema version, that pragma can be used for anything; it doesn't need to be used for schema versioning.

Comment 2

[Shawn Wilsher :sdwilsh]

I'm inclined to WONTFIX this bug.  mozStorage is supposed to be a database abstraction layer, and all this is is an abstraction.  Users cannot actually use the schema version in sqlite, but they can store their own schema version in user_version.

Comment 3

[lazierthanthou]

(In reply to comment #2)
> I'm inclined to WONTFIX this bug.  mozStorage is supposed to be a database
> abstraction layer, and all this is is an abstraction.  Users cannot actually
> use the schema version in sqlite, but they can store their own schema version
> in user_version.
> 

I think I agree with you; but probably there could be a rethink on calling it something other than schemaVersion. 
schemaVersion unnecessarily gets confused with schema_version in pragma schema_version, and hence this bug. 

Comment 4

[Adam Kowalczyk]

How about calling it databaseVersion?

Comment 5

[Shawn Wilsher :sdwilsh]

I don't quite like that - it's used to check the version of the database schema.  I really think the only issue here is that sqlite uses schema_version internally, and as a result, causes this confusion.  However, mozStorage is a database abstraction layer (currently only wrapping sqlite).

Comment 6

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #5)
> mozStorage is a database abstraction layer (currently only wrapping sqlite).

Is that really true?  I've always thought of it as an API specifically for accessing SQLite through XPCOM.  Its primitives closely mirror SQLite's, unlike the older mozSQL interface in extensions/sql, which was actually designed to be an abstraction layer.  And MDC describes it like that, saying "Storage is a SQLite database API." <http://developer.mozilla.org/en/docs/Storage>

mozStorage also doesn't provide any schema manipulation facilities, and consumers can stick arbitrary values into the user_version pragma that it exposes through the schemaVersion attribute.  They don't have to use it to store a schema version.

schemaVersion would make sense to me as an attribute in an API for managed schema revisions (which may or may not use the user_version pragma to track that value).  But in the general API, and given that mozStorage developers frequently consult SQLite documentation to figure out how to use the API, it makes more sense to call it what it is.

Comment 7

[Shawn Wilsher :sdwilsh]

While it is true that it currently closely maps to the SQLite api, I've been trying to move away from that because it's too easy to do things wrong.  Please see the thread in dev.planning for mozStorage and moz2:
http://groups.google.com/group/mozilla.dev.planning/browse_thread/thread/240a264c08c6eb7a

Comment 8

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #7)
> While it is true that it currently closely maps to the SQLite api, I've been
> trying to move away from that because it's too easy to do things wrong.  Please
> see the thread in dev.planning for mozStorage and moz2:
> http://groups.google.com/group/mozilla.dev.planning/browse_thread/thread/240a264c08c6eb7a

Yup, I'm aware of the thread, and I think your proposal is a great idea, but it's a great idea because it makes using SQLite safer and easier for consumers (for which there is great demand), not because it makes it possible for consumers to use mozStorage with other databases (for which, as far as I am aware, there is no demand).

And in any case, this attribute is not part of that proposed SQLAlchemy-like API, it's part of the existing SQLite-like API.

Comment 9

[Shawn Wilsher :sdwilsh]

bug 452897 helps out a bit

Comment 10

[Shawn Wilsher :sdwilsh]

disregard last comment - wrong bug

Comment 11

[Shawn Wilsher :sdwilsh]

I am going to close this bug as WONTFIX.  Other than the comments in this bug, I haven't heard any complaints about it, and the arguments made against it haven't convinced me to change my mind.  Sorry.