Closed Bug 707124 Opened 13 years ago Closed 13 years ago

Add URI to fetch schema version in BrowserProvider

Categories

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

All
Android
defect

Tracking

(firefox11 fixed, fennec11+)

RESOLVED FIXED
Tracking Status
firefox11 --- fixed
fennec 11+ ---

People

(Reporter: lucasr, Assigned: lucasr)

References

Details

Attachments

(1 file)

To allow sync handle different browser schema versions.
Blocks: 707240
Attachment #580072 - Flags: review?(blassey.bugs)
Attachment #580072 - Flags: feedback?(rnewman)
Assignee: nobody → lucasr.at.mozilla
Priority: -- → P3
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?
Attachment #580072 - Flags: feedback?(rnewman) → feedback-
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+ :)
Attachment #580072 - Flags: feedback- → feedback+
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
Attachment #580072 - Flags: review?(blassey.bugs) → review+
(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.
Pushed: http://hg.mozilla.org/mozilla-central/rev/1817ebca70d2
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
tracking-fennec: --- → 11+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: