Bug 709962 - Eliminate required table references in BrowserProvider's query interface (r=blassey, a=mfinkle)
authorLucas Rocha <lucasr@mozilla.com>
Tue, 13 Dec 2011 19:47:40 +0000
changeset 84152 50b3a3a55715fd952f783b1ec811f7dfb5d7e1c2
parent 84151 a111d03d465cf8de2332fc196d3639d7ed74663d
child 84153 096f185e573dd7339ee445a04499d05bcadc3bbb
push id519
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 00:38:35 +0000
treeherdermozilla-beta@788ea1ef610b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersblassey, mfinkle
bugs709962
milestone11.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 709962 - Eliminate required table references in BrowserProvider's query interface (r=blassey, a=mfinkle)
mobile/android/base/db/BrowserProvider.java
--- a/mobile/android/base/db/BrowserProvider.java
+++ b/mobile/android/base/db/BrowserProvider.java
@@ -78,16 +78,19 @@ public class BrowserProvider extends Con
 
     // Number of records marked as deleted to be removed
     static final long DELETED_RECORDS_PURGE_LIMIT = 5;
 
     static final String TABLE_BOOKMARKS = "bookmarks";
     static final String TABLE_HISTORY = "history";
     static final String TABLE_IMAGES = "images";
 
+    static final String VIEW_BOOKMARKS_WITH_IMAGES = "bookmarks_with_images";
+    static final String VIEW_HISTORY_WITH_IMAGES = "history_with_images";
+
     // Bookmark matches
     static final int BOOKMARKS = 100;
     static final int BOOKMARKS_ID = 101;
     static final int BOOKMARKS_FOLDER_ID = 102;
 
     // History matches
     static final int HISTORY = 200;
     static final int HISTORY_ID = 201;
@@ -101,22 +104,30 @@ public class BrowserProvider extends Con
 
     static final String DEFAULT_BOOKMARKS_SORT_ORDER = Bookmarks.IS_FOLDER
             + " DESC, " + Bookmarks.POSITION + " ASC, " + Bookmarks._ID
             + " ASC";
 
     static final String DEFAULT_HISTORY_SORT_ORDER = History.DATE_LAST_VISITED + " DESC";
 
     static final String TABLE_BOOKMARKS_JOIN_IMAGES = TABLE_BOOKMARKS + " LEFT OUTER JOIN " +
-            TABLE_IMAGES + " ON " + qualifyColumnValue(TABLE_BOOKMARKS, Bookmarks.URL) +
-            " = " + qualifyColumnValue(TABLE_IMAGES, Images.URL);
+            "(SELECT " + Images.URL + ", " + Images.FAVICON + ", " + Images.THUMBNAIL + " FROM " +
+            TABLE_IMAGES + ", " + TABLE_BOOKMARKS + " WHERE " +
+            qualifyColumn(TABLE_BOOKMARKS, Bookmarks.URL) + " = " +
+            qualifyColumn(TABLE_IMAGES, Images.URL) + ") AS bookmark_images ON " +
+            qualifyColumn(TABLE_BOOKMARKS, Bookmarks.URL) + " = " +
+            qualifyColumn("bookmark_images", Images.URL);
 
     static final String TABLE_HISTORY_JOIN_IMAGES = TABLE_HISTORY + " LEFT OUTER JOIN " +
-            TABLE_IMAGES + " ON " + qualifyColumnValue(TABLE_HISTORY, History.URL) +
-            " = " + qualifyColumnValue(TABLE_IMAGES, Images.URL);
+            "(SELECT " + Images.URL + ", " + Images.FAVICON + ", " + Images.THUMBNAIL + " FROM " +
+            TABLE_IMAGES + ", " + TABLE_HISTORY + " WHERE " +
+            qualifyColumn(TABLE_HISTORY, History.URL) + " = " +
+            qualifyColumn(TABLE_IMAGES, Images.URL) + ") AS history_images ON " +
+            qualifyColumn(TABLE_HISTORY, History.URL) + " = " +
+            qualifyColumn("history_images", Images.URL);
 
     static final UriMatcher URI_MATCHER = new UriMatcher(UriMatcher.NO_MATCH);
 
     static final HashMap<String, String> BOOKMARKS_PROJECTION_MAP = new HashMap<String, String>();
     static final HashMap<String, String> HISTORY_PROJECTION_MAP = new HashMap<String, String>();
     static final HashMap<String, String> IMAGES_PROJECTION_MAP = new HashMap<String, String>();
     static final HashMap<String, String> SCHEMA_PROJECTION_MAP = new HashMap<String, String>();
 
