Skip to content

Commit 5c91e50

Browse files
committed
Add Data::Medium::load_track_durations in place of view join
751b58f changed the medium data layer to always perform a join againt the new medium_track_durations view, but this unfortunately leads to a very poorly optimized query plan: https://gist.github.com/mwiencek/8f117c96e244213f28333551c753cdd4 I've partially reverted that commit and introduced a load_track_durations method to query the view separately from the main query, which is optimized as expected.
1 parent 51fe67c commit 5c91e50

File tree

8 files changed

+44
-10
lines changed

8 files changed

+44
-10
lines changed

lib/MusicBrainz/Server/Controller/CDTOC.pm

+2
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ sub set_durations : Chained('load') PathPart('set-durations') Edit
127127
message => l('Could not find medium')
128128
);
129129

130+
$c->model('Medium')->load_track_durations($medium);
130131
$c->model('Release')->load($medium);
131132

132133
$c->model('Track')->load_for_mediums($medium);
@@ -378,6 +379,7 @@ sub move : Local Edit
378379
) unless $medium->may_have_discids;
379380

380381
$c->model('Medium')->load($medium_cdtoc);
382+
$c->model('Medium')->load_track_durations($medium_cdtoc->medium);
381383

382384
$c->model('Track')->load_for_mediums($medium);
383385
$c->model('Recording')->load($medium->all_tracks);

lib/MusicBrainz/Server/Controller/Release.pm

+1
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ sub discids : Chained('load') {
145145
my @medium_cdtocs = $c->model('MediumCDTOC')->load_for_mediums($release->all_mediums);
146146
$c->model('CDTOC')->load(@medium_cdtocs);
147147
$c->model('Medium')->load(@medium_cdtocs);
148+
$c->model('Medium')->load_track_durations(map { $_->medium } @medium_cdtocs);
148149
$c->stash( has_cdtocs => scalar(@medium_cdtocs) > 0 );
149150
}
150151

lib/MusicBrainz/Server/Data/Medium.pm

+28-5
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,13 @@ use Scalar::Util qw( weaken );
1818

1919
sub _table
2020
{
21-
return 'medium LEFT JOIN medium_track_durations mtd ON mtd.medium = medium.id';
21+
return 'medium';
2222
}
2323

2424
sub _columns
2525
{
2626
return 'medium.id, release, position, format, medium.name,
2727
medium.edits_pending, track_count,
28-
mtd.pregap_length, mtd.cdtoc_track_lengths, mtd.data_track_lengths,
2928
COALESCE((SELECT true FROM track WHERE medium = medium.id AND position = 0), false) AS has_pregap,
3029
(SELECT count(*) FROM track WHERE medium = medium.id AND position > 0 AND is_data_track = false) AS cdtoc_track_count';
3130
}
@@ -47,9 +46,6 @@ sub _column_mapping
4746
edits_pending => 'edits_pending',
4847
has_pregap => 'has_pregap',
4948
cdtoc_track_count => 'cdtoc_track_count',
50-
cdtoc_track_lengths => 'cdtoc_track_lengths',
51-
data_track_lengths => 'data_track_lengths',
52-
pregap_length => 'pregap_length',
5349
};
5450
}
5551

@@ -64,6 +60,32 @@ sub load
6460
return load_subobjects($self, 'medium', @objs);
6561
}
6662

63+
sub load_track_durations {
64+
my ($self, @mediums) = @_;
65+
66+
@mediums = grep { !$_->durations_loaded } @mediums;
67+
return unless @mediums;
68+
69+
my %id_to_medium = object_to_ids(@mediums);
70+
my @ids = keys %id_to_medium;
71+
return unless @ids;
72+
73+
my $query =
74+
'SELECT medium, pregap_length, cdtoc_track_lengths, data_track_lengths ' .
75+
'FROM medium_track_durations ' .
76+
'WHERE medium = any(?)';
77+
78+
my $duration_rows = $self->sql->select_list_of_hashes($query, [\@ids]);
79+
for my $row (@$duration_rows) {
80+
for my $medium (@{ $id_to_medium{ $row->{medium} } }) {
81+
$medium->pregap_length($row->{pregap_length});
82+
$medium->cdtoc_track_lengths($row->{cdtoc_track_lengths});
83+
$medium->data_track_lengths($row->{data_track_lengths});
84+
$medium->durations_loaded(1);
85+
}
86+
}
87+
}
88+
6789
sub load_for_releases
6890
{
6991
my ($self, @releases) = @_;
@@ -89,6 +111,7 @@ sub load_for_releases
89111
}
90112
}
91113

