From db61133753257604e93a0be568ee3b9415c56b8c Mon Sep 17 00:00:00 2001 From: Lebbeous Fogle-Weekley Date: Thu, 5 Apr 2012 11:46:05 -0400 Subject: [PATCH] Constrain serial.issuance.holding_code to be valid JSON or NULL This avoids serial.materialize_holding_code() failing on bad data. The upgrade script will actually throw away bad values for serial.issuance.holding_code. This is no real loss, since bad data there prevents any serials functions around the row in question from working properly anyway. This problem was reported by Martha Driscoll and Ben Shum. *Also* put a couple of changes missed from the 0700 upgrade script into 210.schema.serials.sql. Fix new serial constraint upgrades 1. None of the upgrades so far have moved is_json() from the public to the evergreen schema. That's probably a separate issue, but it should be safe to call it unqualified, and that's what the rest of the upgrade file does, so we will too. 2. Add a specific SET CONSTRAINT to avoid deferred trigger problems when ALTERing the table. 3. Make sure that the unwanted columns on materialized_holding_code do not exist regardless of your upgrade path. Signed-off-by: Dan Wells Signed-off-by: Lebbeous Fogle-Weekley --- Open-ILS/src/sql/Pg/002.schema.config.sql | 2 +- Open-ILS/src/sql/Pg/210.schema.serials.sql | 18 +++--- .../0706.schema.serial-holding-code-constraint.sql | 74 ++++++++++++++++++++++ .../sql/Pg/version-upgrade/2.1-2.2-upgrade-db.sql | 14 ++++ 4 files changed, 98 insertions(+), 10 deletions(-) create mode 100644 Open-ILS/src/sql/Pg/upgrade/0706.schema.serial-holding-code-constraint.sql diff --git a/Open-ILS/src/sql/Pg/002.schema.config.sql b/Open-ILS/src/sql/Pg/002.schema.config.sql index dc9d191ba0..0fc21e7d06 100644 --- a/Open-ILS/src/sql/Pg/002.schema.config.sql +++ b/Open-ILS/src/sql/Pg/002.schema.config.sql @@ -87,7 +87,7 @@ CREATE TRIGGER no_overlapping_deps BEFORE INSERT OR UPDATE ON config.db_patch_dependencies FOR EACH ROW EXECUTE PROCEDURE evergreen.array_overlap_check ('deprecates'); -INSERT INTO config.upgrade_log (version, applied_to) VALUES ('0705', :eg_version); -- dbs +INSERT INTO config.upgrade_log (version, applied_to) VALUES ('0706', :eg_version); -- dbwells/senator CREATE TABLE config.bib_source ( id SERIAL PRIMARY KEY, diff --git a/Open-ILS/src/sql/Pg/210.schema.serials.sql b/Open-ILS/src/sql/Pg/210.schema.serials.sql index 8a82141e66..39fb75e057 100644 --- a/Open-ILS/src/sql/Pg/210.schema.serials.sql +++ b/Open-ILS/src/sql/Pg/210.schema.serials.sql @@ -202,6 +202,7 @@ CREATE TABLE serial.issuance ( holding_link_id INT -- probably defunct -- TODO: add columns for separate enumeration/chronology values ); +ALTER TABLE serial.issuance ADD CHECK (holding_code IS NULL OR evergreen.is_json(holding_code)); CREATE INDEX serial_issuance_sub_idx ON serial.issuance (subscription); CREATE INDEX serial_issuance_caption_and_pattern_idx ON serial.issuance (caption_and_pattern); CREATE INDEX serial_issuance_date_published_idx ON serial.issuance (date_published); @@ -348,9 +349,6 @@ CREATE VIEW serial.any_summary AS CREATE TABLE serial.materialized_holding_code ( id BIGSERIAL PRIMARY KEY, issuance INTEGER NOT NULL REFERENCES serial.issuance (id) ON DELETE CASCADE, - holding_type TEXT NOT NULL, - ind1 TEXT, - ind2 TEXT, subfield CHAR, value TEXT ); @@ -362,6 +360,11 @@ use strict; use MARC::Field; use JSON::XS; +if (not defined $_TD->{new}{holding_code}) { + elog(WARNING, 'NULL in "holding_code" column of serial.issuance allowed for now, but may not be useful'); + return; +} + # Do nothing if holding_code has not changed... if ($_TD->{new}{holding_code} eq $_TD->{old}{holding_code}) { @@ -388,18 +391,15 @@ spi_exec_prepared($dstmt, $_TD->{new}{id}); my $istmt = spi_prepare( q{ INSERT INTO serial.materialized_holding_code ( - issuance, holding_type, ind1, ind2, subfield, value - ) VALUES ($1, $2, $3, $4, $5, $6) - }, qw{INT TEXT TEXT TEXT CHAR TEXT} + issuance, subfield, value + ) VALUES ($1, $2, $3) + }, qw{INT CHAR TEXT} ); foreach ($field->subfields) { spi_exec_prepared( $istmt, $_TD->{new}{id}, - $_TD->{new}{holding_type}, - $field->indicator(1), - $field->indicator(2), $_->[0], $_->[1] ); diff --git a/Open-ILS/src/sql/Pg/upgrade/0706.schema.serial-holding-code-constraint.sql b/Open-ILS/src/sql/Pg/upgrade/0706.schema.serial-holding-code-constraint.sql new file mode 100644 index 0000000000..1cdd47b168 --- /dev/null +++ b/Open-ILS/src/sql/Pg/upgrade/0706.schema.serial-holding-code-constraint.sql @@ -0,0 +1,74 @@ +BEGIN; + +SELECT evergreen.upgrade_deps_block_check('0706', :eg_version); + +-- This throws away data, but only data that causes breakage anyway. +UPDATE serial.issuance SET holding_code = NULL WHERE NOT is_json(holding_code); + +-- If we don't do this, we have unprocessed triggers and we can't alter the table +SET CONSTRAINTS serial.issuance_caption_and_pattern_fkey IMMEDIATE; + +ALTER TABLE serial.issuance ADD CHECK (holding_code IS NULL OR is_json(holding_code)); + +-- For the sake of completeness if these sneaked through +ALTER TABLE serial.materialized_holding_code DROP COLUMN IF EXISTS holding_type; +ALTER TABLE serial.materialized_holding_code DROP COLUMN IF EXISTS ind1; +ALTER TABLE serial.materialized_holding_code DROP COLUMN IF EXISTS ind2; + +CREATE OR REPLACE FUNCTION serial.materialize_holding_code() RETURNS TRIGGER +AS $func$ +use strict; + +use MARC::Field; +use JSON::XS; + +if (not defined $_TD->{new}{holding_code}) { + elog(WARNING, 'NULL in "holding_code" column of serial.issuance allowed for now, but may not be useful'); + return; +} + +# Do nothing if holding_code has not changed... + +if ($_TD->{new}{holding_code} eq $_TD->{old}{holding_code}) { + # ... unless the following internal flag is set. + + my $flag_rv = spi_exec_query(q{ + SELECT * FROM config.internal_flag + WHERE name = 'serial.rematerialize_on_same_holding_code' AND enabled + }, 1); + return unless $flag_rv->{processed}; +} + + +my $holding_code = (new JSON::XS)->decode($_TD->{new}{holding_code}); + +my $field = new MARC::Field('999', @$holding_code); # tag doesnt matter + +my $dstmt = spi_prepare( + 'DELETE FROM serial.materialized_holding_code WHERE issuance = $1', + 'INT' +); +spi_exec_prepared($dstmt, $_TD->{new}{id}); + +my $istmt = spi_prepare( + q{ + INSERT INTO serial.materialized_holding_code ( + issuance, subfield, value + ) VALUES ($1, $2, $3) + }, qw{INT CHAR TEXT} +); + +foreach ($field->subfields) { + spi_exec_prepared( + $istmt, + $_TD->{new}{id}, + $_->[0], + $_->[1] + ); +} + +return; + +$func$ LANGUAGE 'plperlu'; + +COMMIT; diff --git a/Open-ILS/src/sql/Pg/version-upgrade/2.1-2.2-upgrade-db.sql b/Open-ILS/src/sql/Pg/version-upgrade/2.1-2.2-upgrade-db.sql index 1a020c92fb..056fe82aca 100644 --- a/Open-ILS/src/sql/Pg/version-upgrade/2.1-2.2-upgrade-db.sql +++ b/Open-ILS/src/sql/Pg/version-upgrade/2.1-2.2-upgrade-db.sql @@ -15376,6 +15376,15 @@ INSERT INTO config.org_unit_setting_type ( name, label, description, datatype, g SELECT evergreen.upgrade_deps_block_check('0700', :eg_version); +SELECT evergreen.upgrade_deps_block_check('0706', :eg_version); + +-- This throws away data, but only data that causes breakage anyway. +UPDATE serial.issuance SET holding_code = NULL WHERE NOT is_json(holding_code); + +-- If we don't do this, we have unprocessed triggers and we can't alter the table +SET CONSTRAINTS serial.issuance_caption_and_pattern_fkey IMMEDIATE; + +ALTER TABLE serial.issuance ADD CHECK (holding_code IS NULL OR is_json(holding_code)); INSERT INTO config.internal_flag (name, value, enabled) VALUES ( 'serial.rematerialize_on_same_holding_code', NULL, FALSE @@ -15456,6 +15465,11 @@ use strict; use MARC::Field; use JSON::XS; +if (not defined $_TD->{new}{holding_code}) { + elog(WARNING, 'NULL in "holding_code" column of serial.issuance allowed for now, but may not be useful'); + return; +} + # Do nothing if holding_code has not changed... if ($_TD->{new}{holding_code} eq $_TD->{old}{holding_code}) { -- 2.11.0