From c923d4ffca85423f73a2084bea1ad97717af7eda Mon Sep 17 00:00:00 2001 From: miker Date: Tue, 8 Feb 2011 17:22:00 +0000 Subject: [PATCH] Patch from Thomas Berezansky addressing circ/hold config constraints, and their lack of usefulness. The config.circ_matrix_matchpoint and config.hold_matrix_matchpoint tables have unique constraints that are intended to ensure that you have at most one entry in the table for each set of input conditions. These constraints operate over a set of fields that can (in whole or part) be set to null, and thus don't do what they are intended to in most (if not basically all) cases. [This patch] replaces the unique constraints with unique indexes. Nullable fields are coalesced into empty strings so that they can be matched against, and the index only applies to those rows set as active. The included upgrade script cleans up the tables by taking each set of non-unique rows the unique index would fail on and setting all but the first (in id order) to active=false. If an administrator determines that the wrong row was left active they can toggle active off on the one that was left and back on for the correct one. git-svn-id: svn://svn.open-ils.org/ILS/trunk@19408 dcc99617-32d9-48b4-a31d-7c20da2025e4 --- Open-ILS/src/sql/Pg/002.schema.config.sql | 2 +- Open-ILS/src/sql/Pg/100.circ_matrix.sql | 11 ++- Open-ILS/src/sql/Pg/110.hold_matrix.sql | 8 +- .../0482.schema.fix_matchpoint_unique.sql | 82 +++++++++++++++++++ 4 files changed, 94 insertions(+), 9 deletions(-) create mode 100644 Open-ILS/src/sql/Pg/upgrade/0482.schema.fix_matchpoint_unique.sql diff --git a/Open-ILS/src/sql/Pg/002.schema.config.sql b/Open-ILS/src/sql/Pg/002.schema.config.sql index b0c37b8f97..d497ff35bf 100644 --- a/Open-ILS/src/sql/Pg/002.schema.config.sql +++ b/Open-ILS/src/sql/Pg/002.schema.config.sql @@ -70,7 +70,7 @@ CREATE TABLE config.upgrade_log ( install_date TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT NOW() ); -INSERT INTO config.upgrade_log (version) VALUES ('0481'); -- dbs +INSERT INTO config.upgrade_log (version) VALUES ('0482'); -- miker CREATE TABLE config.bib_source ( id SERIAL PRIMARY KEY, diff --git a/Open-ILS/src/sql/Pg/100.circ_matrix.sql b/Open-ILS/src/sql/Pg/100.circ_matrix.sql index c16adaf28c..ed70f9d7d6 100644 --- a/Open-ILS/src/sql/Pg/100.circ_matrix.sql +++ b/Open-ILS/src/sql/Pg/100.circ_matrix.sql @@ -79,6 +79,7 @@ INSERT INTO config.videorecording_format_map VALUES ('z','Other'); CREATE TABLE config.circ_matrix_matchpoint ( id SERIAL PRIMARY KEY, active BOOL NOT NULL DEFAULT TRUE, + -- Match Fields org_unit INT NOT NULL REFERENCES actor.org_unit (id) DEFERRABLE INITIALLY DEFERRED, -- Set to the top OU for the matchpoint applicability range; we can use org_unit_prox to choose the "best" grp INT NOT NULL REFERENCES permission.grp_tree (id) DEFERRABLE INITIALLY DEFERRED, -- Set to the top applicable group from the group tree; will need descendents and prox functions for filtering circ_modifier TEXT REFERENCES config.circ_modifier (code) DEFERRABLE INITIALLY DEFERRED, @@ -93,6 +94,7 @@ CREATE TABLE config.circ_matrix_matchpoint ( is_renewal BOOL, usr_age_lower_bound INTERVAL, usr_age_upper_bound INTERVAL, + -- "Result" Fields circulate BOOL NOT NULL DEFAULT TRUE, -- Hard "can't circ" flag requiring an override duration_rule INT NOT NULL REFERENCES config.rule_circ_duration (id) DEFERRABLE INITIALLY DEFERRED, recurring_fine_rule INT NOT NULL REFERENCES config.rule_recurring_fine (id) DEFERRABLE INITIALLY DEFERRED, @@ -100,14 +102,11 @@ CREATE TABLE config.circ_matrix_matchpoint ( hard_due_date INT REFERENCES config.hard_due_date (id) DEFERRABLE INITIALLY DEFERRED, script_test TEXT, -- javascript source total_copy_hold_ratio FLOAT, - available_copy_hold_ratio FLOAT, - CONSTRAINT ep_once_per_grp_loc_mod_marc UNIQUE ( - grp, org_unit, circ_modifier, marc_type, marc_form, marc_vr_format, ref_flag, - juvenile_flag, usr_age_lower_bound, usr_age_upper_bound, is_renewal, copy_circ_lib, - copy_owning_lib - ) + available_copy_hold_ratio FLOAT ); +-- Nulls don't count for a constraint match, so we have to coalesce them into something that does. +CREATE UNIQUE INDEX ccmm_once_per_paramset ON config.circ_matrix_matchpoint (org_unit, grp, COALESCE(circ_modifier, ''), COALESCE(marc_type, ''), COALESCE(marc_form, ''), COALESCE(marc_vr_format, ''), COALESCE(copy_circ_lib::TEXT, ''), COALESCE(copy_owning_lib::TEXT, ''), COALESCE(user_home_ou::TEXT, ''), COALESCE(ref_flag::TEXT, ''), COALESCE(juvenile_flag::TEXT, ''), COALESCE(is_renewal::TEXT, ''), COALESCE(usr_age_lower_bound::TEXT, ''), COALESCE(usr_age_upper_bound::TEXT, '')) WHERE active; -- Tests for max items out by circ_modifier CREATE TABLE config.circ_matrix_circ_mod_test ( diff --git a/Open-ILS/src/sql/Pg/110.hold_matrix.sql b/Open-ILS/src/sql/Pg/110.hold_matrix.sql index 84c6b117e5..ae5d742f41 100644 --- a/Open-ILS/src/sql/Pg/110.hold_matrix.sql +++ b/Open-ILS/src/sql/Pg/110.hold_matrix.sql @@ -32,6 +32,7 @@ CREATE TABLE config.hold_matrix_matchpoint ( id SERIAL PRIMARY KEY, active BOOL NOT NULL DEFAULT TRUE, strict_ou_match BOOL NOT NULL DEFAULT FALSE, + -- Match Fields user_home_ou INT REFERENCES actor.org_unit (id) DEFERRABLE INITIALLY DEFERRED, -- Set to the top OU for the matchpoint applicability range; we can use org_unit_prox to choose the "best" request_ou INT REFERENCES actor.org_unit (id) DEFERRABLE INITIALLY DEFERRED, -- Set to the top OU for the matchpoint applicability range; we can use org_unit_prox to choose the "best" pickup_ou INT REFERENCES actor.org_unit (id) DEFERRABLE INITIALLY DEFERRED, -- Set to the top OU for the matchpoint applicability range; we can use org_unit_prox to choose the "best" @@ -45,16 +46,19 @@ CREATE TABLE config.hold_matrix_matchpoint ( marc_vr_format TEXT REFERENCES config.videorecording_format_map (code) DEFERRABLE INITIALLY DEFERRED, juvenile_flag BOOL, ref_flag BOOL, + -- "Result" Fields holdable BOOL NOT NULL DEFAULT TRUE, -- Hard "can't hold" flag requiring an override distance_is_from_owner BOOL NOT NULL DEFAULT FALSE, -- How to calculate transit_range. True means owning lib, false means copy circ lib transit_range INT REFERENCES actor.org_unit_type (id) DEFERRABLE INITIALLY DEFERRED, -- Can circ inside range of cn.owner/cp.circ_lib at depth of the org_unit_type specified here max_holds INT, -- Total hold requests must be less than this, NULL means skip (always pass) include_frozen_holds BOOL NOT NULL DEFAULT TRUE, -- Include frozen hold requests in the count for max_holds test stop_blocked_user BOOL NOT NULL DEFAULT FALSE, -- Stop users who cannot check out items from placing holds - age_hold_protect_rule INT REFERENCES config.rule_age_hold_protect (id) DEFERRABLE INITIALLY DEFERRED, -- still not sure we want to move this off the copy - CONSTRAINT hous_once_per_grp_loc_mod_marc UNIQUE (user_home_ou, request_ou, pickup_ou, item_owning_ou, item_circ_ou, requestor_grp, usr_grp, circ_modifier, marc_type, marc_form, marc_vr_format, ref_flag, juvenile_flag) + age_hold_protect_rule INT REFERENCES config.rule_age_hold_protect (id) DEFERRABLE INITIALLY DEFERRED -- still not sure we want to move this off the copy ); +-- Nulls don't count for a constraint match, so we have to coalesce them into something that does. +CREATE UNIQUE INDEX chmm_once_per_paramset ON config.hold_matrix_matchpoint (COALESCE(user_home_ou::TEXT, ''), COALESCE(request_ou::TEXT, ''), COALESCE(pickup_ou::TEXT, ''), COALESCE(item_owning_ou::TEXT, ''), COALESCE(item_circ_ou::TEXT, ''), COALESCE(usr_grp::TEXT, ''), COALESCE(requestor_grp::TEXT, ''), COALESCE(circ_modifier, ''), COALESCE(marc_type, ''), COALESCE(marc_form, ''), COALESCE(marc_vr_format, ''), COALESCE(juvenile_flag::TEXT, ''), COALESCE(ref_flag::TEXT, '')) WHERE active; + CREATE OR REPLACE FUNCTION action.find_hold_matrix_matchpoint(pickup_ou integer, request_ou integer, match_item bigint, match_user integer, match_requestor integer) RETURNS integer AS $func$ diff --git a/Open-ILS/src/sql/Pg/upgrade/0482.schema.fix_matchpoint_unique.sql b/Open-ILS/src/sql/Pg/upgrade/0482.schema.fix_matchpoint_unique.sql new file mode 100644 index 0000000000..73826053c7 --- /dev/null +++ b/Open-ILS/src/sql/Pg/upgrade/0482.schema.fix_matchpoint_unique.sql @@ -0,0 +1,82 @@ +BEGIN; + +INSERT INTO config.upgrade_log (version) VALUES ('0482'); + +-- Drop old (non-functional) constraints + +ALTER TABLE config.circ_matrix_matchpoint + DROP CONSTRAINT ep_once_per_grp_loc_mod_marc; + +ALTER TABLE config.hold_matrix_matchpoint + DROP CONSTRAINT hous_once_per_grp_loc_mod_marc; + +-- Clean up tables before making normalized index + +CREATE OR REPLACE FUNCTION action.cleanup_matrix_matchpoints() RETURNS void AS $func$ +DECLARE + temp_row RECORD; +BEGIN + -- Circ Matrix + FOR temp_row IN + SELECT org_unit, grp, circ_modifier, marc_type, marc_form, marc_vr_format, copy_circ_lib, copy_owning_lib, user_home_ou, ref_flag, juvenile_flag, is_renewal, usr_age_lower_bound, usr_age_upper_bound, COUNT(id) as rowcount, MIN(id) as firstrow + FROM config.circ_matrix_matchpoint + WHERE active + GROUP BY org_unit, grp, circ_modifier, marc_type, marc_form, marc_vr_format, copy_circ_lib, copy_owning_lib, user_home_ou, ref_flag, juvenile_flag, is_renewal, usr_age_lower_bound, usr_age_upper_bound + HAVING COUNT(id) > 1 LOOP + + UPDATE config.circ_matrix_matchpoint SET active=false + WHERE id > temp_row.firstrow + AND org_unit = temp_row.org_unit + AND grp = temp_row.grp + AND circ_modifier IS NOT DISTINCT FROM temp_row.circ_modifier + AND marc_type IS NOT DISTINCT FROM temp_row.marc_type + AND marc_form IS NOT DISTINCT FROM temp_row.marc_form + AND marc_vr_format IS NOT DISTINCT FROM temp_row.marc_vr_format + AND copy_circ_lib IS NOT DISTINCT FROM temp_row.copy_circ_lib + AND copy_owning_lib IS NOT DISTINCT FROM temp_row.copy_owning_lib + AND user_home_ou IS NOT DISTINCT FROM temp_row.user_home_ou + AND ref_flag IS NOT DISTINCT FROM temp_row.ref_flag + AND juvenile_flag IS NOT DISTINCT FROM temp_row.juvenile_flag + AND is_renewal IS NOT DISTINCT FROM temp_row.is_renewal + AND usr_age_lower_bound IS NOT DISTINCT FROM temp_row.usr_age_lower_bound + AND usr_age_upper_bound IS NOT DISTINCT FROM temp_row.usr_age_upper_bound; + END LOOP; + + -- Hold Matrix + FOR temp_row IN + SELECT user_home_ou, request_ou, pickup_ou, item_owning_ou, item_circ_ou, usr_grp, requestor_grp, circ_modifier, marc_type, marc_form, marc_vr_format, juvenile_flag, ref_flag, COUNT(id) as rowcount, MIN(id) as firstrow + FROM config.hold_matrix_matchpoint + WHERE active + GROUP BY user_home_ou, request_ou, pickup_ou, item_owning_ou, item_circ_ou, usr_grp, requestor_grp, circ_modifier, marc_type, marc_form, marc_vr_format, juvenile_flag, ref_flag + HAVING COUNT(id) > 1 LOOP + + UPDATE config.hold_matrix_matchpoint SET active=false + WHERE id > temp_row.firstrow + AND user_home_ou IS NOT DISTINCT FROM temp_row.user_home_ou + AND request_ou IS NOT DISTINCT FROM temp_row.request_ou + AND pickup_ou IS NOT DISTINCT FROM temp_row.pickup_ou + AND item_owning_ou IS NOT DISTINCT FROM temp_row.item_owning_ou + AND item_circ_ou IS NOT DISTINCT FROM temp_row.item_circ_ou + AND usr_grp IS NOT DISTINCT FROM temp_row.usr_grp + AND requestor_grp IS NOT DISTINCT FROM temp_row.requestor_grp + AND circ_modifier IS NOT DISTINCT FROM temp_row.circ_modifier + AND marc_type IS NOT DISTINCT FROM temp_row.marc_type + AND marc_form IS NOT DISTINCT FROM temp_row.marc_form + AND marc_vr_format IS NOT DISTINCT FROM temp_row.marc_vr_format + AND juvenile_flag IS NOT DISTINCT FROM temp_row.juvenile_flag + AND ref_flag IS NOT DISTINCT FROM temp_row.ref_flag; + END LOOP; +END; +$func$ LANGUAGE plpgsql; + +SELECT action.cleanup_matrix_matchpoints(); + +DROP FUNCTION IF EXISTS action.cleanup_matrix_matchpoints(); + +-- Create Normalized indexes + +CREATE UNIQUE INDEX ccmm_once_per_paramset ON config.circ_matrix_matchpoint (org_unit, grp, COALESCE(circ_modifier, ''), COALESCE(marc_type, ''), COALESCE(marc_form, ''), COALESCE(marc_vr_format, ''), COALESCE(copy_circ_lib::TEXT, ''), COALESCE(copy_owning_lib::TEXT, ''), COALESCE(user_home_ou::TEXT, ''), COALESCE(ref_flag::TEXT, ''), COALESCE(juvenile_flag::TEXT, ''), COALESCE(is_renewal::TEXT, ''), COALESCE(usr_age_lower_bound::TEXT, ''), COALESCE(usr_age_upper_bound::TEXT, '')) WHERE active; + +CREATE UNIQUE INDEX chmm_once_per_paramset ON config.hold_matrix_matchpoint (COALESCE(user_home_ou::TEXT, ''), COALESCE(request_ou::TEXT, ''), COALESCE(pickup_ou::TEXT, ''), COALESCE(item_owning_ou::TEXT, ''), COALESCE(item_circ_ou::TEXT, ''), COALESCE(usr_grp::TEXT, ''), COALESCE(requestor_grp::TEXT, ''), COALESCE(circ_modifier, ''), COALESCE(marc_type, ''), COALESCE(marc_form, ''), COALESCE(marc_vr_format, ''), COALESCE(juvenile_flag::TEXT, ''), COALESCE(ref_flag::TEXT, '')) WHERE active; + +COMMIT; -- 2.43.2