mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-11 15:29:15 +01:00
address review: offset fallback for far backward album seeks; pin the playlist keyset
- The NULL-aware backward album predicate is not sargable, so a backward seek from a far cursor scans the whole album partition. Fall back to the offset query when the anchor is more than twice the offset away; near-cursor backward reads still use keyset. - Cover the playlist keyset path: a DAO test for `playlistSongsBefore` and a repository test scanning a playlist forward and backward. Both fail if the playlist order key stops driving the seek.
This commit is contained in:
@@ -764,6 +764,11 @@ class QueueRepository(
|
|||||||
): List<SongItem>? {
|
): List<SongItem>? {
|
||||||
if (!cursor.key.seekable(entry.kind)) return null
|
if (!cursor.key.seekable(entry.kind)) return null
|
||||||
val from = cursor.ordinal
|
val from = cursor.ordinal
|
||||||
|
// A backward seek from a far cursor is not cheaper than the offset query: the album
|
||||||
|
// predicate is not sargable (it has to include NULL disc/track), so it scans the partition
|
||||||
|
// back to `offset` instead of using a range. Fall back when the anchor is more than twice
|
||||||
|
// the offset away, matching the documented far-jump behaviour.
|
||||||
|
if (from > offset && from - offset >= offset) return null
|
||||||
return if (offset >= from) {
|
return if (offset >= from) {
|
||||||
rowsFrom(entry, cursor.key, offset - from, limit)
|
rowsFrom(entry, cursor.key, offset - from, limit)
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -120,6 +120,44 @@ class QueueKeysetTest {
|
|||||||
assertEquals(listOf("n2", "s1"), actual)
|
assertEquals(listOf("n2", "s1"), actual)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun playlistKeysetBeforeMatchesTheOffsetWindow() =
|
||||||
|
runTest {
|
||||||
|
seedLibrary()
|
||||||
|
val all = allPlaylistRows()
|
||||||
|
val anchor = all[10]
|
||||||
|
|
||||||
|
val expected = all.drop(5).take(5).map { it.song.id }
|
||||||
|
val actual =
|
||||||
|
db
|
||||||
|
.queueDao()
|
||||||
|
.playlistSongsBefore(1, "pl1", anchor.position, skip = 0, limit = 5)
|
||||||
|
.asReversed()
|
||||||
|
.map { it.song.id }
|
||||||
|
|
||||||
|
assertEquals(expected, actual)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun scanningAPlaylistThroughTheRepositoryUsesKeysetSeeks() =
|
||||||
|
runTest {
|
||||||
|
seedLibrary()
|
||||||
|
repository.replace(listOf(repository.playlistEntry(1, "pl1")))
|
||||||
|
val snapshot = repository.snapshot()
|
||||||
|
val expected = allPlaylistRows().map { it.song.id }
|
||||||
|
assertEquals(expected.size, snapshot.size.toInt())
|
||||||
|
|
||||||
|
val forward = (0 until snapshot.size).map { repository.itemAt(snapshot, it)?.song?.id }
|
||||||
|
assertEquals(expected, forward)
|
||||||
|
assertTrue("forward keyset seeks: ${repository.keysetSeeks}", repository.keysetSeeks >= snapshot.size - 1)
|
||||||
|
assertEquals(1L, repository.offsetSeeks)
|
||||||
|
|
||||||
|
val beforeBackward = repository.keysetSeeks
|
||||||
|
val backward = (snapshot.size - 1 downTo 0).map { repository.itemAt(snapshot, it)?.song?.id }.asReversed()
|
||||||
|
assertEquals(expected, backward)
|
||||||
|
assertTrue("backward keyset seeks", repository.keysetSeeks > beforeBackward)
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun scanningAnAlbumWithNullDiscAndTrackBackwardsResolvesEveryRow() =
|
fun scanningAnAlbumWithNullDiscAndTrackBackwardsResolvesEveryRow() =
|
||||||
runTest {
|
runTest {
|
||||||
@@ -176,7 +214,9 @@ class QueueKeysetTest {
|
|||||||
val resolved =
|
val resolved =
|
||||||
(snapshot.size - 1 downTo 0).map { repository.itemAt(snapshot, it)?.song?.id }.asReversed()
|
(snapshot.size - 1 downTo 0).map { repository.itemAt(snapshot, it)?.song?.id }.asReversed()
|
||||||
assertEquals(expected, resolved)
|
assertEquals(expected, resolved)
|
||||||
assertTrue(repository.keysetSeeks >= snapshot.size - 1)
|
// Most steps are keyset; the first few near the start fall back to OFFSET, which is
|
||||||
|
// cheaper there.
|
||||||
|
assertTrue("backward keyset seeks: ${repository.keysetSeeks}", repository.keysetSeeks >= snapshot.size * 9 / 10)
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
Reference in New Issue
Block a user