@@ -126,75 +137,71 @@ public class BrowserProvider extends Con
         HashMap<String, String> map;
 
         // Bookmarks
         URI_MATCHER.addURI(BrowserContract.AUTHORITY, "bookmarks", BOOKMARKS);
         URI_MATCHER.addURI(BrowserContract.AUTHORITY, "bookmarks/#", BOOKMARKS_ID);
         URI_MATCHER.addURI(BrowserContract.AUTHORITY, "bookmarks/folder/#", BOOKMARKS_FOLDER_ID);
 
         map = BOOKMARKS_PROJECTION_MAP;
-        map.put(Bookmarks._ID, qualifyColumn(TABLE_BOOKMARKS, Bookmarks._ID));
+        map.put(Bookmarks._ID, Bookmarks._ID);
         map.put(Bookmarks.TITLE, Bookmarks.TITLE);
         map.put(Bookmarks.URL, Bookmarks.URL);
         map.put(Bookmarks.FAVICON, Bookmarks.FAVICON);
         map.put(Bookmarks.THUMBNAIL, Bookmarks.THUMBNAIL);
         map.put(Bookmarks.IS_FOLDER, Bookmarks.IS_FOLDER);
         map.put(Bookmarks.PARENT, Bookmarks.PARENT);
         map.put(Bookmarks.POSITION, Bookmarks.POSITION);
         map.put(Bookmarks.TAGS, Bookmarks.TAGS);
         map.put(Bookmarks.DESCRIPTION, Bookmarks.DESCRIPTION);
         map.put(Bookmarks.KEYWORD, Bookmarks.KEYWORD);
-        map.put(Bookmarks.DATE_CREATED, qualifyColumn(TABLE_BOOKMARKS, Bookmarks.DATE_CREATED));
-        map.put(Bookmarks.DATE_MODIFIED, qualifyColumn(TABLE_BOOKMARKS, Bookmarks.DATE_MODIFIED));
-        map.put(Bookmarks.GUID, qualifyColumn(TABLE_BOOKMARKS, Bookmarks.GUID));
-        map.put(Bookmarks.IS_DELETED, qualifyColumn(TABLE_BOOKMARKS, Bookmarks.IS_DELETED));
+        map.put(Bookmarks.DATE_CREATED, Bookmarks.DATE_CREATED);
+        map.put(Bookmarks.DATE_MODIFIED, Bookmarks.DATE_MODIFIED);
+        map.put(Bookmarks.GUID, Bookmarks.GUID);
+        map.put(Bookmarks.IS_DELETED, Bookmarks.IS_DELETED);
 
         // History
         URI_MATCHER.addURI(BrowserContract.AUTHORITY, "history", HISTORY);
         URI_MATCHER.addURI(BrowserContract.AUTHORITY, "history/#", HISTORY_ID);
 
         map = HISTORY_PROJECTION_MAP;
-        map.put(History._ID, qualifyColumn(TABLE_HISTORY, History._ID));
+        map.put(History._ID, History._ID);
         map.put(History.TITLE, History.TITLE);
         map.put(History.URL, History.URL);
         map.put(History.FAVICON, History.FAVICON);
         map.put(History.THUMBNAIL, History.THUMBNAIL);
         map.put(History.VISITS, History.VISITS);
         map.put(History.DATE_LAST_VISITED, History.DATE_LAST_VISITED);
