crash in android.database.sqlite.SQLiteException: near ")": syntax error (code 1): , while compiling: INSERT INTO… when suggested sites list is empty

RESOLVED FIXED in Firefox 47

Status

()

Firefox for Android
Data Providers
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: kbrosnan, Assigned: ahunt)

Tracking

({crash})

unspecified
Firefox 47
Unspecified
Android
crash
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

(crash signature)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Reporter)

Description

2 years ago
This bug was filed from the Socorro interface and is 
report bp-fcc19f54-f594-4cf1-b6fe-ebe0c2160301.
=============================================================

New crash today. Fallout from bug 760956.

android.database.sqlite.SQLiteException: near ")": syntax error (code 1): , while compiling: INSERT INTO topsites SELECT * FROM (SELECT _id, url, title, NULL AS history_id, 3 as type FROM (  ) WHERE url NOT IN (SELECT url FROM topsites) AND url NOT IN (SELECT url FROM bookmarks WHERE parent == ?) LIMIT MAX(0, (? - (SELECT COUNT(*) FROM topsites) - (SELECT COUNT(*) FROM bookmarks WHERE parent == ?)))  )
#################################################################
Error Code : 1 (SQLITE_ERROR)
Caused By : SQL(query) error or missing database.
	(near ")": syntax error (code 1): , while compiling: INSERT INTO topsites SELECT * FROM (SELECT _id, url, title, NULL AS history_id, 3 as type FROM (  ) WHERE url NOT IN (SELECT url FROM topsites) AND url NOT IN (SELECT url FROM bookmarks WHERE parent == ?) LIMIT MAX(0, (? - (SELECT COUNT(*) FROM topsites) - (SELECT COUNT(*) FROM bookmarks WHERE parent == ?)))  ))
#################################################################
	at android.database.sqlite.SQLiteConnection.nativePrepareStatement(Native Method)
	at android.database.sqlite.SQLiteConnection.acquirePreparedStatement(SQLiteConnection.java:1093)
	at android.database.sqlite.SQLiteConnection.prepare(SQLiteConnection.java:670)
	at android.database.sqlite.SQLiteSession.prepare(SQLiteSession.java:588)
	at android.database.sqlite.SQLiteProgram.<init>(SQLiteProgram.java:59)
	at android.database.sqlite.SQLiteStatement.<init>(SQLiteStatement.java:31)
	at android.database.sqlite.SQLiteDatabase.executeSql(SQLiteDatabase.java:1812)
	at android.database.sqlite.SQLiteDatabase.execSQL(SQLiteDatabase.java:1793)
	at org.mozilla.gecko.db.BrowserProvider.getTopSites(BrowserProvider.java:819)
	at org.mozilla.gecko.db.BrowserProvider.query(BrowserProvider.java:894)
	at android.content.ContentProvider.query(ContentProvider.java:1007)
	at android.content.ContentProvider$Transport.query(ContentProvider.java:218)
	at android.content.ContentResolver.query(ContentResolver.java:489)
	at android.content.ContentResolver.query(ContentResolver.java:433)
	at org.mozilla.gecko.db.LocalBrowserDB.getTopSites$5514332a(LocalBrowserDB.java:1667)
	at org.mozilla.gecko.home.TopSitesPanel$TopSitesLoader.loadCursor(TopSitesPanel.java:505)
	at org.mozilla.gecko.home.SimpleCursorLoader.loadInBackground(SimpleCursorLoader.java:44)
	at org.mozilla.gecko.home.SimpleCursorLoader.loadInBackground(SimpleCursorLoader.java:26)
	at android.support.v4.content.AsyncTaskLoader$LoadTask.doInBackground$532ebdd5(Unknown Source)
	at android.support.v4.content.AsyncTaskLoader$LoadTask.doInBackground$42af7916(Unknown Source)
	at android.support.v4.content.ModernAsyncTask$2.call(Unknown Source)
	at java.util.concurrent.FutureTask.run(FutureTask.java:237)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1112)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:587)
	at java.lang.Thread.run(Thread.java:818)
(Assignee)

Comment 1

2 years ago
This seems to be because we're not being provided any suggested sites, resulting in an empty string from suggestedSitesBuilder.toString().
Assignee: nobody → ahunt
(Assignee)

Comment 2

