feat: improve performance for sqlite queries (#425)

* [sqlite] Added has_content column and relevant migration

* Fix typo in docstring

* [sqlite] Fetch content for bookmarks separately

* [sqlite] Store has_content alongside with bookmark

* [sqlite] Rename variable to distinguish it from main query

* [sqlite] Use by-reference instead of no-op copy

* [sqlite] Reduce queries count from ≈30 to 1 to fetch tags

* Lint fixes

* Make minor logging improvements

* Wrap entire migration in transaction block

* Added «down» migration

* Drop workaround for old SQLite versions

internal/database/database.go

@@ -57,7 +57,7 @@ type DB interface {
 	// DeleteBookmarks removes all record with matching ids from database.
 	DeleteBookmarks(ids ...int) error
 
-	// GetBookmark fetchs bookmark based on its ID or URL.
+	// GetBookmark fetches bookmark based on its ID or URL.
 	GetBookmark(id int, url string) (model.Bookmark, bool)
 
 	// SaveAccount saves new account in database

internal/database/migrations/sqlite/0002_denormalize_content.down.sql

@@ -0,0 +1,3 @@
+BEGIN TRANSACTION;
+ALTER TABLE bookmark DROP COLUMN has_content;
+COMMIT;

internal/database/migrations/sqlite/0002_denormalize_content.up.sql

@@ -0,0 +1,8 @@
+BEGIN TRANSACTION;
+ALTER TABLE bookmark
+    ADD has_content BOOLEAN DEFAULT FALSE NOT NULL;
+
+UPDATE bookmark
+SET has_content = bc.has_content FROM (SELECT docid, content <> '' AS has_content FROM bookmark_content) AS bc
+WHERE bookmark.id = bc.docid;
+COMMIT;

internal/database/sqlite.go

@@ -21,6 +21,17 @@ type SQLiteDatabase struct {
 	sqlx.DB
 }
 
+type bookmarkContent struct {
+	ID      int    `db:"docid"`
+	Content string `db:"content"`
+	HTML    string `db:"html"`
+}
+
+type tagContent struct {
+	ID int `db:"bookmark_id"`
+	model.Tag
+}
+
 // OpenSQLiteDatabase creates and open connection to new SQLite3 database.
 func OpenSQLiteDatabase(databasePath string) (sqliteDB *SQLiteDatabase, err error) {
 	// Open database
@@ -72,11 +83,11 @@ func (db *SQLiteDatabase) SaveBookmarks(bookmarks ...model.Bookmark) (result []m
 
 	// Prepare statement
 	stmtInsertBook, _ := tx.Preparex(`INSERT INTO bookmark
-		(id, url, title, excerpt, author, public, modified)
+		(id, url, title, excerpt, author, public, modified, has_content)
-		VALUES(?, ?, ?, ?, ?, ?, ?)
+		VALUES(?, ?, ?, ?, ?, ?, ?, ?)
 		ON CONFLICT(id) DO UPDATE SET
 		url = ?, title = ?,	excerpt = ?, author = ?,
-		public = ?, modified = ?`)
+		public = ?, modified = ?, has_content = ?`)
 
 	stmtInsertBookContent, _ := tx.Preparex(`INSERT OR REPLACE INTO bookmark_content
 		(docid, title, content, html)
@@ -119,12 +130,13 @@ func (db *SQLiteDatabase) SaveBookmarks(bookmarks ...model.Bookmark) (result []m
 		book.Modified = modifiedTime
 
 		// Save bookmark
+		hasContent := book.Content != ""
 		stmtInsertBook.MustExec(book.ID,
-			book.URL, book.Title, book.Excerpt, book.Author, book.Public, book.Modified,
+			book.URL, book.Title, book.Excerpt, book.Author, book.Public, book.Modified, hasContent,
-			book.URL, book.Title, book.Excerpt, book.Author, book.Public, book.Modified)
+			book.URL, book.Title, book.Excerpt, book.Author, book.Public, book.Modified, hasContent)
 
 		// Try to update it first to check for existence, we can't do an UPSERT here because
-		// bookmant_content is a virtual table
+		// bookmark_content is a virtual table
 		res := stmtUpdateBookContent.MustExec(book.Title, book.Content, book.HTML, book.ID)
 		rows, _ := res.RowsAffected()
 		if rows == 0 {
@@ -180,23 +192,16 @@ func (db *SQLiteDatabase) SaveBookmarks(bookmarks ...model.Bookmark) (result []m
 // GetBookmarks fetch list of bookmarks based on submitted options.
 func (db *SQLiteDatabase) GetBookmarks(opts GetBookmarksOptions) ([]model.Bookmark, error) {
 	// Create initial query
-	columns := []string{
+	query := `SELECT 
-		`b.id`,
+		b.id,
-		`b.url`,
+		b.url,
-		`b.title`,
+		b.title,
-		`b.excerpt`,
+		b.excerpt,
-		`b.author`,
+		b.author,
-		`b.public`,
+		b.public,
-		`b.modified`,
+		b.modified,
-		`bc.content <> "" has_content`}
+		b.has_content
-
-	if opts.WithContent {
-		columns = append(columns, `bc.content`, `bc.html`)
-	}
-
-	query := `SELECT ` + strings.Join(columns, ",") + `
 		FROM bookmark b
-		LEFT JOIN bookmark_content bc ON bc.docid = b.id
 		WHERE 1`
 
 	// Add where clause
@@ -302,25 +307,75 @@ func (db *SQLiteDatabase) GetBookmarks(opts GetBookmarksOptions) ([]model.Bookma
 		return nil, fmt.Errorf("failed to fetch data: %v", err)
 	}
 
-	// Fetch tags for each bookmarks
+	// store bookmark IDs for further enrichment
-	stmtGetTags, err := db.Preparex(`SELECT t.id, t.name
+	var bookmarkIds = make([]int, 0, len(bookmarks))
+	for _, book := range bookmarks {
+		bookmarkIds = append(bookmarkIds, book.ID)
+	}
+
+	// If content needed, fetch it separately
+	// It's faster than join with virtual table
+	if opts.WithContent {
+		contents := make([]bookmarkContent, 0, len(bookmarks))
+		contentMap := make(map[int]bookmarkContent, len(bookmarks))
+
+		contentQuery, args, err := sqlx.In(`SELECT docid, content, html FROM bookmark_content WHERE docid IN (?)`, bookmarkIds)
+		contentQuery = db.Rebind(contentQuery)
+		if err != nil {
+			return nil, fmt.Errorf("failed to expand bookmark_content query: %v", err)
+		}
+
+		err = db.Select(&contents, contentQuery, args...)
+		if err != nil && err != sql.ErrNoRows {
+			return nil, fmt.Errorf("failed to fetch content for bookmarks (%v): %v", bookmarkIds, err)
+		}
+		for _, content := range contents {
+			contentMap[content.ID] = content
+		}
+		for i := range bookmarks[:] {
+			book := &bookmarks[i]
+			if bookmarkContent, found := contentMap[book.ID]; found {
+				book.Content = bookmarkContent.Content
+				book.HTML = bookmarkContent.HTML
+			} else {
+				log.Printf("not found content for bookmark %d, but it should be; check DB consistency", book.ID)
+			}
+		}
+
+	}
+
+	// Fetch tags for each bookmark
+	tags := make([]tagContent, 0, len(bookmarks))
+	tagsMap := make(map[int][]model.Tag, len(bookmarks))
+
+	tagsQuery, tagArgs, err := sqlx.In(`SELECT bt.bookmark_id, t.id, t.name
 		FROM bookmark_tag bt
 		LEFT JOIN tag t ON bt.tag_id = t.id
-		WHERE bt.bookmark_id = ?
+		WHERE bt.bookmark_id IN (?)
-		ORDER BY t.name`)
+		ORDER BY t.name`, bookmarkIds)
+	tagsQuery = db.Rebind(tagsQuery)
 	if err != nil {
-		return nil, fmt.Errorf("failed to prepare tag query: %v", err)
+		return nil, fmt.Errorf("failed to expand bookmark_tag query: %v", err)
 	}
-	defer stmtGetTags.Close()
 
-	for i, book := range bookmarks {
+	err = db.Select(&tags, tagsQuery, tagArgs...)
-		book.Tags = []model.Tag{}
-		err = stmtGetTags.Select(&book.Tags, book.ID)
 	if err != nil && err != sql.ErrNoRows {
-			return nil, fmt.Errorf("failed to fetch tags: %v", err)
+		return nil, fmt.Errorf("failed to fetch tags for bookmarks (%v): %v", bookmarkIds, err)
+	}
+	for _, fetchedTag := range tags {
+		if tags, found := tagsMap[fetchedTag.ID]; found {
+			tagsMap[fetchedTag.ID] = append(tags, fetchedTag.Tag)
+		} else {
+			tagsMap[fetchedTag.ID] = []model.Tag{fetchedTag.Tag}
+		}
+	}
+	for i := range bookmarks[:] {
+		book := &bookmarks[i]
+		if tags, found := tagsMap[book.ID]; found {
+			book.Tags = tags
+		} else {
+			book.Tags = []model.Tag{}
 		}
-
-		bookmarks[i] = book
 	}
 
 	return bookmarks, nil
@@ -476,13 +531,13 @@ func (db *SQLiteDatabase) DeleteBookmarks(ids ...int) (err error) {
 	return err
 }
 
-// GetBookmark fetchs bookmark based on its ID or URL.
+// GetBookmark fetches bookmark based on its ID or URL.
 // Returns the bookmark and boolean whether it's exist or not.
 func (db *SQLiteDatabase) GetBookmark(id int, url string) (model.Bookmark, bool) {
 	args := []interface{}{id}
 	query := `SELECT
 		b.id, b.url, b.title, b.excerpt, b.author, b.public, b.modified,
-		bc.content, bc.html, bc.content <> "" has_content
+		bc.content, bc.html, b.has_content
 		FROM bookmark b
 		LEFT JOIN bookmark_content bc ON bc.docid = b.id
 		WHERE b.id = ?`

internal/webserver/server.go

@@ -83,7 +83,7 @@ func (r *loggingResponseWriter) WriteHeader(statusCode int) {
 	r.responseData.status = statusCode       // capture status code
 }
 
-// Log through logrus, 200 will log as info, anything else as an error.
+// Logger Log through logrus, 200 will log as info, anything else as an error.
 func Logger(r *http.Request, statusCode int, size int) {
 	if statusCode == http.StatusOK {
 		logrus.WithFields(logrus.Fields{