-        map.put(History.DATE_CREATED, qualifyColumn(TABLE_HISTORY, History.DATE_CREATED));
-        map.put(History.DATE_MODIFIED, qualifyColumn(TABLE_HISTORY, History.DATE_MODIFIED));
-        map.put(History.GUID, qualifyColumn(TABLE_HISTORY, History.GUID));
-        map.put(History.IS_DELETED, qualifyColumn(TABLE_HISTORY, History.IS_DELETED));
+        map.put(History.DATE_CREATED, History.DATE_CREATED);
+        map.put(History.DATE_MODIFIED, History.DATE_MODIFIED);
+        map.put(History.GUID, History.GUID);
+        map.put(History.IS_DELETED, History.IS_DELETED);
 
         // Images
         URI_MATCHER.addURI(BrowserContract.AUTHORITY, "images", IMAGES);
 
         map = IMAGES_PROJECTION_MAP;
-        map.put(Images._ID, qualifyColumn(TABLE_IMAGES, Images._ID));
+        map.put(Images._ID, Images._ID);
         map.put(Images.URL, Images.URL);
         map.put(Images.FAVICON, Images.FAVICON);
         map.put(Images.FAVICON_URL, Images.FAVICON_URL);
         map.put(Images.THUMBNAIL, Images.THUMBNAIL);
-        map.put(Images.DATE_CREATED, qualifyColumn(TABLE_IMAGES, Images.DATE_CREATED));
-        map.put(Images.DATE_MODIFIED, qualifyColumn(TABLE_IMAGES, Images.DATE_MODIFIED));
-        map.put(Images.GUID, qualifyColumn(TABLE_IMAGES, Images.GUID));
-        map.put(Images.IS_DELETED, qualifyColumn(TABLE_IMAGES, Images.IS_DELETED));
+        map.put(Images.DATE_CREATED, Images.DATE_CREATED);
+        map.put(Images.DATE_MODIFIED, Images.DATE_MODIFIED);
+        map.put(Images.GUID, Images.GUID);
+        map.put(Images.IS_DELETED, Images.IS_DELETED);
 
         // Schema
         URI_MATCHER.addURI(BrowserContract.AUTHORITY, "schema", SCHEMA);
 
         map = SCHEMA_PROJECTION_MAP;
         map.put(Schema.VERSION, Schema.VERSION);
     }
 
     static final String qualifyColumn(String table, String column) {
-        return table + "." + column + " AS " + column;
-    }
-
-    static final String qualifyColumnValue(String table, String column) {
         return table + "." + column;
     }
 
     public static String generateGuid() {
         byte[] encodedBytes = Base64.encode(generateRandomBytes(9), Base64.URL_SAFE);
         return new String(encodedBytes);
     }
 
@@ -234,16 +241,26 @@ public class BrowserProvider extends Con
 
         String[] result = new String[originalValues.length + newValues.length];
         System.arraycopy(originalValues, 0, result, 0, originalValues.length);
         System.arraycopy(newValues, 0, result, originalValues.length, newValues.length);
 
         return result;
     }
 
