actor.org_unit_parent_protect can fail on inserts

[Dan Wells]

EG Version 2.0.8
PG Version 8.4

On a fresh install of 2.0.8, 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"

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

I am far from a PL/pgSQL expert, but attached is an alternate version which seems to work. I suspect there may be some more conventional (i.e. better style) way to do this in this language, but I'll leave it to others to make that determination.

Thanks,
Dan

[Dan Wells (dbw2)]

I'm working up a branch for this, will push early tomorrow.

[Dan Wells (dbw2)]

Branch is pushed to http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/dbwells/lp826844_org_unit_parent_protect

I changed the function a bit to make it a little cleaner than the one I posted above, but I am still interested in knowing if there is a more typical way to deal with such a case.

The branch picks cleanly back through 2.0, but please remember to update the upgrade file to the 2.0 style when stamping it. I tend to forget how it goes, so here's a basic example (I think):

-SELECT evergreen.upgrade_deps_block_check('XXXX', :eg_version);
+INSERT INTO config.upgrade_log (version) VALUES ('XXXX');

Thanks,
Dan

[Dan Wells (dbw2)]

Since the change was two commits for whitespace reasons, I went ahead and made 2_1 and 2_0 branches for clarity:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/dbwells/lp826844_org_unit_parent_protect_2_1

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/dbwells/lp826844_org_unit_parent_protect_2_0

In the future maybe I we should just leave the upgrade_log/block_check line out until the file is stamped, if that is the only difference.

Thanks,
Dan

[Galen Charlton (gmc)]

As far as why, I confirm that PL/pgSQL doesn't do short-circuit evaluation in the manner that the original version of actor.org_unit_parent_protect expected. See

http://www.postgresql.org/docs/9.0/static/sql-expressions.html#SYNTAX-EXPRESS-EVAL
http://archives.postgresql.org/pgsql-bugs/2005-01/msg00035.php

[Dan Scott (denials)]

Thanks Dan. I've committed this to the rel_2_0 branch first as it is a release blocker.

[Dan Scott (denials)]

And pushed to rel_2_1 now.

[Dan Scott (denials)]

And finally pushed to master.

Note that if I knew how (& if!) the supersedes/deprecates stuff worked, I would mark the 0580.schema.aou_parent_protec.sql upgrade script as superseded and/or deprecated as part of the 0605 upgrade script.