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 <jason@sigio.com>
Signed-off-by: Mike Rylander <mrylander@gmail.com>

diff --git a/Open-ILS/src/sql/Pg/030.schema.metabib.sql b/Open-ILS/src/sql/Pg/030.schema.metabib.sql
index e840911..58163fa 100644 (file)
--- 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 22bcb3a..ffc9908 100644 (file)
--- 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 64fea89..abcf9d3 100644 (file)
--- 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;