Skip to content

CMRT 24: DAO: Add getGroupsForMbid() and getAllGroupCandidates() - #33

Open
Swarnadip-Kar wants to merge 1 commit into
pr/cmrt-phase1-completefrom
cmrt-dao-mbid-lookup
Open

CMRT 24: DAO: Add getGroupsForMbid() and getAllGroupCandidates()#33
Swarnadip-Kar wants to merge 1 commit into
pr/cmrt-phase1-completefrom
cmrt-dao-mbid-lookup

Conversation

@Swarnadip-Kar

Copy link
Copy Markdown
Owner

Adds the two read-only lookups the CMRT grouping pipeline needs as its
first step, before any fingerprint comparison happens. Pure plumbing —
no behavior changes for anything that already exists.

getGroupsForMbid(mbid)

Returns every cmrt_group whose canonical track's library row has the given
musicbrainz_recording_id. Joins through canonical_track_id rather than
adding an mbid column to cmrt_groups, since that FK already exists and
the canonical track's MBID is exactly what we want anyway.

getAllGroupCandidates()

Returns every cmrt_group, unfiltered. Only meant for the no-MBID fallback
path (AcoustID found no match for the new track, so there's nothing to
pre-filter the SimHash scan with). Documented as such in the header so it
doesn't get reached for in the normal case by mistake.

MbidGroupCandidate

New DTO carrying just group_id, fingerprint_hash, and canonical_track_id.
Deliberately smaller than CmrtGroup -- the SimHash pre-filter step (CMRT 26)
only needs these three fields, not the MusicBrainz-sync or local_preferred
columns CmrtGroup also carries.

Notes for reviewers

  • Neither method touches anything write-side; both are read-only and safe
    to call from the AcoustID worker thread.
  • getAllGroupCandidates() will get slower as a library's group count grows,
    same caveat as any full-table scan. Acceptable for now since it's a
    fallback path, not the common case.

Depends on: CMRT Phase 1 (#32)

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 27872292646

Warning

No base build found for commit 6eeb7ce on pr/cmrt-phase1-complete.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 30.437%

Details

  • Patch coverage: 56 uncovered changes across 1 file (0 of 56 lines covered, 0.0%).

Uncovered Changes

File Changed Covered %
src/library/dao/trackfingerprintdao.cpp 56 0 0.0%

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 116230
Covered Lines: 35377
Line Coverage: 30.44%
Coverage Strength: 59272.14 hits per line

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants