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 <dbw2@calvin.edu>
Signed-off-by: Lebbeous Fogle-Weekley <lebbeous@esilibrary.com>

diff --git a/Open-ILS/src/sql/Pg/002.schema.config.sql b/Open-ILS/src/sql/Pg/002.schema.config.sql
index dc9d191..0fc21e7 100644 (file)
--- 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 8a82141..39fb75e 100644 (file)
--- 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 (file)
index 0000000..1cdd47b
--- /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 1a020c9..056fe82 100644 (file)
--- 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}) {