2 years ago
Created attachment 8725409 [details]
MozReview Request: Bug 1252610 - Don't insert suggested sites into topsites table unless they actually exist r=rnewman

Review commit: https://reviewboard.mozilla.org/r/37473/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37473/
Attachment #8725409 - Flags: review?(rnewman)
Status: NEW → ASSIGNED
Component: General → Data Providers
Summary: crash in android.database.sqlite.SQLiteException: ################################################################# → crash in android.database.sqlite.SQLiteException: near ")": syntax error (code 1): , while compiling: INSERT INTO… when suggested sites list is empty
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1252613
Attachment #8725409 - Flags: review?(rnewman) → review+
Comment on attachment 8725409 [details]
MozReview Request: Bug 1252610 - Don't insert suggested sites into topsites table unless they actually exist r=rnewman

https://reviewboard.mozilla.org/r/37473/#review34005

::: mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java:821
(Diff revision 1)
> +            if (suggestedSitesBuilder.length() > 0) {

Rename `firstClause` to `hasProcessedAnySuggestedSites` and use it here.

File a follow-up to run more efficient SQL altogether if there are no suggested sites.
(Assignee)

Comment 5

2 years ago
Comment on attachment 8725409 [details]
MozReview Request: Bug 1252610 - Don't insert suggested sites into topsites table unless they actually exist r=rnewman

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37473/diff/1-2/
(Assignee)

Comment 6

2 years ago
Created attachment 8725434 [details]
MozReview Request: Bug 1252610 - Extract SuggestedSites interface to simplify testing r=liuche

Review commit: https://reviewboard.mozilla.org/r/37485/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37485/
Attachment #8725434 - Flags: review?(liuche)
(Assignee)

Comment 7

2 years ago
Created attachment 8725435 [details]
MozReview Request: Bug 1252610 - Expand TestTopSites to cover the empty SuggestedSites case r=liuche

There is some duplication of the column names, however we're aiming to remove
all the MatrixCursor stuff in Bug 1249018 anyways.

Review commit: https://reviewboard.mozilla.org/r/37487/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37487/
Attachment #8725435 - Flags: review?(liuche)
(Assignee)

Comment 8

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/9aefc0d2d3b4e787b94fb88469d04f100ef32754
Bug 1252610 - Don't insert suggested sites into topsites table unless they actually exist r=rnewman
Comment on attachment 8725434 [details]
MozReview Request: Bug 1252610 - Extract SuggestedSites interface to simplify testing r=liuche

https://reviewboard.mozilla.org/r/37485/#review34057

This seems to be fine, since we're just moving code. I'd like to see a green try build though.

::: mobile/android/base/java/org/mozilla/gecko/db/LocalSuggestedSites.java:6
(Diff revision 1)
> +package org.mozilla.gecko.db;

You'll need to add this file to moz.build

::: mobile/android/base/java/org/mozilla/gecko/db/SuggestedSites.java:42
(Diff revision 1)
> +    Cursor get(int limit);

Do these need access modifiers? (public, etc) Generally we avoid package-level implicit access.
Attachment #8725434 - Flags: review?(liuche)
Comment on attachment 8725435 [details]
MozReview Request: Bug 1252610 - Expand TestTopSites to cover the empty SuggestedSites case r=liuche

https://reviewboard.mozilla.org/r/37487/#review34067

Make sure this also runs on try.

::: mobile/android/base/java/org/mozilla/gecko/db/StubSuggestedSites.java:1
(Diff revision 1)
> +/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-

Make sure to add this to the test moz.build.
http://mxr.mozilla.org/mozilla-central/source/mobile/android/tests/background/junit3/background_junit3_sources.mozbuild#24

::: mobile/android/tests/background/junit3/src/org/mozilla/gecko/background/db/TestTopSites.java:56
(Diff revision 1)
> +        verifyGetTopSites(mContext, mEmptySuggestedSites);

It looks like the mEmptySuggestedSites that gets passed in gets overwritten by verifGetTopSites anyways.

I don't think you need the member variables anymore and can just create the suggestedSites instance that you want to test within each test method and pass that into verifyGetTopSites.
Attachment #8725435 - Flags: review?(liuche) → review+

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9aefc0d2d3b4
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
(Reporter)

Updated

2 years ago
Duplicate of this bug: 1252867
You need to log in before you can comment on or make changes to this bug.