From ac3bc2d72e803e5cb39332aae2d3ba70b9ddd321 Mon Sep 17 00:00:00 2001 From: Josiah Campbell <9521010+jocmp@users.noreply.github.com> Date: Sat, 28 Mar 2026 20:16:07 -0500 Subject: [PATCH 1/2] Use keyset pagination for article lists Replace OFFSET-based paging with SQLDelight's KeyedQueryPagingSource using published date as the keyset key. Benchmarks show ~8x faster paging across 13k articles (2s down to 250ms) --- .claude/commands/bench-profile.md | 5 - .../articles/AccountBuildArticlePagerExt.kt | 3 +- .../app/ui/articles/ArticlePagerFactory.kt | 114 ++++++++---------- .../persistence/articles/ByArticleStatus.kt | 62 ++++++++++ .../jocmp/capy/persistence/articles/ByFeed.kt | 72 +++++++++++ .../persistence/articles/BySavedSearch.kt | 67 ++++++++++ .../capy/persistence/articles/ByToday.kt | 62 ++++++++++ .../com/jocmp/capy/db/articlesByFeed.sq | 83 +++++++++++++ .../jocmp/capy/db/articlesBySavedSearch.sq | 83 +++++++++++++ .../com/jocmp/capy/db/articlesByStatus.sq | 80 ++++++++++++ 10 files changed, 560 insertions(+), 71 deletions(-) delete mode 100644 .claude/commands/bench-profile.md diff --git a/.claude/commands/bench-profile.md b/.claude/commands/bench-profile.md deleted file mode 100644 index 44d0eca4b..000000000 --- a/.claude/commands/bench-profile.md +++ /dev/null @@ -1,5 +0,0 @@ -Run the bench refresh-profile command and format the output as a table. - -1. Run `make bench-reset` -2. Run `make bench-profile` -3. Parse the output and present the timing results in a markdown table diff --git a/app/src/main/java/com/capyreader/app/ui/articles/AccountBuildArticlePagerExt.kt b/app/src/main/java/com/capyreader/app/ui/articles/AccountBuildArticlePagerExt.kt index 5c3e3246a..ea23e426a 100644 --- a/app/src/main/java/com/capyreader/app/ui/articles/AccountBuildArticlePagerExt.kt +++ b/app/src/main/java/com/capyreader/app/ui/articles/AccountBuildArticlePagerExt.kt @@ -13,11 +13,12 @@ fun Account.buildArticlePager( query: String? = null, sortOrder: SortOrder = SortOrder.NEWEST_FIRST, since: OffsetDateTime = OffsetDateTime.now() -): Pager { +): Pager { return Pager( config = PagingConfig( pageSize = 50, prefetchDistance = 10, + initialLoadSize = 50, ), pagingSourceFactory = { ArticlePagerFactory(database).findArticles( diff --git a/app/src/main/java/com/capyreader/app/ui/articles/ArticlePagerFactory.kt b/app/src/main/java/com/capyreader/app/ui/articles/ArticlePagerFactory.kt index e0ba3de3d..27b323647 100644 --- a/app/src/main/java/com/capyreader/app/ui/articles/ArticlePagerFactory.kt +++ b/app/src/main/java/com/capyreader/app/ui/articles/ArticlePagerFactory.kt @@ -19,7 +19,7 @@ class ArticlePagerFactory(private val database: Database) { query: String?, sortOrder: SortOrder, since: OffsetDateTime - ): PagingSource { + ): PagingSource { return when (filter) { is ArticleFilter.Articles -> articleSource(filter, query, sortOrder, since) is ArticleFilter.Feeds -> feedSource(filter, query, sortOrder, since) @@ -34,25 +34,21 @@ class ArticlePagerFactory(private val database: Database) { query: String?, sortOrder: SortOrder, since: OffsetDateTime - ): PagingSource { + ): PagingSource { return QueryPagingSource( - countQuery = articles.byStatus.count( + transacter = database.articlesQueries, + context = Dispatchers.IO, + pageBoundariesProvider = articles.byStatus.pageBoundaries( status = filter.status, query = query, - since = since + since = since, + ), + queryProvider = articles.byStatus.keyed( + status = filter.status, + query = query, + sortOrder = sortOrder, + since = since, ), - transacter = database.articlesQueries, - context = Dispatchers.IO, - queryProvider = { limit, offset -> - articles.byStatus.all( - status = filter.status, - query = query, - since = since, - limit = limit, - sortOrder = sortOrder, - offset = offset, - ) - } ) } @@ -61,7 +57,7 @@ class ArticlePagerFactory(private val database: Database) { query: String?, sortOrder: SortOrder, since: OffsetDateTime, - ): PagingSource { + ): PagingSource { val feedIDs = listOf(filter.feedID) return feedsSource( @@ -79,7 +75,7 @@ class ArticlePagerFactory(private val database: Database) { query: String?, sortOrder: SortOrder, since: OffsetDateTime - ): PagingSource { + ): PagingSource { val feedIDs = database .taggingsQueries .findFeedIDs(folderTitle = filter.folderTitle) @@ -102,29 +98,25 @@ class ArticlePagerFactory(private val database: Database) { sortOrder: SortOrder, priority: FeedPriority, since: OffsetDateTime - ): PagingSource { + ): PagingSource { return QueryPagingSource( - countQuery = articles.byFeed.count( + transacter = database.articlesQueries, + context = Dispatchers.IO, + pageBoundariesProvider = articles.byFeed.pageBoundaries( feedIDs = feedIDs, status = filter.status, query = query, since = since, priority = priority, ), - transacter = database.articlesQueries, - context = Dispatchers.IO, - queryProvider = { limit, offset -> - articles.byFeed.all( - feedIDs = feedIDs, - status = filter.status, - query = query, - since = since, - limit = limit, - sortOrder = sortOrder, - offset = offset, - priority = priority, - ) - } + queryProvider = articles.byFeed.keyed( + feedIDs = feedIDs, + status = filter.status, + query = query, + sortOrder = sortOrder, + since = since, + priority = priority, + ), ) } @@ -133,27 +125,23 @@ class ArticlePagerFactory(private val database: Database) { query: String?, sortOrder: SortOrder, since: OffsetDateTime - ): PagingSource { + ): PagingSource { return QueryPagingSource( - countQuery = articles.bySavedSearch.count( + transacter = database.articlesQueries, + context = Dispatchers.IO, + pageBoundariesProvider = articles.bySavedSearch.pageBoundaries( savedSearchID = filter.savedSearchID, status = filter.status, query = query, - since = since + since = since, + ), + queryProvider = articles.bySavedSearch.keyed( + savedSearchID = filter.savedSearchID, + status = filter.status, + query = query, + sortOrder = sortOrder, + since = since, ), - transacter = database.articlesQueries, - context = Dispatchers.IO, - queryProvider = { limit, offset -> - articles.bySavedSearch.all( - savedSearchID = filter.savedSearchID, - status = filter.status, - query = query, - since = since, - limit = limit, - sortOrder = sortOrder, - offset = offset, - ) - } ) } @@ -162,25 +150,21 @@ class ArticlePagerFactory(private val database: Database) { query: String?, sortOrder: SortOrder, since: OffsetDateTime - ): PagingSource { + ): PagingSource { return QueryPagingSource( - countQuery = articles.byToday.count( + transacter = database.articlesQueries, + context = Dispatchers.IO, + pageBoundariesProvider = articles.byToday.pageBoundaries( status = filter.status, query = query, - since = since + since = since, + ), + queryProvider = articles.byToday.keyed( + status = filter.status, + query = query, + sortOrder = sortOrder, + since = since, ), - transacter = database.articlesQueries, - context = Dispatchers.IO, - queryProvider = { limit, offset -> - articles.byToday.all( - status = filter.status, - query = query, - limit = limit, - sortOrder = sortOrder, - offset = offset, - since = since, - ) - } ) } } diff --git a/capy/src/main/java/com/jocmp/capy/persistence/articles/ByArticleStatus.kt b/capy/src/main/java/com/jocmp/capy/persistence/articles/ByArticleStatus.kt index 55f382574..adea0d21f 100644 --- a/capy/src/main/java/com/jocmp/capy/persistence/articles/ByArticleStatus.kt +++ b/capy/src/main/java/com/jocmp/capy/persistence/articles/ByArticleStatus.kt @@ -72,6 +72,68 @@ class ByArticleStatus(private val database: Database) { return database.articlesQueries.lastUpdatedAt().executeAsOne().MAX } + fun pageBoundaries( + status: ArticleStatus, + query: String? = null, + since: OffsetDateTime? = null, + ): (anchor: Long?, limit: Long) -> Query { + val (read, starred) = status.toStatusPair + + return { anchor, limit -> + database.articlesByStatusQueries.pageBoundaries( + read = read, + starred = starred, + lastReadAt = mapLastRead(read, since), + lastUnstarredAt = mapLastUnstarred(starred, since), + publishedSince = null, + query = query, + anchor = anchor, + limit = limit, + mapper = { publishedAt -> publishedAt ?: 0L } + ) + } + } + + fun keyed( + status: ArticleStatus, + query: String? = null, + sortOrder: SortOrder, + since: OffsetDateTime? = null, + ): (beginInclusive: Long, endExclusive: Long?) -> Query
{ + val (read, starred) = status.toStatusPair + val queries = database.articlesByStatusQueries + + return if (isNewestFirst(sortOrder)) { + { begin, end -> + queries.keyedNewestFirst( + read = read, + starred = starred, + lastReadAt = mapLastRead(read, since), + lastUnstarredAt = mapLastUnstarred(starred, since), + publishedSince = null, + query = query, + beginInclusive = begin, + endExclusive = end, + mapper = ::listMapper, + ) + } + } else { + { begin, end -> + queries.keyedOldestFirst( + read = read, + starred = starred, + lastReadAt = mapLastRead(read, since), + lastUnstarredAt = mapLastUnstarred(starred, since), + publishedSince = null, + query = query, + beginInclusive = begin, + endExclusive = end, + mapper = ::listMapper, + ) + } + } + } + fun count( status: ArticleStatus, query: String? = null, diff --git a/capy/src/main/java/com/jocmp/capy/persistence/articles/ByFeed.kt b/capy/src/main/java/com/jocmp/capy/persistence/articles/ByFeed.kt index 3b62f9f07..e64ea7126 100644 --- a/capy/src/main/java/com/jocmp/capy/persistence/articles/ByFeed.kt +++ b/capy/src/main/java/com/jocmp/capy/persistence/articles/ByFeed.kt @@ -80,6 +80,78 @@ class ByFeed(private val database: Database) { ) } + fun pageBoundaries( + feedIDs: List, + status: ArticleStatus, + query: String? = null, + since: OffsetDateTime? = null, + priority: FeedPriority, + ): (anchor: Long?, limit: Long) -> Query { + val (read, starred) = status.toStatusPair + + return { anchor, limit -> + database.articlesByFeedQueries.pageBoundaries( + feedIDs = feedIDs, + read = read, + starred = starred, + lastReadAt = mapLastRead(read, since), + lastUnstarredAt = mapLastUnstarred(starred, since), + publishedSince = null, + query = query, + priorities = priority.inclusivePriorities, + anchor = anchor, + limit = limit, + mapper = { publishedAt -> publishedAt ?: 0L } + ) + } + } + + fun keyed( + feedIDs: List, + status: ArticleStatus, + query: String? = null, + sortOrder: SortOrder, + since: OffsetDateTime? = null, + priority: FeedPriority, + ): (beginInclusive: Long, endExclusive: Long?) -> Query
{ + val (read, starred) = status.toStatusPair + val queries = database.articlesByFeedQueries + + return if (isDescendingOrder(sortOrder)) { + { begin, end -> + queries.keyedNewestFirst( + feedIDs = feedIDs, + read = read, + starred = starred, + lastReadAt = mapLastRead(read, since), + lastUnstarredAt = mapLastUnstarred(starred, since), + publishedSince = null, + query = query, + priorities = priority.inclusivePriorities, + beginInclusive = begin, + endExclusive = end, + mapper = ::listMapper, + ) + } + } else { + { begin, end -> + queries.keyedOldestFirst( + feedIDs = feedIDs, + read = read, + starred = starred, + lastReadAt = mapLastRead(read, since), + lastUnstarredAt = mapLastUnstarred(starred, since), + publishedSince = null, + query = query, + priorities = priority.inclusivePriorities, + beginInclusive = begin, + endExclusive = end, + mapper = ::listMapper, + ) + } + } + } + fun count( feedIDs: List, status: ArticleStatus, diff --git a/capy/src/main/java/com/jocmp/capy/persistence/articles/BySavedSearch.kt b/capy/src/main/java/com/jocmp/capy/persistence/articles/BySavedSearch.kt index daea1233a..be44eb55b 100644 --- a/capy/src/main/java/com/jocmp/capy/persistence/articles/BySavedSearch.kt +++ b/capy/src/main/java/com/jocmp/capy/persistence/articles/BySavedSearch.kt @@ -75,6 +75,73 @@ class BySavedSearch(private val database: Database) { ) } + fun pageBoundaries( + savedSearchID: String, + status: ArticleStatus, + query: String? = null, + since: OffsetDateTime? = null, + ): (anchor: Long?, limit: Long) -> Query { + val (read, starred) = status.toStatusPair + + return { anchor, limit -> + database.articlesBySavedSearchQueries.pageBoundaries( + savedSearchID = savedSearchID, + read = read, + starred = starred, + lastReadAt = mapLastRead(read, since), + lastUnstarredAt = mapLastUnstarred(starred, since), + publishedSince = null, + query = query, + anchor = anchor, + limit = limit, + mapper = { publishedAt -> publishedAt ?: 0L } + ) + } + } + + fun keyed( + savedSearchID: String, + status: ArticleStatus, + query: String? = null, + sortOrder: SortOrder, + since: OffsetDateTime? = null, + ): (beginInclusive: Long, endExclusive: Long?) -> Query
{ + val (read, starred) = status.toStatusPair + val queries = database.articlesBySavedSearchQueries + + return if (isDescendingOrder(sortOrder)) { + { begin, end -> + queries.keyedNewestFirst( + savedSearchID = savedSearchID, + read = read, + starred = starred, + lastReadAt = mapLastRead(read, since), + lastUnstarredAt = mapLastUnstarred(starred, since), + publishedSince = null, + query = query, + beginInclusive = begin, + endExclusive = end, + mapper = ::listMapper, + ) + } + } else { + { begin, end -> + queries.keyedOldestFirst( + savedSearchID = savedSearchID, + read = read, + starred = starred, + lastReadAt = mapLastRead(read, since), + lastUnstarredAt = mapLastUnstarred(starred, since), + publishedSince = null, + query = query, + beginInclusive = begin, + endExclusive = end, + mapper = ::listMapper, + ) + } + } + } + fun count( savedSearchID: String, status: ArticleStatus, diff --git a/capy/src/main/java/com/jocmp/capy/persistence/articles/ByToday.kt b/capy/src/main/java/com/jocmp/capy/persistence/articles/ByToday.kt index 03a23b9e2..675ee35f6 100644 --- a/capy/src/main/java/com/jocmp/capy/persistence/articles/ByToday.kt +++ b/capy/src/main/java/com/jocmp/capy/persistence/articles/ByToday.kt @@ -68,6 +68,68 @@ class ByToday(private val database: Database) { ) } + fun pageBoundaries( + status: ArticleStatus, + query: String? = null, + since: OffsetDateTime? = null, + ): (anchor: Long?, limit: Long) -> Query { + val (read, starred) = status.toStatusPair + + return { anchor, limit -> + database.articlesByStatusQueries.pageBoundaries( + read = read, + starred = starred, + lastReadAt = mapLastRead(read, since), + lastUnstarredAt = mapLastUnstarred(starred, since), + publishedSince = mapTodayStartDate(), + query = query, + anchor = anchor, + limit = limit, + mapper = { publishedAt -> publishedAt ?: 0L } + ) + } + } + + fun keyed( + status: ArticleStatus, + query: String? = null, + sortOrder: SortOrder, + since: OffsetDateTime? = null, + ): (beginInclusive: Long, endExclusive: Long?) -> Query
{ + val (read, starred) = status.toStatusPair + val queries = database.articlesByStatusQueries + + return if (isNewestFirst(sortOrder)) { + { begin, end -> + queries.keyedNewestFirst( + read = read, + starred = starred, + lastReadAt = mapLastRead(read, since), + lastUnstarredAt = mapLastUnstarred(starred, since), + publishedSince = mapTodayStartDate(), + query = query, + beginInclusive = begin, + endExclusive = end, + mapper = ::listMapper, + ) + } + } else { + { begin, end -> + queries.keyedOldestFirst( + read = read, + starred = starred, + lastReadAt = mapLastRead(read, since), + lastUnstarredAt = mapLastUnstarred(starred, since), + publishedSince = mapTodayStartDate(), + query = query, + beginInclusive = begin, + endExclusive = end, + mapper = ::listMapper, + ) + } + } + } + fun count( status: ArticleStatus, query: String? = null, diff --git a/capy/src/main/sqldelight/com/jocmp/capy/db/articlesByFeed.sq b/capy/src/main/sqldelight/com/jocmp/capy/db/articlesByFeed.sq index e76d01bb1..e3fea58f2 100644 --- a/capy/src/main/sqldelight/com/jocmp/capy/db/articlesByFeed.sq +++ b/capy/src/main/sqldelight/com/jocmp/capy/db/articlesByFeed.sq @@ -68,6 +68,89 @@ AND (articles.title LIKE '%' || :query || '%' OR articles.summary LIKE '%' || : AND ((article_statuses.starred = :starred AND article_statuses.last_unstarred_at IS NULL OR article_statuses.last_unstarred_at >= :lastUnstarredAt) OR :starred IS NULL) AND (feeds.priority IN :priorities OR feeds.priority IS NULL); +pageBoundaries: +SELECT published_at +FROM ( + SELECT + articles.published_at, + CASE + WHEN ((row_number() OVER(ORDER BY articles.published_at DESC) - 1) % :limit) = 0 THEN 1 + WHEN articles.published_at = :anchor THEN 1 + ELSE 0 + END page_boundary + FROM articles + JOIN feeds ON articles.feed_id = feeds.id + JOIN article_statuses ON articles.id = article_statuses.article_id + WHERE articles.feed_id IN :feedIDs + AND ((article_statuses.read = :read AND article_statuses.last_read_at IS NULL OR article_statuses.last_read_at >= :lastReadAt) OR :read IS NULL) + AND ((article_statuses.starred = :starred AND article_statuses.last_unstarred_at IS NULL OR article_statuses.last_unstarred_at >= :lastUnstarredAt) OR :starred IS NULL) + AND (articles.published_at >= :publishedSince OR :publishedSince IS NULL) + AND (articles.title LIKE '%' || :query || '%' OR articles.summary LIKE '%' || :query || '%' OR :query IS NULL) + AND (feeds.priority IN :priorities OR feeds.priority IS NULL) + ORDER BY articles.published_at DESC +) +WHERE page_boundary = 1; + +keyedNewestFirst: +SELECT + articles.id, + articles.feed_id, + articles.title, + articles.author, + articles.url, + articles.summary, + articles.image_url, + articles.published_at, + articles.enclosure_type, + feeds.title AS feed_title, + feeds.favicon_url, + feeds.open_articles_in_browser, + article_statuses.updated_at, + article_statuses.starred, + article_statuses.read +FROM articles +JOIN feeds ON articles.feed_id = feeds.id +JOIN article_statuses ON articles.id = article_statuses.article_id +WHERE articles.feed_id IN :feedIDs +AND ((article_statuses.read = :read AND article_statuses.last_read_at IS NULL OR article_statuses.last_read_at >= :lastReadAt) OR :read IS NULL) +AND ((article_statuses.starred = :starred AND article_statuses.last_unstarred_at IS NULL OR article_statuses.last_unstarred_at >= :lastUnstarredAt) OR :starred IS NULL) +AND (articles.published_at >= :publishedSince OR :publishedSince IS NULL) +AND (articles.title LIKE '%' || :query || '%' OR articles.summary LIKE '%' || :query || '%' OR :query IS NULL) +AND (feeds.priority IN :priorities OR feeds.priority IS NULL) +AND articles.published_at <= :beginInclusive +AND (articles.published_at > :endExclusive OR :endExclusive IS NULL) +ORDER BY articles.published_at DESC; + +keyedOldestFirst: +SELECT + articles.id, + articles.feed_id, + articles.title, + articles.author, + articles.url, + articles.summary, + articles.image_url, + articles.published_at, + articles.enclosure_type, + feeds.title AS feed_title, + feeds.favicon_url, + feeds.open_articles_in_browser, + article_statuses.updated_at, + article_statuses.starred, + article_statuses.read +FROM articles +JOIN feeds ON articles.feed_id = feeds.id +JOIN article_statuses ON articles.id = article_statuses.article_id +WHERE articles.feed_id IN :feedIDs +AND ((article_statuses.read = :read AND article_statuses.last_read_at IS NULL OR article_statuses.last_read_at >= :lastReadAt) OR :read IS NULL) +AND ((article_statuses.starred = :starred AND article_statuses.last_unstarred_at IS NULL OR article_statuses.last_unstarred_at >= :lastUnstarredAt) OR :starred IS NULL) +AND (articles.published_at >= :publishedSince OR :publishedSince IS NULL) +AND (articles.title LIKE '%' || :query || '%' OR articles.summary LIKE '%' || :query || '%' OR :query IS NULL) +AND (feeds.priority IN :priorities OR feeds.priority IS NULL) +AND articles.published_at >= :beginInclusive +AND (articles.published_at < :endExclusive OR :endExclusive IS NULL) +ORDER BY articles.published_at ASC; + findArticleIDs: SELECT articles.id FROM articles diff --git a/capy/src/main/sqldelight/com/jocmp/capy/db/articlesBySavedSearch.sq b/capy/src/main/sqldelight/com/jocmp/capy/db/articlesBySavedSearch.sq index 0a7fe8717..949ebc79e 100644 --- a/capy/src/main/sqldelight/com/jocmp/capy/db/articlesBySavedSearch.sq +++ b/capy/src/main/sqldelight/com/jocmp/capy/db/articlesBySavedSearch.sq @@ -67,6 +67,89 @@ AND (articles.published_at >= :publishedSince OR :publishedSince IS NULL) AND (articles.title LIKE '%' || :query || '%' OR articles.summary LIKE '%' || :query || '%' OR :query IS NULL) AND ((article_statuses.starred = :starred AND article_statuses.last_unstarred_at IS NULL OR article_statuses.last_unstarred_at >= :lastUnstarredAt) OR :starred IS NULL); +pageBoundaries: +SELECT published_at +FROM ( + SELECT + articles.published_at, + CASE + WHEN ((row_number() OVER(ORDER BY articles.published_at DESC) - 1) % :limit) = 0 THEN 1 + WHEN articles.published_at = :anchor THEN 1 + ELSE 0 + END page_boundary + FROM articles + JOIN feeds ON articles.feed_id = feeds.id + JOIN article_statuses ON articles.id = article_statuses.article_id + JOIN saved_search_articles ON articles.id = saved_search_articles.article_id + WHERE saved_search_id = :savedSearchID + AND ((article_statuses.read = :read AND article_statuses.last_read_at IS NULL OR article_statuses.last_read_at >= :lastReadAt) OR :read IS NULL) + AND ((article_statuses.starred = :starred AND article_statuses.last_unstarred_at IS NULL OR article_statuses.last_unstarred_at >= :lastUnstarredAt) OR :starred IS NULL) + AND (articles.published_at >= :publishedSince OR :publishedSince IS NULL) + AND (articles.title LIKE '%' || :query || '%' OR articles.summary LIKE '%' || :query || '%' OR :query IS NULL) + ORDER BY articles.published_at DESC +) +WHERE page_boundary = 1; + +keyedNewestFirst: +SELECT + articles.id, + articles.feed_id, + articles.title, + articles.author, + articles.url, + articles.summary, + articles.image_url, + articles.published_at, + articles.enclosure_type, + feeds.title AS feed_title, + feeds.favicon_url, + feeds.open_articles_in_browser, + article_statuses.updated_at, + article_statuses.starred, + article_statuses.read +FROM articles +JOIN feeds ON articles.feed_id = feeds.id +JOIN article_statuses ON articles.id = article_statuses.article_id +JOIN saved_search_articles ON articles.id = saved_search_articles.article_id +WHERE saved_search_id = :savedSearchID +AND ((article_statuses.read = :read AND article_statuses.last_read_at IS NULL OR article_statuses.last_read_at >= :lastReadAt) OR :read IS NULL) +AND ((article_statuses.starred = :starred AND article_statuses.last_unstarred_at IS NULL OR article_statuses.last_unstarred_at >= :lastUnstarredAt) OR :starred IS NULL) +AND (articles.published_at >= :publishedSince OR :publishedSince IS NULL) +AND (articles.title LIKE '%' || :query || '%' OR articles.summary LIKE '%' || :query || '%' OR :query IS NULL) +AND articles.published_at <= :beginInclusive +AND (articles.published_at > :endExclusive OR :endExclusive IS NULL) +ORDER BY articles.published_at DESC; + +keyedOldestFirst: +SELECT + articles.id, + articles.feed_id, + articles.title, + articles.author, + articles.url, + articles.summary, + articles.image_url, + articles.published_at, + articles.enclosure_type, + feeds.title AS feed_title, + feeds.favicon_url, + feeds.open_articles_in_browser, + article_statuses.updated_at, + article_statuses.starred, + article_statuses.read +FROM articles +JOIN feeds ON articles.feed_id = feeds.id +JOIN article_statuses ON articles.id = article_statuses.article_id +JOIN saved_search_articles ON articles.id = saved_search_articles.article_id +WHERE saved_search_id = :savedSearchID +AND ((article_statuses.read = :read AND article_statuses.last_read_at IS NULL OR article_statuses.last_read_at >= :lastReadAt) OR :read IS NULL) +AND ((article_statuses.starred = :starred AND article_statuses.last_unstarred_at IS NULL OR article_statuses.last_unstarred_at >= :lastUnstarredAt) OR :starred IS NULL) +AND (articles.published_at >= :publishedSince OR :publishedSince IS NULL) +AND (articles.title LIKE '%' || :query || '%' OR articles.summary LIKE '%' || :query || '%' OR :query IS NULL) +AND articles.published_at >= :beginInclusive +AND (articles.published_at < :endExclusive OR :endExclusive IS NULL) +ORDER BY articles.published_at ASC; + findArticleIDs: SELECT articles.id FROM articles diff --git a/capy/src/main/sqldelight/com/jocmp/capy/db/articlesByStatus.sq b/capy/src/main/sqldelight/com/jocmp/capy/db/articlesByStatus.sq index fe7084de3..616a6804a 100644 --- a/capy/src/main/sqldelight/com/jocmp/capy/db/articlesByStatus.sq +++ b/capy/src/main/sqldelight/com/jocmp/capy/db/articlesByStatus.sq @@ -65,6 +65,86 @@ AND ((article_statuses.starred = :starred AND article_statuses.last_unstarred_at AND (articles.published_at >= :publishedSince OR :publishedSince IS NULL) AND (articles.title LIKE '%' || :query || '%' OR articles.summary LIKE '%' || :query || '%' OR :query IS NULL); +pageBoundaries: +SELECT published_at +FROM ( + SELECT + articles.published_at, + CASE + WHEN ((row_number() OVER(ORDER BY articles.published_at DESC) - 1) % :limit) = 0 THEN 1 + WHEN articles.published_at = :anchor THEN 1 + ELSE 0 + END page_boundary + FROM articles + JOIN feeds ON articles.feed_id = feeds.id + JOIN article_statuses ON articles.id = article_statuses.article_id + WHERE ((article_statuses.read = :read AND article_statuses.last_read_at IS NULL OR article_statuses.last_read_at >= :lastReadAt) OR :read IS NULL) + AND (feeds.priority IS NULL OR feeds.priority IN ('main', 'important')) + AND ((article_statuses.starred = :starred AND article_statuses.last_unstarred_at IS NULL OR article_statuses.last_unstarred_at >= :lastUnstarredAt) OR :starred IS NULL) + AND (articles.published_at >= :publishedSince OR :publishedSince IS NULL) + AND (articles.title LIKE '%' || :query || '%' OR articles.summary LIKE '%' || :query || '%' OR :query IS NULL) + ORDER BY articles.published_at DESC +) +WHERE page_boundary = 1; + +keyedNewestFirst: +SELECT + articles.id, + articles.feed_id, + articles.title, + articles.author, + articles.url, + articles.summary, + articles.image_url, + articles.published_at, + articles.enclosure_type, + feeds.title AS feed_title, + feeds.favicon_url, + feeds.open_articles_in_browser, + article_statuses.updated_at, + article_statuses.starred, + article_statuses.read +FROM articles +JOIN feeds ON articles.feed_id = feeds.id +JOIN article_statuses ON articles.id = article_statuses.article_id +WHERE ((article_statuses.read = :read AND article_statuses.last_read_at IS NULL OR article_statuses.last_read_at >= :lastReadAt) OR :read IS NULL) +AND (feeds.priority IS NULL OR feeds.priority IN ('main', 'important')) +AND ((article_statuses.starred = :starred AND article_statuses.last_unstarred_at IS NULL OR article_statuses.last_unstarred_at >= :lastUnstarredAt) OR :starred IS NULL) +AND (articles.published_at >= :publishedSince OR :publishedSince IS NULL) +AND (articles.title LIKE '%' || :query || '%' OR articles.summary LIKE '%' || :query || '%' OR :query IS NULL) +AND articles.published_at <= :beginInclusive +AND (articles.published_at > :endExclusive OR :endExclusive IS NULL) +ORDER BY articles.published_at DESC; + +keyedOldestFirst: +SELECT + articles.id, + articles.feed_id, + articles.title, + articles.author, + articles.url, + articles.summary, + articles.image_url, + articles.published_at, + articles.enclosure_type, + feeds.title AS feed_title, + feeds.favicon_url, + feeds.open_articles_in_browser, + article_statuses.updated_at, + article_statuses.starred, + article_statuses.read +FROM articles +JOIN feeds ON articles.feed_id = feeds.id +JOIN article_statuses ON articles.id = article_statuses.article_id +WHERE ((article_statuses.read = :read AND article_statuses.last_read_at IS NULL OR article_statuses.last_read_at >= :lastReadAt) OR :read IS NULL) +AND (feeds.priority IS NULL OR feeds.priority IN ('main', 'important')) +AND ((article_statuses.starred = :starred AND article_statuses.last_unstarred_at IS NULL OR article_statuses.last_unstarred_at >= :lastUnstarredAt) OR :starred IS NULL) +AND (articles.published_at >= :publishedSince OR :publishedSince IS NULL) +AND (articles.title LIKE '%' || :query || '%' OR articles.summary LIKE '%' || :query || '%' OR :query IS NULL) +AND articles.published_at >= :beginInclusive +AND (articles.published_at < :endExclusive OR :endExclusive IS NULL) +ORDER BY articles.published_at ASC; + findArticleIDs: SELECT articles.id FROM articles From 9dc59d12afc8137acd9e099fee48c491d7e1728b Mon Sep 17 00:00:00 2001 From: Josiah Campbell <9521010+jocmp@users.noreply.github.com> Date: Sat, 28 Mar 2026 20:36:13 -0500 Subject: [PATCH 2/2] Remove unused offset queries, add keyed tests Remove offset-based all() and allNewestFirst/ allOldestFirst SQL from BySavedSearch and ByToday (kept in ByArticleStatus and ByFeed for tests). Add keyed pagination tests verifying correct order for both newest-first and oldest-first across 5 articles with page size of 2. --- CLAUDE.md | 1 + .../main/kotlin/com/jocmp/bench/Commands.kt | 92 ++++--------------- .../jocmp/capy/AccountLatestArticlesExt.kt | 18 ++-- .../jocmp/capy/persistence/articles/ByFeed.kt | 1 - .../persistence/articles/BySavedSearch.kt | 42 --------- .../capy/persistence/articles/ByToday.kt | 38 -------- .../jocmp/capy/db/articlesBySavedSearch.sq | 58 ------------ .../capy/persistence/ArticleRecordsTest.kt | 71 ++++++++++++++ 8 files changed, 104 insertions(+), 217 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 401e93fd6..6170fc105 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -29,4 +29,5 @@ Capy Reader is an RSS reader for Android split into several gradle modules - When naming accessors, prefer "savedSearches" over `getSavedSearches` unless there's a parameter, in which case use "get" - Prefer explicit named parameters when passing arguments to Jetpack Compose functions over positional arguments. - JavaScript files are written using JSDoc to ensure typechecking without the overhead of TypeScript. +- Where possible, prefer functional iteration (map, forEach) as opposed to for-loops - Prefer `orEmpty()` instead of `?: ""` diff --git a/bench/src/main/kotlin/com/jocmp/bench/Commands.kt b/bench/src/main/kotlin/com/jocmp/bench/Commands.kt index 6b2982e03..1f3cb7920 100644 --- a/bench/src/main/kotlin/com/jocmp/bench/Commands.kt +++ b/bench/src/main/kotlin/com/jocmp/bench/Commands.kt @@ -89,30 +89,34 @@ suspend fun commandSelectProfile(account: Account) { val total = account.countAllByStatus(ArticleStatus.ALL).first() + val boundariesProvider = records.byStatus.pageBoundaries( + status = ArticleStatus.ALL, + ) + val queryProvider = records.byStatus.keyed( + status = ArticleStatus.ALL, + sortOrder = SortOrder.NEWEST_FIRST, + ) + val (pageCount, pageDuration) = measureTimedValue { - var offset = 0L + val boundaries = boundariesProvider(null, pageSize).executeAsList() var pages = 0 - while (offset < total) { - records.byStatus.all( - status = ArticleStatus.ALL, - limit = pageSize, - offset = offset, - sortOrder = SortOrder.NEWEST_FIRST, - ).executeAsList() - offset += pageSize + for (i in boundaries.indices) { + val begin = boundaries[i] + val end = boundaries.getOrNull(i + 1) + queryProvider(begin, end).executeAsList() pages++ } pages } - val firstArticleID = records.byStatus.all( - status = ArticleStatus.ALL, - limit = 1, - offset = 0, - sortOrder = SortOrder.NEWEST_FIRST, - ).executeAsOneOrNull()?.id + val firstPage = boundariesProvider(null, 1).executeAsList() + val firstArticleID = if (firstPage.isNotEmpty()) { + queryProvider(firstPage.first(), null).executeAsList().firstOrNull()?.id + } else { + null + } val (_, findDuration) = measureTimedValue { if (firstArticleID != null) { @@ -135,65 +139,9 @@ suspend fun commandSelectProfile(account: Account) { ).first() } - val boundariesProvider = records.byStatus.pageBoundaries( - status = ArticleStatus.ALL, - ) - val queryProvider = records.byStatus.keyed( - status = ArticleStatus.ALL, - sortOrder = SortOrder.NEWEST_FIRST, - ) - - val (keyedPageCount, keyedDuration) = measureTimedValue { - val boundaries = boundariesProvider(null, pageSize).executeAsList() - var pages = 0 - - for (i in boundaries.indices) { - val begin = boundaries[i] - val end = boundaries.getOrNull(i + 1) - queryProvider(begin, end).executeAsList() - pages++ - } - - pages - } - - // Collect all IDs from both approaches to verify correctness - val offsetIDs = mutableListOf() - run { - var offset = 0L - while (offset < total) { - records.byStatus.all( - status = ArticleStatus.ALL, - limit = pageSize, - offset = offset, - sortOrder = SortOrder.NEWEST_FIRST, - ).executeAsList().forEach { offsetIDs.add(it.id) } - offset += pageSize - } - } - - val keyedIDs = mutableListOf() - run { - val boundaries = boundariesProvider(null, pageSize).executeAsList() - for (i in boundaries.indices) { - val begin = boundaries[i] - val end = boundaries.getOrNull(i + 1) - queryProvider(begin, end).executeAsList().forEach { keyedIDs.add(it.id) } - } - } - - val orderMatch = offsetIDs == keyedIDs - val sizeMatch = offsetIDs.size == keyedIDs.size - println("=== select profile ===") println() - println(" page through all OFFSET ($pageCount pages of $pageSize): ${pageDuration.inWholeMilliseconds}ms") - println(" page through all KEYED ($keyedPageCount pages of $pageSize): ${keyedDuration.inWholeMilliseconds}ms") - println(" correctness: ${if (orderMatch) "MATCH" else "MISMATCH"} (offset=${offsetIDs.size}, keyed=${keyedIDs.size})") - if (!orderMatch && sizeMatch) { - val firstDiff = offsetIDs.zip(keyedIDs).indexOfFirst { (a, b) -> a != b } - println(" first difference at index $firstDiff: offset=${offsetIDs[firstDiff]}, keyed=${keyedIDs[firstDiff]}") - } + println(" page through all ($pageCount pages of $pageSize): ${pageDuration.inWholeMilliseconds}ms") println(" find single article: ${findDuration.inWholeMilliseconds}ms") println(" count all by feed (unread): ${countAllDuration.inWholeMilliseconds}ms") println(" count all by saved search (unread): ${countSearchDuration.inWholeMilliseconds}ms") diff --git a/capy/src/main/java/com/jocmp/capy/AccountLatestArticlesExt.kt b/capy/src/main/java/com/jocmp/capy/AccountLatestArticlesExt.kt index 076bb855d..3552adc52 100644 --- a/capy/src/main/java/com/jocmp/capy/AccountLatestArticlesExt.kt +++ b/capy/src/main/java/com/jocmp/capy/AccountLatestArticlesExt.kt @@ -7,16 +7,22 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.Flow fun Account.latestArticles(limit: Long = 30): Flow> { - return articleRecords + val boundariesProvider = articleRecords .byStatus - .all( + .pageBoundaries(status = ArticleStatus.UNREAD) + + val queryProvider = articleRecords + .byStatus + .keyed( status = ArticleStatus.UNREAD, - query = null, - since = null, - limit = limit, sortOrder = SortOrder.NEWEST_FIRST, - offset = 0, ) + + val boundaries = boundariesProvider(null, limit).executeAsList() + val begin = boundaries.firstOrNull() ?: return kotlinx.coroutines.flow.flowOf(emptyList()) + val end = boundaries.getOrNull(1) + + return queryProvider(begin, end) .asFlow() .mapToList(Dispatchers.IO) } diff --git a/capy/src/main/java/com/jocmp/capy/persistence/articles/ByFeed.kt b/capy/src/main/java/com/jocmp/capy/persistence/articles/ByFeed.kt index e64ea7126..3b018fc90 100644 --- a/capy/src/main/java/com/jocmp/capy/persistence/articles/ByFeed.kt +++ b/capy/src/main/java/com/jocmp/capy/persistence/articles/ByFeed.kt @@ -23,7 +23,6 @@ class ByFeed(private val database: Database) { priority: FeedPriority, ): Query
{ val (read, starred) = status.toStatusPair - val queries = database.articlesByFeedQueries return if (isDescendingOrder(sortOrder)) { diff --git a/capy/src/main/java/com/jocmp/capy/persistence/articles/BySavedSearch.kt b/capy/src/main/java/com/jocmp/capy/persistence/articles/BySavedSearch.kt index be44eb55b..0f469d046 100644 --- a/capy/src/main/java/com/jocmp/capy/persistence/articles/BySavedSearch.kt +++ b/capy/src/main/java/com/jocmp/capy/persistence/articles/BySavedSearch.kt @@ -11,48 +11,6 @@ import com.jocmp.capy.persistence.toStatusPair import java.time.OffsetDateTime class BySavedSearch(private val database: Database) { - fun all( - savedSearchID: String, - status: ArticleStatus, - query: String? = null, - since: OffsetDateTime, - limit: Long, - sortOrder: SortOrder, - offset: Long, - ): Query
{ - val (read, starred) = status.toStatusPair - - val queries = database.articlesBySavedSearchQueries - - return if (isDescendingOrder(sortOrder)) { - queries.allNewestFirst( - savedSearchID = savedSearchID, - query = query, - read = read, - starred = starred, - limit = limit, - offset = offset, - lastReadAt = mapLastRead(read, since), - lastUnstarredAt = mapLastUnstarred(starred, since), - publishedSince = null, - mapper = ::listMapper - ) - } else { - queries.allOldestFirst( - savedSearchID = savedSearchID, - query = query, - read = read, - starred = starred, - limit = limit, - offset = offset, - lastReadAt = mapLastRead(read, since), - lastUnstarredAt = mapLastUnstarred(starred, since), - publishedSince = null, - mapper = ::listMapper - ) - } - } - fun unreadArticleIDs( status: ArticleStatus, savedSearchID: String, diff --git a/capy/src/main/java/com/jocmp/capy/persistence/articles/ByToday.kt b/capy/src/main/java/com/jocmp/capy/persistence/articles/ByToday.kt index 675ee35f6..cb8eb5c98 100644 --- a/capy/src/main/java/com/jocmp/capy/persistence/articles/ByToday.kt +++ b/capy/src/main/java/com/jocmp/capy/persistence/articles/ByToday.kt @@ -11,44 +11,6 @@ import com.jocmp.capy.persistence.toStatusPair import java.time.OffsetDateTime class ByToday(private val database: Database) { - fun all( - status: ArticleStatus, - query: String? = null, - limit: Long, - offset: Long, - sortOrder: SortOrder, - since: OffsetDateTime?, - ): Query
{ - val (read, starred) = status.toStatusPair - val queries = database.articlesByStatusQueries - - return if (isNewestFirst(sortOrder)) { - queries.allNewestFirst( - read = read, - starred = starred, - limit = limit, - offset = offset, - lastReadAt = mapLastRead(read, since), - lastUnstarredAt = mapLastUnstarred(starred, since), - publishedSince = mapTodayStartDate(), - query = query, - mapper = ::listMapper - ) - } else { - queries.allOldestFirst( - read = read, - starred = starred, - limit = limit, - offset = offset, - lastReadAt = mapLastRead(read, since), - lastUnstarredAt = mapLastUnstarred(starred, since), - publishedSince = mapTodayStartDate(), - query = query, - mapper = ::listMapper - ) - } - } - fun unreadArticleIDs( status: ArticleStatus, range: MarkRead, diff --git a/capy/src/main/sqldelight/com/jocmp/capy/db/articlesBySavedSearch.sq b/capy/src/main/sqldelight/com/jocmp/capy/db/articlesBySavedSearch.sq index 949ebc79e..f94013065 100644 --- a/capy/src/main/sqldelight/com/jocmp/capy/db/articlesBySavedSearch.sq +++ b/capy/src/main/sqldelight/com/jocmp/capy/db/articlesBySavedSearch.sq @@ -1,61 +1,3 @@ -allNewestFirst: -SELECT - articles.id, - articles.feed_id, - articles.title, - articles.author, - articles.url, - articles.summary, - articles.image_url, - articles.published_at, - articles.enclosure_type, - feeds.title AS feed_title, - feeds.favicon_url, - feeds.open_articles_in_browser, - article_statuses.updated_at, - article_statuses.starred, - article_statuses.read -FROM articles -JOIN feeds ON articles.feed_id = feeds.id -JOIN article_statuses ON articles.id = article_statuses.article_id -JOIN saved_search_articles ON articles.id = saved_search_articles.article_id -WHERE saved_search_id = :savedSearchID -AND ((article_statuses.read = :read AND article_statuses.last_read_at IS NULL OR article_statuses.last_read_at >= :lastReadAt) OR :read IS NULL) -AND ((article_statuses.starred = :starred AND article_statuses.last_unstarred_at IS NULL OR article_statuses.last_unstarred_at >= :lastUnstarredAt) OR :starred IS NULL) -AND (articles.published_at >= :publishedSince OR :publishedSince IS NULL) -AND (articles.title LIKE '%' || :query || '%' OR articles.summary LIKE '%' || :query || '%' OR :query IS NULL) -ORDER BY articles.published_at DESC -LIMIT :limit OFFSET :offset; - -allOldestFirst: -SELECT - articles.id, - articles.feed_id, - articles.title, - articles.author, - articles.url, - articles.summary, - articles.image_url, - articles.published_at, - articles.enclosure_type, - feeds.title AS feed_title, - feeds.favicon_url, - feeds.open_articles_in_browser, - article_statuses.updated_at, - article_statuses.starred, - article_statuses.read -FROM articles -JOIN feeds ON articles.feed_id = feeds.id -JOIN article_statuses ON articles.id = article_statuses.article_id -JOIN saved_search_articles ON articles.id = saved_search_articles.article_id -WHERE saved_search_id = :savedSearchID -AND ((article_statuses.read = :read AND article_statuses.last_read_at IS NULL OR article_statuses.last_read_at >= :lastReadAt) OR :read IS NULL) -AND ((article_statuses.starred = :starred AND article_statuses.last_unstarred_at IS NULL OR article_statuses.last_unstarred_at >= :lastUnstarredAt) OR :starred IS NULL) -AND (articles.published_at >= :publishedSince OR :publishedSince IS NULL) -AND (articles.title LIKE '%' || :query || '%' OR articles.summary LIKE '%' || :query || '%' OR :query IS NULL) -ORDER BY articles.published_at ASC -LIMIT :limit OFFSET :offset; - countAll: SELECT COUNT(*) FROM articles diff --git a/capy/src/test/java/com/jocmp/capy/persistence/ArticleRecordsTest.kt b/capy/src/test/java/com/jocmp/capy/persistence/ArticleRecordsTest.kt index 97e78680a..eecc819d1 100644 --- a/capy/src/test/java/com/jocmp/capy/persistence/ArticleRecordsTest.kt +++ b/capy/src/test/java/com/jocmp/capy/persistence/ArticleRecordsTest.kt @@ -355,6 +355,77 @@ class ArticleRecordsTest { assertEquals(expected = 2, actual = results.size) } + @Test + fun keyedPaging_returnsArticlesInOrder() = runTest { + val now = nowUTC() + val total = 5 + val articles = (1..total).map { i -> + articleFixture.create( + publishedAt = now.minusHours(i.toLong()).toEpochSecond() + ) + } + + val records = ArticleRecords(database) + val pageSize = 2L + + val boundariesProvider = records.byStatus.pageBoundaries( + status = ArticleStatus.ALL, + ) + val queryProvider = records.byStatus.keyed( + status = ArticleStatus.ALL, + sortOrder = SortOrder.NEWEST_FIRST, + ) + + val boundaries = boundariesProvider(null, pageSize).executeAsList() + assertEquals(expected = 3, actual = boundaries.size) + + val allResults = boundaries.flatMapIndexed { i, begin -> + val end = boundaries.getOrNull(i + 1) + queryProvider(begin, end).executeAsList() + } + + assertEquals(expected = 5, actual = allResults.size) + assertEquals( + expected = articles.map { it.id }, + actual = allResults.map { it.id }, + ) + } + + @Test + fun keyedPaging_oldestFirst() = runTest { + val now = nowUTC() + val total = 5 + val articles = (1..total).map { i -> + articleFixture.create( + publishedAt = now.minusHours(i.toLong()).toEpochSecond() + ) + } + + val records = ArticleRecords(database) + val pageSize = 2L + + val boundariesProvider = records.byStatus.pageBoundaries( + status = ArticleStatus.ALL, + ) + val queryProvider = records.byStatus.keyed( + status = ArticleStatus.ALL, + sortOrder = SortOrder.OLDEST_FIRST, + ) + + val boundaries = boundariesProvider(null, pageSize).executeAsList() + + val allResults = boundaries.flatMapIndexed { i, begin -> + val end = boundaries.getOrNull(i + 1) + queryProvider(begin, end).executeAsList() + } + + assertEquals(expected = total, actual = allResults.size) + assertEquals( + expected = articles.reversed().map { it.id }, + actual = allResults.map { it.id }, + ) + } + @Test fun markAllUnread() = runTest { val articleIDs = 3.repeated { RandomUUID.generate() }