From b035da7435ced7f7beca36a742fa9a80b62ac410 Mon Sep 17 00:00:00 2001 From: Jason Stephenson Date: Fri, 19 Nov 2021 14:44:43 -0500 Subject: [PATCH] LP1937294: Fix metarecord master record choice predictability issue The metabib master record was being chosen using indeterminant code. In the case of a quality tie, which record would be chosen depends on the order that they were returned from the database. This commit adds an additional order by on the biblio.record_entry.id to ensure that the best matching record with the lowest id is chosen. The live_t/lp1145213_test_func_asset.merge_record_assets.pg tests are also adjusted to take this into account and should now pass on all Pg versions and regardless of the order that the data is inserted. Signed-off-by: Jason Stephenson Signed-off-by: Mike Rylander --- Open-ILS/src/sql/Pg/030.schema.metabib.sql | 6 +- ...p1145213_test_func_asset.merge_record_assets.pg | 2 +- .../XXXX.function.lp1937244-postgresql-changes.sql | 94 ++++++++++++++++++++++ 3 files changed, 98 insertions(+), 4 deletions(-) diff --git a/Open-ILS/src/sql/Pg/030.schema.metabib.sql b/Open-ILS/src/sql/Pg/030.schema.metabib.sql index e840911d64..58163faad0 100644 --- a/Open-ILS/src/sql/Pg/030.schema.metabib.sql +++ b/Open-ILS/src/sql/Pg/030.schema.metabib.sql @@ -1574,7 +1574,7 @@ BEGIN ELSE -- indeed there are. Update it with a null cache and recalcualated master record UPDATE metabib.metarecord SET mods = NULL, - master_record = ( SELECT id FROM biblio.record_entry WHERE fingerprint = fp AND NOT deleted ORDER BY quality DESC LIMIT 1) + master_record = (SELECT id FROM biblio.record_entry WHERE fingerprint = fp AND NOT deleted ORDER BY quality DESC, id ASC LIMIT 1) WHERE id = old_mr; END IF; END LOOP; @@ -1610,14 +1610,14 @@ BEGIN ELSE -- indeed there is. update it with a null cache and recalcualated master record UPDATE metabib.metarecord SET mods = NULL, - master_record = ( SELECT id FROM biblio.record_entry WHERE fingerprint = fp AND NOT deleted ORDER BY quality DESC LIMIT 1) + master_record = (SELECT id FROM biblio.record_entry WHERE fingerprint = fp AND NOT deleted ORDER BY quality DESC, id ASC LIMIT 1) WHERE id = old_mr; END IF; ELSE -- there was one we already attached to, update its mods cache and master_record UPDATE metabib.metarecord SET mods = NULL, - master_record = ( SELECT id FROM biblio.record_entry WHERE fingerprint = fp AND NOT deleted ORDER BY quality DESC LIMIT 1) + master_record = (SELECT id FROM biblio.record_entry WHERE fingerprint = fp AND NOT deleted ORDER BY quality DESC, id ASC LIMIT 1) WHERE id = old_mr; END IF; diff --git a/Open-ILS/src/sql/Pg/live_t/lp1145213_test_func_asset.merge_record_assets.pg b/Open-ILS/src/sql/Pg/live_t/lp1145213_test_func_asset.merge_record_assets.pg index 22bcb3a041..ffc99085f7 100644 --- a/Open-ILS/src/sql/Pg/live_t/lp1145213_test_func_asset.merge_record_assets.pg +++ b/Open-ILS/src/sql/Pg/live_t/lp1145213_test_func_asset.merge_record_assets.pg @@ -59,7 +59,7 @@ INSERT INTO asset.copy(id, circ_lib, creator, call_number, editor, copy_number, ----------------------------------- -- do merge -SELECT is(asset.merge_record_assets(60000, 60001), 4, 'Record assets merged!'); +SELECT is(asset.merge_record_assets(60000, 60001), 3, 'Record assets merged!'); -- check if copy 4's acn was updated SELECT is( diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.function.lp1937244-postgresql-changes.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.function.lp1937244-postgresql-changes.sql index 64fea89117..abcf9d3dc2 100644 --- a/Open-ILS/src/sql/Pg/upgrade/XXXX.function.lp1937244-postgresql-changes.sql +++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.function.lp1937244-postgresql-changes.sql @@ -737,4 +737,98 @@ BEGIN END; $func$ LANGUAGE PLPGSQL STABLE STRICT; +CREATE OR REPLACE FUNCTION metabib.remap_metarecord_for_bib( + bib_id bigint, + fp text, + bib_is_deleted boolean DEFAULT false, + retain_deleted boolean DEFAULT false +) RETURNS bigint AS $function$ +DECLARE + new_mapping BOOL := TRUE; + source_count INT; + old_mr BIGINT; + tmp_mr metabib.metarecord%ROWTYPE; + deleted_mrs BIGINT[]; +BEGIN + + -- We need to make sure we're not a deleted master record of an MR + IF bib_is_deleted THEN + IF NOT retain_deleted THEN -- Go away for any MR that we're master of, unless retained + DELETE FROM metabib.metarecord_source_map WHERE source = bib_id; + END IF; + + FOR old_mr IN SELECT id FROM metabib.metarecord WHERE master_record = bib_id LOOP + + -- Now, are there any more sources on this MR? + SELECT COUNT(*) INTO source_count FROM metabib.metarecord_source_map WHERE metarecord = old_mr; + + IF source_count = 0 AND NOT retain_deleted THEN -- No other records + deleted_mrs := ARRAY_APPEND(deleted_mrs, old_mr); -- Just in case... + DELETE FROM metabib.metarecord WHERE id = old_mr; + + ELSE -- indeed there are. Update it with a null cache and recalcualated master record + UPDATE metabib.metarecord + SET mods = NULL, + master_record = (SELECT id FROM biblio.record_entry WHERE fingerprint = fp AND NOT deleted ORDER BY quality DESC, id ASC LIMIT 1) + WHERE id = old_mr; + END IF; + END LOOP; + + ELSE -- insert or update + + FOR tmp_mr IN SELECT m.* FROM metabib.metarecord m JOIN metabib.metarecord_source_map s ON (s.metarecord = m.id) WHERE s.source = bib_id LOOP + + -- Find the first fingerprint-matching + IF old_mr IS NULL AND fp = tmp_mr.fingerprint THEN + old_mr := tmp_mr.id; + new_mapping := FALSE; + + ELSE -- Our fingerprint changed ... maybe remove the old MR + DELETE FROM metabib.metarecord_source_map WHERE metarecord = tmp_mr.id AND source = bib_id; -- remove the old source mapping + SELECT COUNT(*) INTO source_count FROM metabib.metarecord_source_map WHERE metarecord = tmp_mr.id; + IF source_count = 0 THEN -- No other records + deleted_mrs := ARRAY_APPEND(deleted_mrs, tmp_mr.id); + DELETE FROM metabib.metarecord WHERE id = tmp_mr.id; + END IF; + END IF; + + END LOOP; + + -- we found no suitable, preexisting MR based on old source maps + IF old_mr IS NULL THEN + SELECT id INTO old_mr FROM metabib.metarecord WHERE fingerprint = fp; -- is there one for our current fingerprint? + + IF old_mr IS NULL THEN -- nope, create one and grab its id + INSERT INTO metabib.metarecord ( fingerprint, master_record ) VALUES ( fp, bib_id ); + SELECT id INTO old_mr FROM metabib.metarecord WHERE fingerprint = fp; + + ELSE -- indeed there is. update it with a null cache and recalcualated master record + UPDATE metabib.metarecord + SET mods = NULL, + master_record = (SELECT id FROM biblio.record_entry WHERE fingerprint = fp AND NOT deleted ORDER BY quality DESC, id ASC LIMIT 1) + WHERE id = old_mr; + END IF; + + ELSE -- there was one we already attached to, update its mods cache and master_record + UPDATE metabib.metarecord + SET mods = NULL, + master_record = (SELECT id FROM biblio.record_entry WHERE fingerprint = fp AND NOT deleted ORDER BY quality DESC, id ASC LIMIT 1) + WHERE id = old_mr; + END IF; + + IF new_mapping THEN + INSERT INTO metabib.metarecord_source_map (metarecord, source) VALUES (old_mr, bib_id); -- new source mapping + END IF; + + END IF; + + IF ARRAY_UPPER(deleted_mrs,1) > 0 THEN + UPDATE action.hold_request SET target = old_mr WHERE target IN ( SELECT unnest(deleted_mrs) ) AND hold_type = 'M'; -- if we had to delete any MRs above, make sure their holds are moved + END IF; + + RETURN old_mr; + +END; +$function$ LANGUAGE plpgsql; + COMMIT; -- 2.11.0