1431438 - SQL error or missing database no such column: f.type

Status: VERIFIED FIXED

(Firefox for iOS :: General, defect, P1)

Description

[Catalin Suciu [:csuciu]]

1. Make a build from commit c7b9aef5d16024a9b2cf4bc3fbfbb23022eb61aa and install it
2. Make a build from commit 878238d182900690aa56f98254b5850a9c5e983d (latest) and install it over the previous one
3. Visit a site and bookmark it.
4. Check the History and Bookmarks panels. 

Result: The History and Bookmarks panels are empty 

Also, pinning to Top Sites does not work.

Visit a site:
[Debug] [SentryIntegration.swift:161] printMessage(message:extra:) > Sentry: SQL error , errorDescription: Error code: 1, Error Domain=org.mozilla Code=1 "SQL error or missing database no such column: f.type" UserInfo={NSLocalizedDescription=SQL error or missing database no such column: f.type} for SQL SELECT * FROM (SELECT * FROM (SELECT historyID, url, title, guid, domain_id, domain, localVisitDate, remoteVisitDate, localVisitCount, remoteVisitCount, iconID, iconURL, iconDate, iconWidth, frecencies, is_bookmarked FROM ( SELECT historyID, url, title, guid, domain_id, domain, max(localVisitDate) AS localVisitDate, max(remoteVisitDate) AS remoteVisitDate, sum(localVisitCount) AS localVisitCount, sum(remoteVisitCount) AS remoteVisitCount, max(frecency), sum(frecency) AS frecencies, 0 AS is_bookm

Pin a site:
[Debug] [SentryIntegration.swift:161] printMessage(message:extra:) > Sentry: SQL error , errorDescription: Error code: 1, Error Domain=org.mozilla Code=1 "SQL error or missing database no such column: favicons.type" UserInfo={NSLocalizedDescription=SQL error or missing database no such column: favicons.type} for SQL SELECT history.id AS historyID, history.url AS url, title, guid, iconID, iconURL, iconDate, iconWidth FROM view_favicons_widest, history WHERE history.id = siteID AND history.url IN .....
2018-01-18 16:58:59.777 [Warning] [PhotonActionSheetProtocol.swift:176] getTabActions(tab:buttonView:presentShareMenu:findInPage:presentableVC:isBookmarked:success:) > Could not get site for ....

Comment 1

[Justin D'Arcangelo [:justindarc]]

After investigating this, it seems that attempting to remove the `type` column from the `favicons` table in Bug 1406165 was a huge mistake. This is a rabbit hole that I don't think we want to go down. SQLite provides no direct way to remove columns from tables. The only way to achieve this is to rename the table to `favicons_old`, create a new `favicons` table without the column, insert all the data from `favicons_old` to `favicons` and finally drop `favicons_old`. Unfortunately, it seems that we have several tables that reference `favicons` with a foreign key constraint. Because of this, the migration used in Bug 1406165 actually re-assigns all those existing foreign key constraints to `favicons_old`! This is definitely not what we want to do. To correct this would require us to essentially use this same rename/create/insert/drop table for EVERY TABLE that references the `favicons` table. The rabbit hole comes in when you realize that you'll need to do the same for EVERY TABLE that references all those tables... and on... and on...

So, regretfully, I'll be attaching a patch to this bug to essentially rollback the part of the commit in Bug 1406165 that added the schema migration that attempted to remove this column. This means that migrating profiles between c7b9aef5d16024a9b2cf4bc3fbfbb23022eb61aa and 878238d182900690aa56f98254b5850a9c5e983d will remain broken. Fortunately, we never shipped master in this state to beta or release channels, so this should only be an issue for internal testers.

Comment 2

[Richard Newman [:rnewman]]

> Because of this, the migration used in Bug 1406165 actually re-assigns all those existing foreign key constraints to `favicons_old`!

Only if foreign key support is enabled. You can turn it off with `pragma foreign_keys off`, do the migration, then turn it back on. The foreign key constraints will then be pointing to your new table.

Comment 3

[Justin D'Arcangelo [:justindarc]]

Attachment: GitHub Pull Request

Comment 4

[Justin D'Arcangelo [:justindarc]]

Landed on master:

https://github.com/mozilla-mobile/firefox-ios/commit/21bb6e3166c5af9724e53d39a3e5718daa413644

Comment 5

[Justin D'Arcangelo [:justindarc]]

Note to Catalin, as mentioned in Comment 1, migrations between c7b9aef5d16024a9b2cf4bc3fbfbb23022eb61aa and 21bb6e3166c5af9724e53d39a3e5718daa413644 will be broken because we've rolled back this schema change. However, going from c7b9aef5d16024a9b2cf4bc3fbfbb23022eb61aa directly to 21bb6e3166c5af9724e53d39a3e5718daa413644 or later should now be fine.

Comment 6

[Simon Basca [:SimonB]]

Following the STR, upgrading from commit c7b9aef5d16024a9b2cf4bc3fbfbb23022eb61aa to f2b6a8d80fa7541c492340307599575c1bc5a36d(latest) is now working correctly. Bookmarks, History and Pinned Sites are correctly saved in their panels.