[whatwg] Couple comments on Database storage spec.

[Ian Hickson]

On Fri, 5 Oct 2007, Scott Hess wrote:
>
> It may be worthwhile for Database to export a quote(arg) function, which 
> will quote the argument in the appropriate manner for use in 
> constructing a statement.  This is useful for cases where it is 
> challenging to reduce something to a static SQL statement with bind 
> parameters.  [A common case for this is something like "SELECT rowid 
> FROM t WHERE c IN (...)", and you want to replace ... with an 
> appropriately quoted comma-separated array.]

   var q = "";
   for each (var i in array)
     q += (q == "" ? "" : ", ") + "?";
   executeSql('SELECT rowid FROM t WHERE c IN (' + q + ')', array, ...);

You'd have to do pretty much the same thing with quote(), except the above 
is safer. Does that remove the need for quote()?


> I didn't fully understand how things would be ordered if there were 
> multiple executeSql() trees open on a given Database object.  For 
> instance:
> 
>   executeSql('sql1', function (args) {
>     executeSql('sql2', function (args) {
>       executeSql('sql3', function (args) {
>       });
>     });
>     executeSql('sql4', function (args) {
>     });
>   });
>   executeSql('sql5', function (args) {
>   });
> 
> I _think_ the spec implies 1, 5, 2, 4, 3 is the order (breadth-first).

There are two transactions there. One is 1,2,4,3, the other is 5. (So 
yes, you are correct.)


> In section 4.11.4, would it be worthwhile to indicate that an empty 
> result set would have rows which was exactly an empty array? Currently, 
> it indicates that in case of failure rows must return null, but I could 
> see an implementor returning null for an empty result set, also.  
> [Maybe I'm reading too much in, here.]

This is explicit now.


> Should rowsAffected be expected to return 0 in case of a SELECT
> statement, or should it return the length of rows array?

Explicitly noted this as zero.


> As a general comment, we've had some arguments within Gears about 
> whether we should use the same function for both update queries and 
> data-returning queries.  Right now, everything goes through execute(), 
> and if the query returned no data (such as "INSERT ..."), you get a 
> ResultSet which doesn't need to be closed and where many functions throw 
> exceptions.  Likewise, if you call "SELECT ...", then sections of the 
> results which make sense for updates (insertId, rowsAffected) make no 
> sense.  One suggestion within Gears was to have two functions:
> 
>   ResultSet query(sql, [arg, ...]);
>   int do(sql, [arg, ...]);
> 
> The first would return a ResultSet which needed to be closed, the second 
> would return the number of rows changed by the query.  The main thing is 
> that what you do with your results would always depend on the API 
> function you called, rather than the SQL statement you passed. You could 
> still do query("INSERT...") or do("SELECT..."), but the return values 
> would still be a ResultSet you needed to close, and 0, respectively.
>
> The Database storage spec does away with the Gears ResultSet.close() 
> issue, but I think the idea still has merit.  It could potentially do 
> away with the ResultSet entirely, the different types of SQL query could 
> have callbacks accepting the right few arguments directly.

I'm not really sure I understand what would be different in the spec 
version of the API (given the lack of a close() method). There aren't that 
many APIs on SQLResultSet, and I don't really see what's wrong with 
insertId raising exceptions unless something was inserted, or rowsAffected 
being zero unless something was affected, or rows being null unless 
something was returned. What's the advantage of being able to do

   do('SELECT ...')

...? And if you can do query('UPDATE ...'), why do we need do()?

Cheers,
-- 
Ian Hickson               U+1047E                )\._.,--....,'``.    fL
http://ln.hixie.ch/       U+263A                /,   _.. \   _\  ;`._ ,.
Things that are impossible just take longer.   `._.-(,_..'--(,_..'`-.;.'