From 18cb25a0ce9a16647787143125f689cddba6da2a Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Fri, 8 Apr 2016 21:46:35 -0400 Subject: [PATCH] LP#1568195: fix retrieving big OUS batches This patch fixes a regression introduced in LP#1501471 where the Library Settings Editor could fail to retrieve the values of org unit settings if more than 99 were requested at a time. To test ------- [1] Open the XUL library settings editor and ensure that no search filters are in effect. Note that values are not displayed for any of the OU settings, and that the Pg log contains error messages like this: "ERROR: cannot pass more than 100 arguments to a function" [2] Apply the patch. [3] Repeat step one, and verify that values are now retrieved for all of the OU settings that have values set. [4] Verify that the pgTAP tests in live_t/aous_batch.pg pass. Signed-off-by: Galen Charlton Signed-off-by: Mike Rylander --- .../lib/OpenILS/Application/AppUtils.pm | 8 ++++- Open-ILS/src/sql/Pg/020.schema.functions.sql | 4 +-- Open-ILS/src/sql/Pg/live_t/aous_batch.pg | 6 ++-- ...ke_ous_batch_retrieve_func_nonvariadic.sql | 36 +++++++++++++++++++ 4 files changed, 48 insertions(+), 6 deletions(-) create mode 100644 Open-ILS/src/sql/Pg/upgrade/XXXX.schema.make_ous_batch_retrieve_func_nonvariadic.sql diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm index 1378a471ba..5f4714b0ae 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm @@ -1315,7 +1315,13 @@ sub ou_ancestor_setting_batch_insecure { my( $self, $orgid, $names ) = @_; my %result = map { $_ => undef } @$names; - my $query = {from => ['actor.org_unit_ancestor_setting_batch', $orgid, @$names]}; + my $query = { + from => [ + 'actor.org_unit_ancestor_setting_batch', + $orgid, + '{' . join(',', @$names) . '}' + ] + }; my $e = OpenILS::Utils::CStoreEditor->new(); my $settings = $e->json_query($query); foreach my $setting (@$settings) { diff --git a/Open-ILS/src/sql/Pg/020.schema.functions.sql b/Open-ILS/src/sql/Pg/020.schema.functions.sql index 71b5116f14..7cc5be0c6c 100644 --- a/Open-ILS/src/sql/Pg/020.schema.functions.sql +++ b/Open-ILS/src/sql/Pg/020.schema.functions.sql @@ -293,7 +293,7 @@ Search "up" the org_unit tree until we find the first occurrence of an org_unit_setting with the given name. $$; -CREATE OR REPLACE FUNCTION actor.org_unit_ancestor_setting_batch( org_id INT, VARIADIC setting_names TEXT[] ) RETURNS SETOF actor.org_unit_setting AS $$ +CREATE OR REPLACE FUNCTION actor.org_unit_ancestor_setting_batch( org_id INT, setting_names TEXT[] ) RETURNS SETOF actor.org_unit_setting AS $$ DECLARE setting RECORD; setting_name TEXT; @@ -316,7 +316,7 @@ BEGIN END; $$ LANGUAGE plpgsql STABLE; -COMMENT ON FUNCTION actor.org_unit_ancestor_setting_batch( INT, VARIADIC TEXT[] ) IS $$ +COMMENT ON FUNCTION actor.org_unit_ancestor_setting_batch( INT, TEXT[] ) IS $$ For each setting name passed, search "up" the org_unit tree until we find the first occurrence of an org_unit_setting with the given name. $$; diff --git a/Open-ILS/src/sql/Pg/live_t/aous_batch.pg b/Open-ILS/src/sql/Pg/live_t/aous_batch.pg index 4a0dabd33e..87c05e5b86 100644 --- a/Open-ILS/src/sql/Pg/live_t/aous_batch.pg +++ b/Open-ILS/src/sql/Pg/live_t/aous_batch.pg @@ -10,20 +10,20 @@ INSERT INTO actor.org_unit_setting (org_unit, name, value) VALUES (2, 'foo', '"f INSERT INTO actor.org_unit_setting (org_unit, name, value) VALUES (2, 'bar', '"bar 2"'); SELECT results_eq( - $$ SELECT name, value FROM actor.org_unit_ancestor_setting_batch(1, 'foo', 'bar') $$, + $$ SELECT name, value FROM actor.org_unit_ancestor_setting_batch(1, '{foo,bar}') $$, $$ VALUES ('foo', '"foo 1"') $$, 'can retrieve batch of org unit settings' ); SELECT results_eq( - $$ SELECT name, value FROM actor.org_unit_ancestor_setting_batch(2, 'foo', 'bar') $$, + $$ SELECT name, value FROM actor.org_unit_ancestor_setting_batch(2, '{foo,bar}') $$, $$ VALUES ('foo', '"foo 2"'), ('bar', '"bar 2"') $$, 'can retrieve batch of org unit settings at lower level' ); DELETE FROM actor.org_unit_setting WHERE name = 'foo' AND org_unit = 2; SELECT results_eq( - $$ SELECT name, value FROM actor.org_unit_ancestor_setting_batch(2, 'foo', 'bar') $$, + $$ SELECT name, value FROM actor.org_unit_ancestor_setting_batch(2, '{foo,bar}') $$, $$ VALUES ('foo', '"foo 1"'), ('bar', '"bar 2"') $$, 'can retrieve batch of org unit settings with fallback' ); diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.make_ous_batch_retrieve_func_nonvariadic.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.make_ous_batch_retrieve_func_nonvariadic.sql new file mode 100644 index 0000000000..f933345f83 --- /dev/null +++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.make_ous_batch_retrieve_func_nonvariadic.sql @@ -0,0 +1,36 @@ +BEGIN; + +--SELECT evergreen.upgrade_deps_block_check('XXXX', :eg_version); + +-- note: it is not necessary to explicitly drop the previous VARIADIC +-- version of this stored procedure; create or replace function... +-- suffices. +CREATE OR REPLACE FUNCTION actor.org_unit_ancestor_setting_batch( org_id INT, setting_names TEXT[] ) RETURNS SETOF actor.org_unit_setting AS $$ +DECLARE + setting RECORD; + setting_name TEXT; + cur_org INT; +BEGIN + FOREACH setting_name IN ARRAY setting_names + LOOP + cur_org := org_id; + LOOP + SELECT INTO setting * FROM actor.org_unit_setting WHERE org_unit = cur_org AND name = setting_name; + IF FOUND THEN + RETURN NEXT setting; + EXIT; + END IF; + SELECT INTO cur_org parent_ou FROM actor.org_unit WHERE id = cur_org; + EXIT WHEN cur_org IS NULL; + END LOOP; + END LOOP; + RETURN; +END; +$$ LANGUAGE plpgsql STABLE; + +COMMENT ON FUNCTION actor.org_unit_ancestor_setting_batch( INT, TEXT[] ) IS $$ +For each setting name passed, search "up" the org_unit tree until +we find the first occurrence of an org_unit_setting with the given name. +$$; + +COMMIT; -- 2.43.2