Stricter order for actor.org_unit_parent_protect()

actor.org_unit_parent_protect() may not work due to the fact
that 'IF' conditions in PL/pgSQL are not necessarily processed
in the order written. This line:

"IF TG_OP = 'INSERT' OR NEW.parent_ou
IS DISTINCT FROM OLD.parent_ou THEN"

may fail because the 'IS DISTINCT FROM' happens before the
'INSERT' check, and and that fails because there is no 'OLD'
variable for INSERTs.

This commit may not be the optimal style for this circumstance
in this language, but it works.  It also appears to change more
than it really does due to a loss of one level of indentation in
the structure.

Signed-off-by: Dan Wells <dbw2@calvin.edu>
Signed-off-by: Dan Scott <dscott@laurentian.ca>

diff --git a/Open-ILS/src/sql/Pg/002.schema.config.sql b/Open-ILS/src/sql/Pg/002.schema.config.sql
index e72e509..a08f61b 100644 (file)
--- a/Open-ILS/src/sql/Pg/002.schema.config.sql
+++ b/Open-ILS/src/sql/Pg/002.schema.config.sql
@@ -86,7 +86,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 ('0604', :eg_version); -- miker/dbs
+INSERT INTO config.upgrade_log (version, applied_to) VALUES ('0605', :eg_version); -- dbwells/dbs
 
 CREATE TABLE config.bib_source (
        id              SERIAL  PRIMARY KEY,

diff --git a/Open-ILS/src/sql/Pg/005.schema.actors.sql b/Open-ILS/src/sql/Pg/005.schema.actors.sql
index 759d59a..7117157 100644 (file)
--- a/Open-ILS/src/sql/Pg/005.schema.actors.sql
+++ b/Open-ILS/src/sql/Pg/005.schema.actors.sql
@@ -297,23 +297,29 @@ CREATE OR REPLACE FUNCTION actor.org_unit_parent_protect () RETURNS TRIGGER AS $
                current_aou := NEW;
                depth_count := 0;
                seen_ous := ARRAY[NEW.id];
-               IF TG_OP = 'INSERT' OR NEW.parent_ou IS DISTINCT FROM OLD.parent_ou THEN
-                       LOOP
-                               IF current_aou.parent_ou IS NULL THEN -- Top of the org tree?
-                                       RETURN NEW; -- No loop. Carry on.
-                               END IF;
-                               IF current_aou.parent_ou = ANY(seen_ous) THEN -- Parent is one we have seen?
-                                       RAISE 'OU LOOP: Saw % twice', current_aou.parent_ou; -- LOOP! ABORT!
-                               END IF;
-                               -- Get the next one!
-                               SELECT INTO current_aou * FROM actor.org_unit WHERE id = current_aou.parent_ou;
-                               seen_ous := seen_ous || current_aou.id;
-                               depth_count := depth_count + 1;
-                               IF depth_count = 100 THEN
-                                       RAISE 'OU CHECK TOO DEEP';
-                               END IF;
-                       END LOOP;
+
+               IF (TG_OP = 'UPDATE') THEN
+                       IF (NEW.parent_ou IS NOT DISTINCT FROM OLD.parent_ou) THEN
+                               RETURN NEW; -- Doing an UPDATE with no change, just return it
+                       END IF;
                END IF;
+
+               LOOP
+                       IF current_aou.parent_ou IS NULL THEN -- Top of the org tree?
+                               RETURN NEW; -- No loop. Carry on.
+                       END IF;
+                       IF current_aou.parent_ou = ANY(seen_ous) THEN -- Parent is one we have seen?
+                               RAISE 'OU LOOP: Saw % twice', current_aou.parent_ou; -- LOOP! ABORT!
+                       END IF;
+                       -- Get the next one!
+                       SELECT INTO current_aou * FROM actor.org_unit WHERE id = current_aou.parent_ou;
+                       seen_ous := seen_ous || current_aou.id;
+                       depth_count := depth_count + 1;
+                       IF depth_count = 100 THEN
+                               RAISE 'OU CHECK TOO DEEP';
+                       END IF;
+               END LOOP;
+
                RETURN NEW;
        END;
 $$ LANGUAGE PLPGSQL;

diff --git a/Open-ILS/src/sql/Pg/upgrade/0605.schema.lp826844_org_unit_parent_protect_fix.sql b/Open-ILS/src/sql/Pg/upgrade/0605.schema.lp826844_org_unit_parent_protect_fix.sql
new file mode 100644 (file)
index 0000000..4a1f732
--- /dev/null
+++ b/Open-ILS/src/sql/Pg/upgrade/0605.schema.lp826844_org_unit_parent_protect_fix.sql
@@ -0,0 +1,50 @@
+-- Evergreen DB patch XXXX.schema.lp826844_org_unit_parent_protect_fix.sql
+--
+-- Correct the fact that actor.org_unit_parent_protect() may not work
+-- due to 'IF' conditions in PL/pgSQL not necessarily processing in the
+-- order written
+--
+BEGIN;
+
+
+-- check whether patch can be applied
+SELECT evergreen.upgrade_deps_block_check('0605', :eg_version);
+
+CREATE OR REPLACE FUNCTION actor.org_unit_parent_protect () RETURNS TRIGGER AS $$
+       DECLARE
+               current_aou actor.org_unit%ROWTYPE;
+               seen_ous    INT[];
+               depth_count INT;
+       BEGIN
+               current_aou := NEW;
+               depth_count := 0;
+               seen_ous := ARRAY[NEW.id];
+
+               IF (TG_OP = 'UPDATE') THEN
+                       IF (NEW.parent_ou IS NOT DISTINCT FROM OLD.parent_ou) THEN
+                               RETURN NEW; -- Doing an UPDATE with no change, just return it
+                       END IF;
+               END IF;
+
+               LOOP
+                       IF current_aou.parent_ou IS NULL THEN -- Top of the org tree?
+                               RETURN NEW; -- No loop. Carry on.
+                       END IF;
+                       IF current_aou.parent_ou = ANY(seen_ous) THEN -- Parent is one we have seen?
+                               RAISE 'OU LOOP: Saw % twice', current_aou.parent_ou; -- LOOP! ABORT!
+                       END IF;
+                       -- Get the next one!
+                       SELECT INTO current_aou * FROM actor.org_unit WHERE id = current_aou.parent_ou;
+                       seen_ous := seen_ous || current_aou.id;
+                       depth_count := depth_count + 1;
+                       IF depth_count = 100 THEN
+                               RAISE 'OU CHECK TOO DEEP';
+                       END IF;
+               END LOOP;
+
+               RETURN NEW;
+       END;
+$$ LANGUAGE PLPGSQL;
+
+
+COMMIT;