Closed
Bug 261861
Opened 20 years ago
Closed 16 years ago
initial mozStorage interfaces
Categories
(Toolkit :: Storage, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: vlad, Unassigned)
Details
Attachments
(1 file)
4.05 KB,
application/x-compressed-tar
|
Details |
Attached will be a tarball containing the initial mozStorage IDL interfaces. There are some issues that need to get resolved, based on a recent discussion -- specifically, mozIStorageConnection needs to get a CreateTable call, and needs to lose the sqlite3Handle attribute. Also, "mozIStorage" might want to get changed to "stI" or similar. Other feedback welcome.
Reporter | ||
Comment 1•20 years ago
|
||
storage-interfaces.tar
Comment 2•20 years ago
|
||
some comments... note: I do not know sqlite. I consider this an advantage in this case :) I do think these interfaces/their docs should make sense w/o knowing sqlite. mozIStorageConnection: maybe move the initialize method to the top what would connectionReady be used for? Is it true iff initialize was called and succeeded? what's a row id and what can I do with one? what's the error code on this interface? Do I need sqlite headers to make sense of them? is the error string meant to be human-readable? if so, is it localized? Why do nsresult errors from the methods not suffice? * Initialize the connection by opening the specified database. does the file need to exist? * Returns true if a transaction is active on this connection. so if no txn is active, updates are immediately committed? * Begin a new transaction. If a transaction is active, * @throws NS_ERROR_STORAGE_INVALID_OPERATION. that will not make any sense in doxygen-generated documentation. try: * Begin a new transaction. * @throws NS_ERROR_STORAGE_INVALID_OPERATION if a transaction ... void createFunction (in string aFunctionName, so the function name must be ASCII? this is, apparently, not a stored procedure? what are sqlite functions good for? ah... for use with triggers, I see :) void createTrigger (in string aTriggerName, in long aTriggerType, in string aTableName, in string aTriggerFunction, in string aParameters); all of those parameters are limited to ascii? especially, the table name, and parameters are? name is only used for use with removeTrigger? * Abort any current operation, forcing a return * with an aborted error message. does this mean executeSimpleSql and the like is async? how would I get notified for completion? [noscript] readonly attribute sqlite3ptr sqliteHandle; I agree that this should go :) (along with the #include) these interfaces could generally use a comment above them describing their purpose. mozIStorageDataSchema seems a bit unused - not referenced by the other files? on this iface: readonly attribute nsIArray columnNames; maybe an nsIUTF8StringEnumerator would be more pleasant to use from JS... but if not, can you document the type of the array elements? (presumably nsISupportsString?) mozIStorageDataSet: * All the rows in this data set, as a nsIArray ...with elements of type mozIStorageValueArray, right? is an nsISupportsArray better than an nsIArray? (no idea how the two compare) mozIStorageFunction: wouldn't it make sense to pass the connection to it, so it can easily do stuff with the database? mozIStorageService: leaving aside what the meaning of "profile" and "cert" are - do I need to initialize() the resulting connection? if this is the only way that I'm supposed to create connections (I hope it isn't! I want to CreateInstance them), why an initialize method on the connection? * the appropriate storage for the current thread. multiple threads have different storages? mozIStorageStatement: is there a need for initialize()? the connection has a createStatement function, which should be enough... void bindCStringParameter (in unsigned long aParamIndex, in string aValue); I am not sure that it is wise to ignore charsets here. void bindDataParameter (in unsigned long aParamIndex, [array,size_is(aValueSize)] in octet aValue, in unsigned long aValueSize); that array could be [const], I think? it is for BLOBs, right? each call to executeStep gets the next result row, right? what happens if there are no more rows? is there a need for this method, why does executeDataSet not suffice? mozIStorageValueArray: why is this in a file called mozIStorageValue? string getAsCString(in unsigned long aIndex); _please_ reconsider adding this method. I have a fear that people will call it where they shouldn't, causing i18n bugs. /* XXX handle blobs! */ other than the getAsBlob method above? [notxpcom] string asSharedCString(in unsigned long aIndex, out unsigned long aLength); which charset? can these notxpcom methods ever fail?
Comment 3•20 years ago
|
||
SQLite3 appears to use UTF-8 strings without restriction to ASCII, so perhaps anywhere where |string| appears, |AUTF8String| should appear instead. That is unless we intentionally wish to limit certain names to ASCII. mozIStorageConnection::initialize is going away (or so i understood) in favor of using mozIStorageService to access storage connections. > readonly attribute nsIArray columnNames; > > maybe an nsIUTF8StringEnumerator would be more pleasant to use from JS... but > if not, can you document the type of the array elements? (presumably > nsISupportsString?) yeah, an enumerator interface is usually preferrable to a readonly array interface. > * Abort any current operation, forcing a return > * with an aborted error message. > > does this mean executeSimpleSql and the like is async? how would I get > notified for completion? Abort is part of the mozIStorageStatement::executeCallback feature, and AFAIK it is going away, so I presume that Abort will be going away too. executeSimpleSql completes synchronously. all of the |execute| functions complete synchronously. if you need to do things asynchronously then you can use executeStep to access one result row at a time, but vlad is to verify that SQLite3 supports overlapping statements with pending result data. (if not, then implementing iterators on top of sqlite3 is going to suck.) > void bindCStringParameter (in unsigned long aParamIndex, in string aValue); > I am not sure that it is wise to ignore charsets here. If that method is implemented as a direct call to sqlite3, then aValue must be UTF-8, but XPConnect of course restricts it to ASCII. I don't think there's any value in exposing this method to script. biesi: nsISupportsArray is deprecated in favor of nsIArray.
Reporter | ||
Comment 4•20 years ago
|
||
(In reply to comment #2) > some comments... > > note: I do not know sqlite. I consider this an advantage in this case :) I do > think these interfaces/their docs should make sense w/o knowing sqlite. I agree; a lot of the stuff is fairly SQL-specific, and needs to be explained better. > mozIStorageConnection: > > maybe move the initialize method to the top Initialize is going away; mozIStorageService will take care of setting up the connection to the right file, etc. > what would connectionReady be used for? Is it true iff initialize was called and > succeeded? It's true if the connection is ready to execute a statement; if it's in the middle of another execution (e.g. someone's calling executeStep somewhere), then it's not ready. However, as this is entirely single threaded, the connection should never be not-ready, so this can probably go away. > what's a row id and what can I do with one? The database works in terms of rows in tables; each row in each table has a unique oid ("object id")/row id. You can retreive the oid of the most recently created row using this, so that you can reference it later without having to search for it. > what's the error code on this interface? Do I need sqlite headers to make sense > of them? You do for now; I'll copy the values into the idl, but there's some oddness with sqlite's error handling (some error states aren't getting saved as they should be). > is the error string meant to be human-readable? if so, is it localized? It's human readable, but it's not localized. I've sent mail to the sqlite mailing list about this and got no response; I'll probably do a patch to make it localizable and send it out to them. Internally it'll be trivial to make it localizable, as the string formatting already goes through a table of strings and the like. > Why do nsresult errors from the methods not suffice? Things like errmsg can provide extra information, e.g. "Syntax error after 'ON'" or somesuch. > * Returns true if a transaction is active on this connection. > so if no txn is active, updates are immediately committed? Right; if there is no transaction active, each writing statement has an implicit transaction wrapped around it. > * Begin a new transaction. If a transaction is active, > * @throws NS_ERROR_STORAGE_INVALID_OPERATION. > > that will not make any sense in doxygen-generated documentation. try: > * Begin a new transaction. > * @throws NS_ERROR_STORAGE_INVALID_OPERATION if a transaction ... Ah; I need to do a bit of reading up on doxygen, I was sort of making things up as I went along ;) > void createFunction (in string aFunctionName, > > so the function name must be ASCII? It doesn't have to be for sqlite, but I'd like to force it to be. > this is, apparently, not a stored procedure? > what are sqlite functions good for? For defining custom functions that can execute as part of SQL statements; in particular, triggers (notifications when a row is added/deleted/modified in a table). > ah... for use with triggers, I see :) Er, right :) > void createTrigger (in string aTriggerName, > in long aTriggerType, > in string aTableName, > in string aTriggerFunction, > in string aParameters); > > all of those parameters are limited to ascii? especially, the table name, and > parameters are? As with the function name, they're not in sqlite, but I'd like them to be. I see no benefit to allowing arbitrary utf8/utf16 for function/trigger/table/column/etc. names. Are there any reasons for doing so? > name is only used for use with removeTrigger? Yep; it also can't conflict with any currently existing trigger. > * Abort any current operation, forcing a return > * with an aborted error message. > does this mean executeSimpleSql and the like is async? how would I get notified for completion? Nope, it's all sync. The abort stuff is actually only useful with executeStep() stuff -- but as everything is happening on a single thread, I'm not sure now what the value is of exposing abort. > [noscript] readonly attribute sqlite3ptr sqliteHandle; > > I agree that this should go :) (along with the #include) Yes :) > these interfaces could generally use a comment above them describing their purpose. > > mozIStorageDataSchema seems a bit unused - not referenced by the other files? > on this iface: > readonly attribute nsIArray columnNames; Whoops, yeah.. mozIStorageDataSchema was used for something that I removed; however, it might make a comeback. I think the right thing here would be to just have "readonly attribute long numColumns;" and "AUTF8String columnName(in long aColumnIndex)" to simplify things in both C++ and JS. > mozIStorageDataSet: > * All the rows in this data set, as a nsIArray > > ...with elements of type mozIStorageValueArray, right? Yep. > mozIStorageFunction: > wouldn't it make sense to pass the connection to it, so it can easily do stuff > with the database? At the time it gets called, I have no access to the mozIStorageConnection without creating my own closure. I figure since someone will have to implement a class to use mozIStorageFunction anyway, they can just pass in the connection as part of the constructor. > mozIStorageService: > leaving aside what the meaning of "profile" and "cert" are - do I need to > initialize() the resulting connection? > if this is the only way that I'm supposed to create connections (I hope it > isn't! I want to CreateInstance them), why an initialize method on the connection? mozIStorageService needs to be fleshed out more; the "profile" "cert" etc. stuff was a last-minute note in there. > * the appropriate storage for the current thread. > multiple threads have different storages? > > mozIStorageStatement: > is there a need for initialize()? the connection has a createStatement function, > which should be enough... True. > void bindCStringParameter (in unsigned long aParamIndex, in string aValue); > > I am not sure that it is wise to ignore charsets here. This should probably be [noscript]. > void bindDataParameter (in unsigned long aParamIndex, > [array,size_is(aValueSize)] in octet aValue, in unsigned long aValueSize); > > that array could be [const], I think? > it is for BLOBs, right? Yep, forgot to explicitly specify const. > each call to executeStep gets the next result row, right? what happens if there > are no more rows? > is there a need for this method, why does executeDataSet not suffice? null gets returned when there are no more rows, and eventually the nsresult will be some non-NS_OK success value. > mozIStorageValueArray: > why is this in a file called mozIStorageValue? Mm. Good question. There was going to be a mozIStorageValue at some point, but it turned into a ValueArray. > string getAsCString(in unsigned long aIndex); > _please_ reconsider adding this method. I have a fear that people will call it > where they shouldn't, causing i18n bugs. Hmm. We can probably live without it; I put it in for completeness. > /* XXX handle blobs! */ > other than the getAsBlob method above? Erm. comment bug :) > [notxpcom] string asSharedCString(in unsigned long aIndex, out unsigned long > aLength); > which charset? > can these notxpcom methods ever fail? charset will always be UTF8 here. In theory they can fail if aIndex is out of range, but they're way too convenient to not include them. Thanks for the feedback! I'll incorporate the suggestions in the next few days.
Status: NEW → ASSIGNED
Product: Browser → Toolkit
Version: Trunk → unspecified
Comment 5•20 years ago
|
||
Have you looked at http://lxr.mozilla.org/mozilla/source/extensions/sql ? I don't say my component is better, but it is very similar to yours and already supports PostgreSQL, MySQL and SQLite. I didn't watch the tree for some time, but I'm very surprised that nobody else mentioned this component. Was it on purpose?
Reporter | ||
Updated•18 years ago
|
Assignee: vladimir → nobody
Status: ASSIGNED → NEW
Comment 6•18 years ago
|
||
mozStorage has been in the tree for quite a while. Any objections to closing this bug?
Comment 7•17 years ago
|
||
I'm keeping it open because there are a few comments that haven't been addressed, after which I'll close it.
Comment 8•17 years ago
|
||
Is this http://lxr.mozilla.org/mozilla/source/extensions/sql/odbc/ compatible with the XulRunner trunk 1.9? Where can I find a binary package of this extension, where a documentation of how to install it to XulRunner?
Comment 9•16 years ago
|
||
I'm gonna go ahead and close this now. It's not worth keeping open.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•