114+
$self->load_track_durations(@mediums);
92115
$_->mediums_loaded(1) for @releases;
93116
}
94117

lib/MusicBrainz/Server/Edit/Medium/SetTrackLengths.pm

+1
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ sub initialize {
100100
$self->c->model('Release')->load($medium);
101101
$self->c->model('ArtistCredit')->load($medium->release);
102102
$self->c->model('Track')->load_for_mediums($medium);
103+
$self->c->model('Medium')->load_track_durations($medium);
103104

104105
my $cdtoc = $self->c->model('CDTOC')->get_by_id($cdtoc_id);
105106

lib/MusicBrainz/Server/Entity/Medium.pm

+9-3
Original file line numberDiff line numberDiff line change
@@ -132,17 +132,23 @@ has 'cdtoc_track_count' => (
132132

133133
has 'cdtoc_track_lengths' => (
134134
is => 'rw',
135-
isa => 'ArrayRef[Maybe[Int]]'
135+
isa => 'Maybe[ArrayRef[Maybe[Int]]]'
136136
);
137137

138138
has 'data_track_lengths' => (
139139
is => 'rw',
140-
isa => 'ArrayRef[Maybe[Int]]'
140+
isa => 'Maybe[ArrayRef[Maybe[Int]]]'
141141
);
142142

143143
has 'pregap_length' => (
144144
is => 'rw',
145-
isa => 'ArrayRef[Maybe[Int]]'
145+
isa => 'Maybe[ArrayRef[Maybe[Int]]]'
146+
);
147+
148+
has 'durations_loaded' => (
149+
is => 'rw',
150+
isa => 'Bool',
151+
default => 0,
146152
);
147153

148154
sub audio_tracks {

lib/MusicBrainz/Server/Entity/MediumCDTOC.pm

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ sub is_perfect_match {
3333
my ($self) = @_;
3434

3535
my @cdtoc_info = @{ $self->cdtoc->track_details };
36-
my @medium_track_lengths = @{ $self->medium->cdtoc_track_lengths };
36+
my @medium_track_lengths = @{ $self->medium->cdtoc_track_lengths // [] };
3737

3838
return (@cdtoc_info == @medium_track_lengths) && all {
3939
defined $_->[1] && $_->[0]{length_time} == $_->[1]

t/lib/t/MusicBrainz/Server/Data/DurationLookup.pm

+1-1
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ test 'TOC lookup for disc with pregap track' => sub {
126126
my $medium = $c->model('Medium')->get_by_id($created->{id});
127127
isa_ok($medium, 'MusicBrainz::Server::Entity::Medium');
128128

129-
$c->model('Track')->load_for_mediums($medium);
129+
$c->model('Medium')->load_track_durations($medium);
130130
is($medium->length, 1122 + 330160, "inserted medium has expected length");
131131

132132
my ($durationlookup, $hits) = $c->model('DurationLookup')->lookup("1 1 39872 15110", 1);

t/lib/t/MusicBrainz/Server/Data/Medium.pm

+1
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ test 'Insert medium' => sub {
5454
my $medium = $c->model('Medium')->get_by_id($created->{id});
5555
isa_ok($medium, 'MusicBrainz::Server::Entity::Medium');
5656

57+
$c->model('Medium')->load_track_durations($medium);
5758
is($medium->length, 330160 + 262000, "inserted medium has expected length");
5859

5960
my $trackoffset0 = 150;

0 commit comments

Comments
 (0)