Constrain serial.issuance.holding_code to be valid JSON or NULL

*NOTE* when merging to master and rel_2_2, you'll need to replace XXXX
not only in the upgrade script but in the version-upgrade script as well!

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.

Signed-off-by: Lebbeous Fogle-Weekley <lebbeous@esilibrary.com>

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/XXXX.schema.serial-holding-code-constraint.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-code-constraint.sql
new file mode 100644 (file)
index 0000000..3ac6453
--- /dev/null
+++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.serial-holding-code-constraint.sql
@@ -0,0 +1,66 @@
+BEGIN;
+
+SELECT evergreen.upgrade_deps_block_check('XXXX', :eg_version);
+
+-- This throws away data, but only data that causes breakage anyway.
+UPDATE serial.issuance SET holding_code = NULL WHERE NOT evergreen.is_json(holding_code);
+
+ALTER TABLE serial.issuance ADD CHECK (holding_code IS NULL OR evergreen.is_json(holding_code));
+
+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 08cecb4..21d82ee 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,12 @@ 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('XXXX', :eg_version);
+
+-- This throws away data, but only data that causes breakage anyway.
+UPDATE serial.issuance SET holding_code = NULL WHERE NOT evergreen.is_json(holding_code);
+
+ALTER TABLE serial.issuance ADD CHECK (holding_code IS NULL OR evergreen.is_json(holding_code));
 
 INSERT INTO config.internal_flag (name, value, enabled) VALUES (
     'serial.rematerialize_on_same_holding_code', NULL, FALSE
@@ -15456,6 +15462,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}) {