707124 - Add URI to fetch schema version in BrowserProvider

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Lucas Rocha (:lucasr)]

To allow sync handle different browser schema versions.

Comment 1

[Lucas Rocha (:lucasr)]

Attachment: Add query to BrowserProvider to fetch DB schema version

Comment 2

[Richard Newman [:rnewman]]

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]]

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?)]

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)]

(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)]

Pushed: http://hg.mozilla.org/mozilla-central/rev/1817ebca70d2