yonlu/yellowjacket
1
1
Fork 0
Code Issues 55 Pull Requests 6 Actions Packages Projects Releases 12 Wiki Activity

release-group MBID backfill queries a table renamed away in plan 013 #189

New Issue
Closed
opened 2026-08-21 20:17:01 +00:00 by logan · 1 comment
logan commented 2026-08-21 20:17:01 +00:00
Collaborator
Copy Link
Copy Source

Report

BackfillReleaseGroupMBIDs queries a table that has not existed since
plan 013. It fails on the first statement, on every launch, and the
backfill it performs has therefore never run since e7748f1.

Found on the reference device (TLP301, Android 14) with a real
1,577-track library, in the first minute of logcat output after #160
landed — which is the point: this warning has been written on every
launch, desktop included, and on Android it went to /dev/null.

W/yellowjacket: msg="release-group mbid backfill: query failed"
  explore.error="SQL logic error: no such table: release_groups (1)"

Findings

e7748f1 ("shape the library like files, and shrink the catalog")
renamed release_groups to albums. backend/database/sql/schemas/ albums.sql says so in its own second line — "This is release_groups
renamed, and the rename is the point". Two raw-SQL statements in
backend/explore/explore.go were not renamed with it:

  • backfillReleaseGroupMBIDs, the SELECT at :285
  • the UPDATE at :347, in the same function

Both are raw QueryContext / ExecContext strings rather than sqlc
queries, which is the whole reason the rename missed them: sqlc reads
sql/schemas/ and would have failed to generate against a table that
is not declared. grep says these are the only two left in the tree.

The columns the statements want are all present on albums under the
same names (id, mbid, pending_release_mbid), so this looks like a
pure rename rather than a reshaping.

What it costs

The function is what resolves a release-level MBID into the
release-group MBID everything else on the album page is keyed by. Its
own doc comment describes the split: a scan cannot afford a live
MusicBrainz call, so library.updateMBIDs stashes MUSICBRAINZ_ALBUMID
in pending_release_mbid and defers to this. Many taggers write that
tag instead of MUSICBRAINZ_RELEASEGROUPID, so for those albums the
pending value is written by every scan and resolved by nothing —
the album is untagged as far as the catalog is concerned, permanently.

Worth confirming while fixing: how many rows are actually sitting on a
non-null pending_release_mbid in a real library, since that is the
size of the affected set.

Direction

  1. Rename the table in both statements and check the columns against
    albums.sql rather than assuming, since 013 reshaped as well as
    renamed elsewhere.
  2. A test that runs the backfill against NewTestDB would have caught
    this and would catch the next one — the failure is a first-statement
    error, so it does not need a network or a fixture library, only a
    row with a pending_release_mbid.
  3. Worth asking whether these two want to be sqlc queries. That is what
    made every other statement in the repo immune to this rename, and
    neither of them uses anything outside sqlc's grammar.

Related

#160 is what made this visible on Android; it was equally invisible on
desktop only in the sense that nobody was reading a Warn on a launch
that otherwise looked healthy.

