Bug 1024289: Part 1: remove redundant subquery from combined view and add upgrade path. r=rnewman

This commit is contained in:
Chris Kitching 2014-07-07 14:52:37 -07:00
parent c7d32d44f1
commit bed91760d4

View File

@ -34,7 +34,7 @@ import android.util.Log;
final class BrowserDatabaseHelper extends SQLiteOpenHelper {
private static final String LOGTAG = "GeckoBrowserDBHelper";
public static final int DATABASE_VERSION = 18;
public static final int DATABASE_VERSION = 19;
public static final String DATABASE_NAME = "browser.db";
final protected Context mContext;
@ -653,73 +653,108 @@ final class BrowserDatabaseHelper extends SQLiteOpenHelper {
" ON " + Combined.FAVICON_ID + " = " + qualifyColumn(TABLE_FAVICONS, Favicons._ID));
}
private void createCombinedViewOn16(SQLiteDatabase db) {
debug("Creating " + VIEW_COMBINED + " view");
private void createCombinedViewOn19(SQLiteDatabase db) {
/*
The v19 combined view removes the redundant subquery from the v16
combined view and reorders the columns as necessary to prevent this
from breaking any code that might be referencing columns by index.
The rows in the ensuing view are, in order:
Combined.BOOKMARK_ID
Combined.HISTORY_ID
Combined._ID (always 0)
Combined.URL
Combined.TITLE
Combined.VISITS
Combined.DISPLAY
Combined.DATE_LAST_VISITED
Combined.FAVICON_ID
We need to return an _id column because CursorAdapter requires it for its
default implementation for the getItemId() method. However, since
we're not using this feature in the parts of the UI using this view,
we can just use 0 for all rows.
*/
db.execSQL("CREATE VIEW IF NOT EXISTS " + VIEW_COMBINED + " AS" +
" SELECT " + Combined.BOOKMARK_ID + ", " +
Combined.HISTORY_ID + ", " +
// We need to return an _id column because CursorAdapter requires it for its
// default implementation for the getItemId() method. However, since
// we're not using this feature in the parts of the UI using this view,
// we can just use 0 for all rows.
"0 AS " + Combined._ID + ", " +
Combined.URL + ", " +
Combined.TITLE + ", " +
Combined.VISITS + ", " +
Combined.DISPLAY + ", " +
Combined.DATE_LAST_VISITED + ", " +
Combined.FAVICON_ID +
" FROM (" +
// Bookmarks without history.
" SELECT " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks._ID) + " AS " + Combined.BOOKMARK_ID + ", " +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.URL) + " AS " + Combined.URL + ", " +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.TITLE) + " AS " + Combined.TITLE + ", " +
"CASE " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.PARENT) + " WHEN " +
Bookmarks.FIXED_READING_LIST_ID + " THEN " + Combined.DISPLAY_READER + " ELSE " +
Combined.DISPLAY_NORMAL + " END AS " + Combined.DISPLAY + ", " +
"-1 AS " + Combined.HISTORY_ID + ", " +
"-1 AS " + Combined.VISITS + ", " +
"-1 AS " + Combined.DATE_LAST_VISITED + ", " +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.FAVICON_ID) + " AS " + Combined.FAVICON_ID +
" FROM " + TABLE_BOOKMARKS +
" WHERE " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.TYPE) + " = " + Bookmarks.TYPE_BOOKMARK + " AND " +
// Ignore pinned bookmarks.
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.PARENT) + " <> " + Bookmarks.FIXED_PINNED_LIST_ID + " AND " +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.IS_DELETED) + " = 0 AND " +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.URL) +
" NOT IN (SELECT " + History.URL + " FROM " + TABLE_HISTORY + ")" +
" UNION ALL" +
// Bookmarks without history.
" SELECT " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks._ID) + " AS " + Combined.BOOKMARK_ID + "," +
"-1 AS " + Combined.HISTORY_ID + "," +
"0 AS " + Combined._ID + "," +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.URL) + " AS " + Combined.URL + ", " +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.TITLE) + " AS " + Combined.TITLE + ", " +
"-1 AS " + Combined.VISITS + ", " +
"CASE " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.PARENT) +
" WHEN " + Bookmarks.FIXED_READING_LIST_ID + " THEN " +
Combined.DISPLAY_READER +
" ELSE " +
Combined.DISPLAY_NORMAL +
" END AS " + Combined.DISPLAY + "," +
"-1 AS " + Combined.DATE_LAST_VISITED + "," +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.FAVICON_ID) + " AS " + Combined.FAVICON_ID +
" FROM " + TABLE_BOOKMARKS +
" WHERE " +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.TYPE) + " = " + Bookmarks.TYPE_BOOKMARK + " AND " +
// Ignore pinned bookmarks.
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.PARENT) + " <> " + Bookmarks.FIXED_PINNED_LIST_ID + " AND " +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.IS_DELETED) + " = 0 AND " +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.URL) +
" NOT IN (SELECT " + History.URL + " FROM " + TABLE_HISTORY + ")" +
" UNION ALL" +
// History with and without bookmark.
" SELECT " + "CASE " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.IS_DELETED) + " WHEN 0 THEN " +
// Give pinned bookmarks a NULL ID so that they're not treated as bookmarks. We can't
// completely ignore them here because they're joined with history entries we care about.
"CASE " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.PARENT) + " WHEN " +
Bookmarks.FIXED_PINNED_LIST_ID + " THEN NULL ELSE " +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks._ID) + " END " +
"ELSE NULL END AS " + Combined.BOOKMARK_ID + ", " +
qualifyColumn(TABLE_HISTORY, History.URL) + " AS " + Combined.URL + ", " +
// Prioritize bookmark titles over history titles, since the user may have
// customized the title for a bookmark.
"COALESCE(" + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.TITLE) + ", " +
qualifyColumn(TABLE_HISTORY, History.TITLE) +")" + " AS " + Combined.TITLE + ", " +
// Only use DISPLAY_READER if the matching bookmark entry inside reading
// list folder is not marked as deleted.
"CASE " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.IS_DELETED) + " WHEN 0 THEN CASE " +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.PARENT) + " WHEN " + Bookmarks.FIXED_READING_LIST_ID +
" THEN " + Combined.DISPLAY_READER + " ELSE " + Combined.DISPLAY_NORMAL + " END ELSE " +
Combined.DISPLAY_NORMAL + " END AS " + Combined.DISPLAY + ", " +
qualifyColumn(TABLE_HISTORY, History._ID) + " AS " + Combined.HISTORY_ID + ", " +
qualifyColumn(TABLE_HISTORY, History.VISITS) + " AS " + Combined.VISITS + ", " +
qualifyColumn(TABLE_HISTORY, History.DATE_LAST_VISITED) + " AS " + Combined.DATE_LAST_VISITED + ", " +
qualifyColumn(TABLE_HISTORY, History.FAVICON_ID) + " AS " + Combined.FAVICON_ID +
" SELECT " +
"CASE " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.IS_DELETED) +
// Give pinned bookmarks a NULL ID so that they're not treated as bookmarks. We can't
// completely ignore them here because they're joined with history entries we care about.
" WHEN 0 THEN " +
"CASE " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.PARENT) +
" WHEN " + Bookmarks.FIXED_PINNED_LIST_ID + " THEN " +
"NULL " +
"ELSE " +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks._ID) +
" END " +
"ELSE " +
"NULL " +
"END AS " + Combined.BOOKMARK_ID + "," +
qualifyColumn(TABLE_HISTORY, History._ID) + " AS " + Combined.HISTORY_ID + "," +
"0 AS " + Combined._ID + "," +
qualifyColumn(TABLE_HISTORY, History.URL) + " AS " + Combined.URL + "," +
// Prioritize bookmark titles over history titles, since the user may have
// customized the title for a bookmark.
"COALESCE(" + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.TITLE) + ", " +
qualifyColumn(TABLE_HISTORY, History.TITLE) +
") AS " + Combined.TITLE + "," +
qualifyColumn(TABLE_HISTORY, History.VISITS) + " AS " + Combined.VISITS + "," +
// Only use DISPLAY_READER if the matching bookmark entry inside reading
// list folder is not marked as deleted.
"CASE " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.IS_DELETED) + " " +
"WHEN 0 THEN " +
"CASE " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.PARENT) + " " +
"WHEN " + Bookmarks.FIXED_READING_LIST_ID + " THEN " +
Combined.DISPLAY_READER + " " +
"ELSE " + Combined.DISPLAY_NORMAL + " " +
"END " +
"ELSE " +
Combined.DISPLAY_NORMAL + " " +
"END AS " + Combined.DISPLAY + "," +
qualifyColumn(TABLE_HISTORY, History.DATE_LAST_VISITED) + " AS " + Combined.DATE_LAST_VISITED + "," +
qualifyColumn(TABLE_HISTORY, History.FAVICON_ID) + " AS " + Combined.FAVICON_ID +
" FROM " + TABLE_HISTORY + " LEFT OUTER JOIN " + TABLE_BOOKMARKS +
" ON " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.URL) + " = " + qualifyColumn(TABLE_HISTORY, History.URL) +
" WHERE " + qualifyColumn(TABLE_HISTORY, History.URL) + " IS NOT NULL AND " +
qualifyColumn(TABLE_HISTORY, History.IS_DELETED) + " = 0 AND (" +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.TYPE) + " IS NULL OR " +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.TYPE) + " = " + Bookmarks.TYPE_BOOKMARK + ") " +
")");
" ON " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.URL) + " = " + qualifyColumn(TABLE_HISTORY, History.URL) +
" WHERE " +
qualifyColumn(TABLE_HISTORY, History.URL) + " IS NOT NULL AND " +
qualifyColumn(TABLE_HISTORY, History.IS_DELETED) + " = 0 AND " +
"(" +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.TYPE) + " IS NULL OR " +
qualifyColumn(TABLE_BOOKMARKS, Bookmarks.TYPE) + " = " + Bookmarks.TYPE_BOOKMARK +
")"
);
debug("Creating " + VIEW_COMBINED_WITH_FAVICONS + " view");
@ -742,7 +777,7 @@ final class BrowserDatabaseHelper extends SQLiteOpenHelper {
createBookmarksWithFaviconsView(db);
createHistoryWithFaviconsView(db);
createCombinedViewOn16(db);
createCombinedViewOn19(db);
createOrUpdateSpecialFolder(db, Bookmarks.PLACES_FOLDER_GUID,
R.string.bookmarks_folder_places, 0);
@ -1298,10 +1333,10 @@ final class BrowserDatabaseHelper extends SQLiteOpenHelper {
}
private void upgradeDatabaseFrom15to16(SQLiteDatabase db) {
db.execSQL("DROP VIEW IF EXISTS " + VIEW_COMBINED);
db.execSQL("DROP VIEW IF EXISTS " + VIEW_COMBINED_WITH_FAVICONS);
createCombinedViewOn16(db);
// No harm in creating the v19 combined view here: means we don't need two almost-identical
// functions to define both the v16 and v19 ones. The upgrade path will redundantly drop
// and recreate the view again. *shrug*
createV19CombinedView(db);
}
private void upgradeDatabaseFrom16to17(SQLiteDatabase db) {
@ -1378,6 +1413,18 @@ final class BrowserDatabaseHelper extends SQLiteOpenHelper {
}
}
private void upgradeDatabaseFrom18to19(SQLiteDatabase db) {
// Redefine the "combined" view...
createV19CombinedView(db);
}
private void createV19CombinedView(SQLiteDatabase db) {
db.execSQL("DROP VIEW IF EXISTS " + VIEW_COMBINED);
db.execSQL("DROP VIEW IF EXISTS " + VIEW_COMBINED_WITH_FAVICONS);
createCombinedViewOn19(db);
}
@Override
public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
debug("Upgrading browser.db: " + db.getPath() + " from " +
@ -1454,6 +1501,10 @@ final class BrowserDatabaseHelper extends SQLiteOpenHelper {
case 18:
upgradeDatabaseFrom17to18(db);
break;
case 19:
upgradeDatabaseFrom18to19(db);
break;
}
}
@ -1566,4 +1617,4 @@ final class BrowserDatabaseHelper extends SQLiteOpenHelper {
bookmark.remove("folder");
}
}
}
}