+    private static boolean hasImagesInProjection(String[] projection) {
+        for (int i = 0; i < projection.length; ++i) {
+            if (projection[i].equals(Images.FAVICON) ||
+                projection[i].equals(Images.THUMBNAIL))
+                return true;
+        }
+
+        return false;
+    }
+
     final class DatabaseHelper extends SQLiteOpenHelper {
         public DatabaseHelper(Context context, String databasePath) {
             super(context, databasePath, null, DATABASE_VERSION);
         }
 
         @Override
         public void onCreate(SQLiteDatabase db) {
             Log.d(LOGTAG, "Creating browser.db: " + db.getPath());
@@ -309,16 +326,26 @@ public class BrowserProvider extends Con
 
             db.execSQL("CREATE INDEX images_url_index ON " + TABLE_IMAGES + "("
                     + Images.URL + ")");
             db.execSQL("CREATE UNIQUE INDEX images_guid_index ON " + TABLE_IMAGES + "("
                     + Images.GUID + ")");
             db.execSQL("CREATE INDEX images_modified_index ON " + TABLE_IMAGES + "("
                     + Images.DATE_MODIFIED + ")");
 
+            db.execSQL("CREATE VIEW IF NOT EXISTS " + VIEW_BOOKMARKS_WITH_IMAGES + " AS " +
+                    "SELECT " + qualifyColumn(TABLE_BOOKMARKS, "*") +
+                    ", " + Images.FAVICON + ", " + Images.THUMBNAIL + " FROM " +
+                    TABLE_BOOKMARKS_JOIN_IMAGES);
+
+            db.execSQL("CREATE VIEW IF NOT EXISTS " + VIEW_HISTORY_WITH_IMAGES + " AS " +
+                    "SELECT " + qualifyColumn(TABLE_HISTORY, "*") +
+                    ", " + Images.FAVICON + ", " + Images.THUMBNAIL + " FROM " +
+                    TABLE_HISTORY_JOIN_IMAGES);
+
             // FIXME: Create default bookmarks here
         }
 
         @Override
         public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
             Log.d(LOGTAG, "Upgrading browser.db: " + db.getPath() + " from " +
                     oldVersion + " to " + newVersion);
 
@@ -423,21 +450,18 @@ public class BrowserProvider extends Con
                 .appendQueryParameter(BrowserContract.PARAM_SHOW_DELETED, "1")
                 .appendQueryParameter(BrowserContract.PARAM_IS_SYNC, "1")
                 .build();
 
         Cursor cursor = null;
 
         try {
             long now = System.currentTimeMillis();
-            String isDeletedColumn = qualifyColumnValue(tableName, SyncColumns.IS_DELETED);
-            String dateModifiedColumn = qualifyColumnValue(tableName, SyncColumns.DATE_MODIFIED);
-
-            String selection = isDeletedColumn + " = 1 AND " +
-                    dateModifiedColumn + " <= " + (now - MAX_AGE_OF_DELETED_RECORDS);
+            String selection = SyncColumns.IS_DELETED + " = 1 AND " +
+                    SyncColumns.DATE_MODIFIED + " <= " + (now - MAX_AGE_OF_DELETED_RECORDS);
 
             cursor = query(uriWithArgs,
                            new String[] { CommonColumns._ID },
                            selection,
                            null,
                            null);
 
             while (cursor.moveToNext()) {
@@ -796,86 +820,84 @@ public class BrowserProvider extends Con
         switch (match) {
             case BOOKMARKS_FOLDER_ID:
             case BOOKMARKS_ID:
             case BOOKMARKS: {
                 Log.d(LOGTAG, "Query is on bookmarks: " + uri);
 
                 if (match == BOOKMARKS_ID) {
                     Log.d(LOGTAG, "Query is BOOKMARKS_ID: " + uri);
-                    selection = concatenateWhere(selection,
-                            qualifyColumnValue(TABLE_BOOKMARKS, Bookmarks._ID) + " = ?");
+                    selection = concatenateWhere(selection, Bookmarks._ID + " = ?");
                     selectionArgs = appendSelectionArgs(selectionArgs,
                             new String[] { Long.toString(ContentUris.parseId(uri)) });
                 } else if (match == BOOKMARKS_FOLDER_ID) {
                     Log.d(LOGTAG, "Query is BOOKMARKS_FOLDER_ID: " + uri);
-                    selection = concatenateWhere(selection,
-                            qualifyColumnValue(TABLE_BOOKMARKS, Bookmarks.PARENT) + " = ?");
+                    selection = concatenateWhere(selection, Bookmarks.PARENT + " = ?");
                     selectionArgs = appendSelectionArgs(selectionArgs,
                             new String[] { Long.toString(ContentUris.parseId(uri)) });
                 }
 
-                if (!shouldShowDeleted(uri)) {
-                    String isDeletedColumn = qualifyColumnValue(TABLE_BOOKMARKS, Bookmarks.IS_DELETED);
-                    selection = concatenateWhere(isDeletedColumn + " = 0", selection);
-                }
+                if (!shouldShowDeleted(uri))
+                    selection = concatenateWhere(Bookmarks.IS_DELETED + " = 0", selection);
 
                 if (TextUtils.isEmpty(sortOrder)) {
                     Log.d(LOGTAG, "Using default sort order on query: " + uri);
                     sortOrder = DEFAULT_BOOKMARKS_SORT_ORDER;
                 }
 
                 qb.setProjectionMap(BOOKMARKS_PROJECTION_MAP);
-                qb.setTables(TABLE_BOOKMARKS_JOIN_IMAGES);
+
+                if (hasImagesInProjection(projection))
+                    qb.setTables(VIEW_BOOKMARKS_WITH_IMAGES);
+                else
+                    qb.setTables(TABLE_BOOKMARKS);
 
                 break;
             }
 
             case HISTORY_ID:
             case HISTORY: {
                 Log.d(LOGTAG, "Query is on history: " + uri);
 
                 if (match == HISTORY_ID) {
                     Log.d(LOGTAG, "Query is HISTORY_ID: " + uri);
-                    selection = concatenateWhere(selection,
-                            qualifyColumnValue(TABLE_HISTORY, History._ID) + " = ?");
+                    selection = concatenateWhere(selection, History._ID + " = ?");
                     selectionArgs = appendSelectionArgs(selectionArgs,
                             new String[] { Long.toString(ContentUris.parseId(uri)) });
                 }
 
-                if (!shouldShowDeleted(uri)) {
-                    String isDeletedColumn = qualifyColumnValue(TABLE_HISTORY, History.IS_DELETED);
-                    selection = concatenateWhere(isDeletedColumn + " = 0", selection);
-                }
+                if (!shouldShowDeleted(uri))
+                    selection = concatenateWhere(History.IS_DELETED + " = 0", selection);
 
                 if (TextUtils.isEmpty(sortOrder))
                     sortOrder = DEFAULT_HISTORY_SORT_ORDER;
 
                 qb.setProjectionMap(HISTORY_PROJECTION_MAP);
-                qb.setTables(TABLE_HISTORY_JOIN_IMAGES);
+
+                if (hasImagesInProjection(projection))
+                    qb.setTables(VIEW_HISTORY_WITH_IMAGES);
+                else
+                    qb.setTables(TABLE_HISTORY);
 
                 break;
             }
 
             case IMAGES_ID:
             case IMAGES: {
                 Log.d(LOGTAG, "Query is on images: " + uri);
 
                 if (match == IMAGES_ID) {
                     Log.d(LOGTAG, "Query is IMAGES_ID: " + uri);
-                    selection = concatenateWhere(selection,
-                            qualifyColumnValue(TABLE_IMAGES, Images._ID) + " = ?");
+                    selection = concatenateWhere(selection, Images._ID + " = ?");
                     selectionArgs = appendSelectionArgs(selectionArgs,
                             new String[] { Long.toString(ContentUris.parseId(uri)) });
                 }
 
-                if (!shouldShowDeleted(uri)) {
-                    String isDeletedColumn = qualifyColumnValue(TABLE_IMAGES, Images.IS_DELETED);
-                    selection = concatenateWhere(isDeletedColumn + " = 0", selection);
-                }
+                if (!shouldShowDeleted(uri))
+                    selection = concatenateWhere(Images.IS_DELETED + " = 0", selection);
 
                 qb.setProjectionMap(IMAGES_PROJECTION_MAP);
                 qb.setTables(TABLE_IMAGES);
 
                 break;
             }
 
             case SCHEMA: {
@@ -1168,16 +1190,16 @@ public class BrowserProvider extends Con
         }
     }
 
     int deleteUnusedImages(Uri uri) {
         Log.d(LOGTAG, "Deleting all unused images for URI: " + uri);
 
         String selection = Images.URL + " NOT IN (SELECT " + Bookmarks.URL +
                 " FROM " + TABLE_BOOKMARKS + " WHERE " + Bookmarks.URL + " IS NOT NULL AND " +
-                qualifyColumnValue(TABLE_BOOKMARKS, Bookmarks.IS_DELETED) + " = 0) AND " +
+                qualifyColumn(TABLE_BOOKMARKS, Bookmarks.IS_DELETED) + " = 0) AND " +
                 Images.URL + " NOT IN (SELECT " + History.URL + " FROM " + TABLE_HISTORY +
                 " WHERE " + History.URL + " IS NOT NULL AND " +
-                qualifyColumnValue(TABLE_HISTORY, History.IS_DELETED) + " = 0)";
+                qualifyColumn(TABLE_HISTORY, History.IS_DELETED) + " = 0)";
 
         return deleteImages(uri, selection, null);
     }
 }