Skip to content

Commit

Permalink
MBS-12400: fix non-musicbrainz-schema dumps (#2541)
Browse files Browse the repository at this point in the history
Due to the `$table_noschema` addition in
0d38660, we were failing to include the
schema prefix in any dumped tables' file names.  Because MBImport.pl
expects the file names to match the table names as specified in
`@FULL_TABLE_LIST` (which includes schemas for non-musicbrainz-schema
tables), this causes the import to fail at finding data files for those
schemas.

The faulty change referenced above also has the side effect of causing
tables with the same names in different schemas to clobber each other in
the dumps.  For example, the cover-art-archive dump's art_type file
would contain rows from the event_art_archive.art_type table instead!

The only purpose of the `$table_noschema` change was to ensure that the
file names inside dbmirror v2 packets didn't include the "dbmirror2."
prefix.  It wouldn't cause any issue if they did, but it was preferred
to match how dbmirror v1 packets are dumped (whose tables are in the
musicbrainz schema, so are not schema-qualified).  I moved the
re-mapping from dump_table to ExportAllTables where it's needed.
  • Loading branch information
mwiencek authored May 25, 2022
1 parent 9d94b60 commit b753651
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 6 deletions.
10 changes: 9 additions & 1 deletion admin/ExportAllTables
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,15 @@ if ($fDoReplication)
if ($dump_dbmirror2) {
# NULL columns that are useless on slaves.
$sql->do('UPDATE dbmirror2.pending_data SET oldctid = NULL, trgdepth = NULL');
$mbdump->dump_table($_) for @dbmirror2_tables;

for my $table (@dbmirror2_tables) {
# Map the schema-qualified dbmirror2 table names to file
# names without the schema inside the packet.
my $table_noschema = $table =~ s/^dbmirror2\.//r;
$mbdump->table_file_mapping->{$table} = $table_noschema;
$mbdump->dump_table($table);
}

$mbdump->write_file('DBMIRROR_VERSION', "2\n");
}

Expand Down
5 changes: 0 additions & 5 deletions lib/MusicBrainz/Script/DatabaseDump.pm
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,6 @@ sub _open_table_file {
sub dump_table {
my ($self, $table) = @_;

my $table_noschema = $table =~ s/^[^.]+\.//r;
if ($table ne $table_noschema) {
$self->table_file_mapping->{$table} = $table_noschema;
}

my ($dump_fh, $table_file_path) = $self->_open_table_file($table, '>');

my $rows_estimate = $self->row_counts->{$table} //
Expand Down
1 change: 1 addition & 0 deletions script/reset_selenium_env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ fi
echo `date` : Truncating all tables
OUTPUT=`./admin/psql SELENIUM <./admin/sql/TruncateTables.sql 2>&1` || ( echo "$OUTPUT" && exit 1 )
OUTPUT=`./admin/psql SELENIUM <./admin/sql/caa/TruncateTables.sql 2>&1` || ( echo "$OUTPUT" && exit 1 )
OUTPUT=`./admin/psql SELENIUM <./admin/sql/eaa/TruncateTables.sql 2>&1` || ( echo "$OUTPUT" && exit 1 )

echo `date` : Inserting initial test data
OUTPUT=`./admin/psql SELENIUM < ./t/sql/initial.sql 2>&1` || ( echo "$OUTPUT" && exit 1 )
Expand Down
33 changes: 33 additions & 0 deletions t/script/ExportAllTables.t
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ test all => sub {
File::Spec->catfile($output_dir, 'mbdump.tar.bz2'),
File::Spec->catfile($output_dir, 'mbdump-derived.tar.bz2'),
File::Spec->catfile($output_dir, 'mbdump-editor.tar.bz2'),
File::Spec->catfile($output_dir, 'mbdump-cover-art-archive.tar.bz2'),
File::Spec->catfile($output_dir, 'mbdump-event-art-archive.tar.bz2'),
);

my $replication_setup = File::Spec->catfile($root, 'admin/sql/ReplicationSetup.sql');
Expand Down Expand Up @@ -250,6 +252,37 @@ test all => sub {
},
]);

# MBS-12400: Check that non-musicbrainz-schema tables have been dumped
# and imported. One effect of failing to schema-qualify the dumped
# tables' file names might be tables like event_art_archive.art_type and
# cover_art_archive.art_type clobbering each other.

my $cover_art_types = $c->sql->select_list_of_hashes('SELECT * FROM cover_art_archive.art_type WHERE id = 1');

cmp_deeply($cover_art_types, [
{
id => 1,
name => 'Front',
parent => undef,
child_order => 0,
description => undef,
gid => 'ac337166-a2b3-340c-a0b4-e2b00f1d40a2',
},
]);

my $event_art_types = $c->sql->select_list_of_hashes('SELECT * FROM event_art_archive.art_type WHERE id = 1');

cmp_deeply($event_art_types, [
{
id => 1,
name => 'Poster',
parent => undef,
child_order => 0,
description => undef,
gid => '7ced53fc-bb27-33ae-aeef-79d6e24fec3c',
},
]);

$exec_sql->(<<~'SQL');
SET client_min_messages TO WARNING;
TRUNCATE artist CASCADE;
Expand Down
2 changes: 2 additions & 0 deletions t/sql/initial.sql
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ INSERT INTO editor_collection_type VALUES (15, 'Work', 'work', NULL, 2, NULL, '9
INSERT INTO event_alias_type VALUES (1, 'Event name', NULL, 0, NULL, '412aac48-424b-3052-a314-1f926e8018c8');
INSERT INTO event_alias_type VALUES (2, 'Search hint', NULL, 0, NULL, '9b7e72d0-ef3f-3c75-908c-f94c48eb6484');

INSERT INTO event_art_archive.art_type VALUES (1, 'Poster', NULL, 0, NULL, '7ced53fc-bb27-33ae-aeef-79d6e24fec3c');

INSERT INTO event_type VALUES (1, 'Concert', NULL, 1, 'An individual concert by a single artist or collaboration, often with supporting artists who perform before the main act.', 'ef55e8d7-3d00-394a-8012-f5506a29ff0b');
INSERT INTO event_type VALUES (2, 'Festival', NULL, 2, 'An event where a number of different acts perform across the course of the day. Larger festivals may be spread across multiple days.', 'b6ded574-b592-3f0e-b56e-5b5f06aa0678');
INSERT INTO event_type VALUES (3, 'Launch event', NULL, 3, 'A party, reception or other event held specifically for the launch of a release.', 'caee15f1-f9c4-3122-821f-34ea4011ac7d');
Expand Down

0 comments on commit b753651

Please sign in to comment.