From 499d69d64e7c7b660ee1dffc250f8f9552df8daa Mon Sep 17 00:00:00 2001 From: Jason Stephenson Date: Fri, 1 Jul 2016 13:13:16 -0400 Subject: [PATCH] LP#1568046: Replace last two uses of connectby with other function calls. Modify permission functions permission.usr_has_perm_at_nd and permission.usr_has_perm_at_all_nd to no longer require the connectby function from the tablefunc extension. The connectby functionality is replaced by existing database functions to retrieve org. unit descendants. This change removes the need to load the tablefunc extension, so it is dropped from the create scripts. The upgrade script does not remove the extension as sites may be using connectby or other functions from the extension. Signed-off-by: Jason Stephenson Signed-off-by: Ben Shum --- .../src/sql/Pg/006.schema.permissions.sql | 33 +-- .../src/sql/Pg/create_database_contribs.sql | 2 - .../src/sql/Pg/create_database_extensions.sql | 1 - ...ction.perm_functions_without_connectby.sql | 256 ++++++++++++++++++ ...lp1568046-tablefunc-extension-removed.adoc | 15 + 5 files changed, 276 insertions(+), 31 deletions(-) create mode 100644 Open-ILS/src/sql/Pg/upgrade/XXXX.function.perm_functions_without_connectby.sql create mode 100644 docs/RELEASE_NOTES_NEXT/Administration/lp1568046-tablefunc-extension-removed.adoc diff --git a/Open-ILS/src/sql/Pg/006.schema.permissions.sql b/Open-ILS/src/sql/Pg/006.schema.permissions.sql index c351ba8385..30f5ce86ac 100644 --- a/Open-ILS/src/sql/Pg/006.schema.permissions.sql +++ b/Open-ILS/src/sql/Pg/006.schema.permissions.sql @@ -539,21 +539,10 @@ BEGIN -- Use connectby() to find all dependent org units at the specified depth. -- FOR n_curr_ou IN - SELECT ou::INTEGER - FROM connectby( - 'actor.org_unit', -- table name - 'id', -- key column - 'parent_ou', -- recursive foreign key - n_work_ou::TEXT, -- id of starting point - (n_min_depth - n_depth) -- max depth to search, relative - ) -- to starting point - AS t( - ou text, -- dependent org unit - parent_ou text, -- (ignore) - level int -- depth relative to starting point - ) + SELECT id + FROM actor.org_unit_descendants_distance(n_work_ou) WHERE - level = n_min_depth - n_depth + distance = n_min_depth - n_depth LOOP RETURN NEXT n_curr_ou; END LOOP; @@ -596,22 +585,10 @@ BEGIN LOOP -- -- The permission applies only at a depth greater than the work org unit. - -- Use connectby() to find all dependent org units at the specified depth. -- FOR n_child_ou IN - SELECT ou::INTEGER - FROM connectby( - 'actor.org_unit', -- table name - 'id', -- key column - 'parent_ou', -- recursive foreign key - n_head_ou::TEXT, -- id of starting point - 0 -- no limit on search depth - ) - AS t( - ou text, -- dependent org unit - parent_ou text, -- (ignore) - level int -- (ignore) - ) + SELECT id + FROM actor.org_unit_descendants(n_head_ou) LOOP RETURN NEXT n_child_ou; END LOOP; diff --git a/Open-ILS/src/sql/Pg/create_database_contribs.sql b/Open-ILS/src/sql/Pg/create_database_contribs.sql index 49acaa93af..a2fd1c8aae 100644 --- a/Open-ILS/src/sql/Pg/create_database_contribs.sql +++ b/Open-ILS/src/sql/Pg/create_database_contribs.sql @@ -20,8 +20,6 @@ CREATE LANGUAGE plperlu; -- This dance is because :variable/blah doesn't seem to work when doing \i -- But it does when doing \set -- So we \set to a single variable, then use that single variable with \i -\set load_file :contrib_dir/tablefunc.sql -\i :load_file \set load_file :contrib_dir/pgxml.sql \i :load_file \set load_file :contrib_dir/hstore.sql diff --git a/Open-ILS/src/sql/Pg/create_database_extensions.sql b/Open-ILS/src/sql/Pg/create_database_extensions.sql index b61aa5b0b0..ed0a9644a1 100644 --- a/Open-ILS/src/sql/Pg/create_database_extensions.sql +++ b/Open-ILS/src/sql/Pg/create_database_extensions.sql @@ -16,7 +16,6 @@ CREATE DATABASE :db_name TEMPLATE template0 ENCODING 'UNICODE' LC_COLLATE 'C' LC CREATE LANGUAGE plperlu; -CREATE EXTENSION tablefunc; CREATE EXTENSION xml2; CREATE EXTENSION hstore; CREATE EXTENSION intarray; diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.function.perm_functions_without_connectby.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.function.perm_functions_without_connectby.sql new file mode 100644 index 0000000000..b15f73490d --- /dev/null +++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.function.perm_functions_without_connectby.sql @@ -0,0 +1,256 @@ +BEGIN; + +-- SELECT evergreen.upgrade_deps_block_check('XXXX', :eg_version); + +CREATE OR REPLACE FUNCTION permission.usr_has_perm_at_nd( + user_id IN INTEGER, + perm_code IN TEXT +) +RETURNS SETOF INTEGER AS $$ +-- +-- Return a set of all the org units for which a given user has a given +-- permission, granted directly (not through inheritance from a parent +-- org unit). +-- +-- The permissions apply to a minimum depth of the org unit hierarchy, +-- for the org unit(s) to which the user is assigned. (They also apply +-- to the subordinates of those org units, but we don't report the +-- subordinates here.) +-- +-- For purposes of this function, the permission.usr_work_ou_map table +-- defines which users belong to which org units. I.e. we ignore the +-- home_ou column of actor.usr. +-- +-- The result set may contain duplicates, which should be eliminated +-- by a DISTINCT clause. +-- +DECLARE + b_super BOOLEAN; + n_perm INTEGER; + n_min_depth INTEGER; + n_work_ou INTEGER; + n_curr_ou INTEGER; + n_depth INTEGER; + n_curr_depth INTEGER; +BEGIN + -- + -- Check for superuser + -- + SELECT INTO b_super + super_user + FROM + actor.usr + WHERE + id = user_id; + -- + IF NOT FOUND THEN + return; -- No user? No permissions. + ELSIF b_super THEN + -- + -- Super user has all permissions everywhere + -- + FOR n_work_ou IN + SELECT + id + FROM + actor.org_unit + WHERE + parent_ou IS NULL + LOOP + RETURN NEXT n_work_ou; + END LOOP; + RETURN; + END IF; + -- + -- Translate the permission name + -- to a numeric permission id + -- + SELECT INTO n_perm + id + FROM + permission.perm_list + WHERE + code = perm_code; + -- + IF NOT FOUND THEN + RETURN; -- No such permission + END IF; + -- + -- Find the highest-level org unit (i.e. the minimum depth) + -- to which the permission is applied for this user + -- + -- This query is modified from the one in permission.usr_perms(). + -- + SELECT INTO n_min_depth + min( depth ) + FROM ( + SELECT depth + FROM permission.usr_perm_map upm + WHERE upm.usr = user_id + AND (upm.perm = n_perm OR upm.perm = -1) + UNION + SELECT gpm.depth + FROM permission.grp_perm_map gpm + WHERE (gpm.perm = n_perm OR gpm.perm = -1) + AND gpm.grp IN ( + SELECT (permission.grp_ancestors( + (SELECT profile FROM actor.usr WHERE id = user_id) + )).id + ) + UNION + SELECT p.depth + FROM permission.grp_perm_map p + WHERE (p.perm = n_perm OR p.perm = -1) + AND p.grp IN ( + SELECT (permission.grp_ancestors(m.grp)).id + FROM permission.usr_grp_map m + WHERE m.usr = user_id + ) + ) AS x; + -- + IF NOT FOUND THEN + RETURN; -- No such permission for this user + END IF; + -- + -- Identify the org units to which the user is assigned. Note that + -- we pay no attention to the home_ou column in actor.usr. + -- + FOR n_work_ou IN + SELECT + work_ou + FROM + permission.usr_work_ou_map + WHERE + usr = user_id + LOOP -- For each org unit to which the user is assigned + -- + -- Determine the level of the org unit by a lookup in actor.org_unit_type. + -- We take it on faith that this depth agrees with the actual hierarchy + -- defined in actor.org_unit. + -- + SELECT INTO n_depth + type.depth + FROM + actor.org_unit_type type + INNER JOIN actor.org_unit ou + ON ( ou.ou_type = type.id ) + WHERE + ou.id = n_work_ou; + -- + IF NOT FOUND THEN + CONTINUE; -- Maybe raise exception? + END IF; + -- + -- Compare the depth of the work org unit to the + -- minimum depth, and branch accordingly + -- + IF n_depth = n_min_depth THEN + -- + -- The org unit is at the right depth, so return it. + -- + RETURN NEXT n_work_ou; + ELSIF n_depth > n_min_depth THEN + -- + -- Traverse the org unit tree toward the root, + -- until you reach the minimum depth determined above + -- + n_curr_depth := n_depth; + n_curr_ou := n_work_ou; + WHILE n_curr_depth > n_min_depth LOOP + SELECT INTO n_curr_ou + parent_ou + FROM + actor.org_unit + WHERE + id = n_curr_ou; + -- + IF FOUND THEN + n_curr_depth := n_curr_depth - 1; + ELSE + -- + -- This can happen only if the hierarchy defined in + -- actor.org_unit is corrupted, or out of sync with + -- the depths defined in actor.org_unit_type. + -- Maybe we should raise an exception here, instead + -- of silently ignoring the problem. + -- + n_curr_ou = NULL; + EXIT; + END IF; + END LOOP; + -- + IF n_curr_ou IS NOT NULL THEN + RETURN NEXT n_curr_ou; + END IF; + ELSE + -- + -- The permission applies only at a depth greater than the work org unit. + -- Use connectby() to find all dependent org units at the specified depth. + -- + FOR n_curr_ou IN + SELECT id + FROM actor.org_unit_descendants_distance(n_work_ou) + WHERE + distance = n_min_depth - n_depth + LOOP + RETURN NEXT n_curr_ou; + END LOOP; + END IF; + -- + END LOOP; + -- + RETURN; + -- +END; +$$ LANGUAGE 'plpgsql' ROWS 1; + + +CREATE OR REPLACE FUNCTION permission.usr_has_perm_at_all_nd( + user_id IN INTEGER, + perm_code IN TEXT +) +RETURNS SETOF INTEGER AS $$ +-- +-- Return a set of all the org units for which a given user has a given +-- permission, granted either directly or through inheritance from a parent +-- org unit. +-- +-- The permissions apply to a minimum depth of the org unit hierarchy, and +-- to the subordinates of those org units, for the org unit(s) to which the +-- user is assigned. +-- +-- For purposes of this function, the permission.usr_work_ou_map table +-- assigns users to org units. I.e. we ignore the home_ou column of actor.usr. +-- +-- The result set may contain duplicates, which should be eliminated +-- by a DISTINCT clause. +-- +DECLARE + n_head_ou INTEGER; + n_child_ou INTEGER; +BEGIN + FOR n_head_ou IN + SELECT DISTINCT * FROM permission.usr_has_perm_at_nd( user_id, perm_code ) + LOOP + -- + -- The permission applies only at a depth greater than the work org unit. + -- + FOR n_child_ou IN + SELECT id + FROM actor.org_unit_descendants(n_head_ou) + LOOP + RETURN NEXT n_child_ou; + END LOOP; + END LOOP; + -- + RETURN; + -- +END; +$$ LANGUAGE 'plpgsql' ROWS 1; + +COMMIT; + +\qecho The tablefunc database extension is no longer necessary for Evergreen. +\qecho Unless you use some of its functions in your own scripts, you may +\qecho want to run the following command in the database to drop it: +\qecho DROP EXTENSION tablefunc; diff --git a/docs/RELEASE_NOTES_NEXT/Administration/lp1568046-tablefunc-extension-removed.adoc b/docs/RELEASE_NOTES_NEXT/Administration/lp1568046-tablefunc-extension-removed.adoc new file mode 100644 index 0000000000..c7fe7a1beb --- /dev/null +++ b/docs/RELEASE_NOTES_NEXT/Administration/lp1568046-tablefunc-extension-removed.adoc @@ -0,0 +1,15 @@ +Tablefunc Extension No Longer Required +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Changes in the behavior of the connectby function in PostgreSQL 9.5 +have prompted its removal from the database. It is easier for +Evergreen to maintain compatibility with previous versions of +PostgreSQL without this function. + +By eliminating the use of the connectby function, we eliminate the +requirement for the tablefunc database extension. It is no longer +installed when the database is created. If you are upgrading and wish +to remove it from your database, you may run the following statement +in the database to drop it: + + DROP EXTENSION tablefunc; + -- 2.43.2