[Webkit-unassigned] [Bug 28918] Add support for Database.readTransaction()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=28918





--- Comment #5 from Dumitru Daniliuc <dumi at chromium.org>  2009-09-04 12:06:25 PDT ---
(In reply to comment #4)
> This looks pretty nice. So the method is available, and a flag is plumbed
> throughout, and the authorizer enforces readonly'ness. I take it you're saving
> allowing concurrent r/o transactions on the same db thread for a later patch?

Yes. The goal is to be able to allow multiple read transactions at the same
time.

> http://www.sqlite.org/lang_reindex.html
> It looks like REINDEX is a write operation, but I think its being allowed for
> readOnly transactions... is this what you intended and why?

I wasn't sure what to do with REINDEX. On one hand, it seems to just recreate
some existing indexes (and it doesn't seem to trigger any other "write"
operations like DELETE, or UPDATE; at least not for the SQL statement I have in
the test). On the other hand, it does look like a write operation.

I changed the authorizer to not allow REINDEX in read-only transactions.

> What does the HTML5 draft say about the behavior of a readOnly transaction when
> a write operation is attempted? As coded, looks like the statement fails, but
> the transaction is left open and other statements can be executed in that
> transaction. Does that match the spec'd behavior? I can imagine another
> behavior, the transaction fails and is closed when a write operation is
> attempted... just checking.

Here's what the spec says about statements that fail:

1. If the statement had an associated error callback, then queue a task to
invoke that error callback with the SQLTransaction object and a newly
constructed SQLError object that represents the error that caused these
substeps to be run as the two arguments, respectively, and wait for the task to
be run.
2. If the error callback returns false, then move on to the next statement, if
any, or onto the next overall step otherwise.
3. Otherwise, the error callback did not return false, or there was no error
callback. Jump to the last step in the overall steps.

So it seems to me like the read transactions in the test should've failed after
the first statement with an error. I'm not sure why it didn't happen (at the
first look the code seems to do what the spec says); I'm looking into it.

> 7979     void transaction(PassRefPtr<SQLTransactionCallback> callback,
> PassRefPtr<SQLTransactionErrorCallback> errorCallback,
> 80                       PassRefPtr<VoidCallback> successCallback);
>  80                      PassRefPtr<VoidCallback> successCallback, bool
> readOnly = false);
> 
> Are there any callsites that don't explicitly pass a value for the readOnly
> parameter? Consider removing the default param value.

done.

> BTW... have you seen the SAVEPOINT statement in newer versions of SQLite? Looksf that capability
> like support in sqlite3  for transaction nesting. Wondering i
> should be reflected in the HTML5 api in some way?
> http://www.sqlite.org/lang_savepoint.html

Michael and I talked a bit about this, and I think the conclusions were:
1. It's not in the SQLite version we currently use.
2. Doesn't seem to add anything new (could be replaced by multiple
transactions?), but might be useful to app developers anyway.
3. Should probably talk to app developers first before proposing to add it to
the spec.
4. Might be better to wait until all other issues are resolved and we have a
more "final" version of the spec that's approved by everybody.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.