**Report** `BackfillReleaseGroupMBIDs` queries a table that has not existed since plan 013. It fails on the first statement, on **every launch**, and the backfill it performs has therefore never run since `e7748f1`. Found on the reference device (TLP301, Android 14) with a real 1,577-track library, in the first minute of logcat output after #160 landed — which is the point: this warning has been written on every launch, desktop included, and on Android it went to `/dev/null`. ``` W/yellowjacket: msg="release-group mbid backfill: query failed" explore.error="SQL logic error: no such table: release_groups (1)" ``` **Findings** `e7748f1` ("shape the library like files, and shrink the catalog") renamed `release_groups` to `albums`. `backend/database/sql/schemas/ albums.sql` says so in its own second line — *"This is `release_groups` renamed, and the rename is the point"*. Two raw-SQL statements in `backend/explore/explore.go` were not renamed with it: - `backfillReleaseGroupMBIDs`, the `SELECT` at :285 - the `UPDATE` at :347, in the same function Both are **raw `QueryContext` / `ExecContext` strings rather than sqlc queries**, which is the whole reason the rename missed them: sqlc reads `sql/schemas/` and would have failed to generate against a table that is not declared. `grep` says these are the only two left in the tree. The columns the statements want are all present on `albums` under the same names (`id`, `mbid`, `pending_release_mbid`), so this looks like a pure rename rather than a reshaping. **What it costs** The function is what resolves a release-level MBID into the release-group MBID everything else on the album page is keyed by. Its own doc comment describes the split: a scan cannot afford a live MusicBrainz call, so `library.updateMBIDs` stashes `MUSICBRAINZ_ALBUMID` in `pending_release_mbid` and defers to this. Many taggers write that tag instead of `MUSICBRAINZ_RELEASEGROUPID`, so for those albums the pending value is written by every scan and resolved by nothing — the album is untagged as far as the catalog is concerned, permanently. Worth confirming while fixing: how many rows are actually sitting on a non-null `pending_release_mbid` in a real library, since that is the size of the affected set. **Direction** 1. Rename the table in both statements and check the columns against `albums.sql` rather than assuming, since 013 reshaped as well as renamed elsewhere. 2. A test that runs the backfill against `NewTestDB` would have caught this and would catch the next one — the failure is a first-statement error, so it does not need a network or a fixture library, only a row with a `pending_release_mbid`. 3. Worth asking whether these two want to be sqlc queries. That is what made every other statement in the repo immune to this rename, and neither of them uses anything outside sqlc's grammar. **Related** #160 is what made this visible on Android; it was equally invisible on desktop only in the sense that nobody was reading a `Warn` on a launch that otherwise looked healthy.
logan added the Area/ExploreKind/Bug
Priority
High
2
Reviewed
Confirmed
1
 labels 2026-08-21 20:17:01 +00:00
logan self-assigned this 2026-08-21 21:12:08 +00:00
logan added the
Status
In Progress
 label 2026-08-21 21:12:08 +00:00
logan commented 2026-08-21 21:12:09 +00:00
Author
Collaborator
Copy Link
Copy Source

Taking this together with #190 on 189-190-explore-correctness. Both
are Explore correctness, both surfaced in the same minute of logcat
after #160, and both are "supposed to work, doesn't" rather than
ergonomics.

Approach: rename the table in both statements, but check the columns
against albums.sql rather than assuming, since plan 013 reshaped as
well as renamed elsewhere. Then a test against NewTestDB with a row
carrying a pending_release_mbid -- the failure is a first-statement
error, so it needs no network and no fixture library, which is exactly
why nothing caught it.

Also looking at whether these two want to be sqlc queries. That is what
made every other statement in the repo immune to this rename, and
neither uses anything outside sqlc's grammar.

Taking this together with #190 on `189-190-explore-correctness`. Both are Explore correctness, both surfaced in the same minute of logcat after #160, and both are "supposed to work, doesn't" rather than ergonomics. Approach: rename the table in both statements, but check the columns against `albums.sql` rather than assuming, since plan 013 reshaped as well as renamed elsewhere. Then a test against `NewTestDB` with a row carrying a `pending_release_mbid` -- the failure is a first-statement error, so it needs no network and no fixture library, which is exactly why nothing caught it. Also looking at whether these two want to be sqlc queries. That is what made every other statement in the repo immune to this rename, and neither uses anything outside sqlc's grammar.
logan closed this issue 2026-08-21 21:43:38 +00:00
gitea-actions bot removed the
Status
In Progress
 label 2026-08-21 21:43:49 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Area/Design
Area/Downloads
Area/Explore
Area/Library-UI
Area/Metadata
Area/Packaging
Area/Player
Area/Queue
Area/Settings
Area/Shell-Nav
Compat/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
 Platform/Android
Platform/Desktop
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Medium
3
The priority is medium
Priority
Low
4
The priority is low
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
In Progress
Somebody is actively working on this right now
No labels Area/Explore Kind/Bug
Priority
High
2
Reviewed
Confirmed
1
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
logan yonlu
No Assignees logan
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: yonlu/yellowjacket#189