Last Comment Bug 707124 - Add URI to fetch schema version in BrowserProvider
: Add URI to fetch schema version in BrowserProvider
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All Android
: P3 normal (vote)
: ---
Assigned To: Lucas Rocha (:lucasr)
:
Mentors:
Depends on:
Blocks: 707240
  Show dependency treegraph
 
Reported: 2011-12-02 03:37 PST by Lucas Rocha (:lucasr)
Modified: 2012-01-09 11:39 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
11+


Attachments
Add query to BrowserProvider to fetch DB schema version (6.83 KB, patch)
2011-12-08 09:25 PST, Lucas Rocha (:lucasr)
blassey.bugs: review+
rnewman: feedback+
Details | Diff | Review

Description Lucas Rocha (:lucasr) 2011-12-02 03:37:37 PST
To allow sync handle different browser schema versions.
Comment 1 Lucas Rocha (:lucasr) 2011-12-08 09:25:58 PST
Created attachment 580072 [details] [diff] [review]
Add query to BrowserProvider to fetch DB schema version
Comment 2 Richard Newman [:rnewman] 2011-12-08 12:38:02 PST
Comment on attachment 580072 [details] [diff] [review]
Add query to BrowserProvider to fetch DB schema version

Review of attachment 580072 [details] [diff] [review]:
-----------------------------------------------------------------

I'm flagging this as f- for the reason below, *but* if you can preserve the schema access API and switch out the storage beneath it, then you can fix this concern in a follow-up. You just have to do it prior to your first schema change.

---

How will you migrate databases between versions?

Unless I'm majorly misunderstanding, the schema version here is not a property of the database, it's a property of the code that's opening it. So when you need to bump Fennec's bookmark schema, and thus upgrade the DB, you can't find out how stale the DB is: you have to figure out by inspection of the tables. That's not fun.

Furthermore, if Fennec hasn't run yet after being upgraded, your migration code might not have run, unless you want to hook that into every query in the provider. We'll ask for a version, and you'll return the in-code constant "2", despite the schema on disk still being the one created by a v1 Fennec.


The Firefox storage component uses a sqlite pragma for this purpose:

Connection::GetSchemaVersion(PRInt32 *_version)
{
  if (!mDBConn) return NS_ERROR_NOT_INITIALIZED;

  nsCOMPtr<mozIStorageStatement> stmt;
  (void)CreateStatement(NS_LITERAL_CSTRING("PRAGMA user_version"),
                        getter_AddRefs(stmt));
  NS_ENSURE_TRUE(stmt, NS_ERROR_OUT_OF_MEMORY);

  *_version = 0;
  bool hasResult;
  if (NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult)
    *_version = stmt->AsInt32(0);

  return NS_OK;
}

Perhaps consider something similar?
Comment 3 Richard Newman [:rnewman] 2011-12-08 12:41:29 PST
Comment on attachment 580072 [details] [diff] [review]
Add query to BrowserProvider to fetch DB schema version

"unless you want to hook that into every query in the provider"…

and that's exactly what happens. So f+ :)
Comment 4 Brad Lassey [:blassey] (use needinfo?) 2011-12-09 23:04:37 PST
Comment on attachment 580072 [details] [diff] [review]
Add query to BrowserProvider to fetch DB schema version

Review of attachment 580072 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/db/BrowserProvider.java
@@ +704,5 @@
>                  break;
>              }
>  
> +            case SCHEMA: {
> +                Log.d(LOGTAG, "Query is on schema: " + uri);

loose the logging
Comment 5 Lucas Rocha (:lucasr) 2011-12-12 04:52:15 PST
(In reply to Brad Lassey [:blassey] from comment #4)
> Comment on attachment 580072 [details] [diff] [review]
> Add query to BrowserProvider to fetch DB schema version
> 
> Review of attachment 580072 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/db/BrowserProvider.java
> @@ +704,5 @@
> >                  break;
> >              }
> >  
> > +            case SCHEMA: {
> > +                Log.d(LOGTAG, "Query is on schema: " + uri);
> 
> loose the logging

This logging is very useful for debugging. Especially for the sync team.
Comment 6 Lucas Rocha (:lucasr) 2011-12-13 06:15:38 PST
Pushed: http://hg.mozilla.org/mozilla-central/rev/1817ebca70d2

Note You need to log in before you can comment on or make changes to this bug.