From 1c35ee36029aa85e6a04168fc544d5a287638a9a Mon Sep 17 00:00:00 2001 From: Ben Shum Date: Wed, 2 May 2018 17:31:08 -0500 Subject: [PATCH] LP#1441750: use string_agg (again) Some older style ARRAY_TO_STRING(ARRAY_AGG()) should be replaced to use the native STRING_AGG() that comes with PG 9+. This should improve performance for these functions. Signed-off-by: Ben Shum Signed-off-by: Jason Stephenson Signed-off-by: Jason Boyer --- Open-ILS/src/sql/Pg/300.schema.staged_search.sql | 14 +- Open-ILS/src/sql/Pg/990.schema.unapi.sql | 2 +- .../XXXX.schema.function-fixes-for-string-agg.sql | 219 +++++++++++++++++++++ 3 files changed, 227 insertions(+), 8 deletions(-) create mode 100644 Open-ILS/src/sql/Pg/upgrade/XXXX.schema.function-fixes-for-string-agg.sql diff --git a/Open-ILS/src/sql/Pg/300.schema.staged_search.sql b/Open-ILS/src/sql/Pg/300.schema.staged_search.sql index 2455aadd76..9533dcc80c 100644 --- a/Open-ILS/src/sql/Pg/300.schema.staged_search.sql +++ b/Open-ILS/src/sql/Pg/300.schema.staged_search.sql @@ -350,25 +350,25 @@ CREATE TRIGGER z_opac_vis_mat_view_tgr AFTER INSERT OR UPDATE ON asset.copy FOR CREATE TRIGGER z_opac_vis_mat_view_tgr AFTER INSERT OR UPDATE ON serial.unit FOR EACH ROW EXECUTE PROCEDURE asset.cache_copy_visibility(); CREATE OR REPLACE FUNCTION asset.all_visible_flags () RETURNS TEXT AS $f$ - SELECT '(' || ARRAY_TO_STRING(ARRAY_AGG(search.calculate_visibility_attribute(1 << x, 'copy_flags')),'&') || ')' + SELECT '(' || STRING_AGG(search.calculate_visibility_attribute(1 << x, 'copy_flags')::TEXT,'&') || ')' FROM GENERATE_SERIES(0,0) AS x; -- increment as new flags are added. $f$ LANGUAGE SQL STABLE; CREATE OR REPLACE FUNCTION asset.visible_orgs (otype TEXT) RETURNS TEXT AS $f$ - SELECT '(' || ARRAY_TO_STRING(ARRAY_AGG(search.calculate_visibility_attribute(id, $1)),'|') || ')' + SELECT '(' || STRING_AGG(search.calculate_visibility_attribute(id, $1)::TEXT,'|') || ')' FROM actor.org_unit WHERE opac_visible; $f$ LANGUAGE SQL STABLE; CREATE OR REPLACE FUNCTION asset.invisible_orgs (otype TEXT) RETURNS TEXT AS $f$ - SELECT '!(' || ARRAY_TO_STRING(ARRAY_AGG(search.calculate_visibility_attribute(id, $1)),'|') || ')' + SELECT '!(' || STRING_AGG(search.calculate_visibility_attribute(id, $1)::TEXT,'|') || ')' FROM actor.org_unit WHERE NOT opac_visible; $f$ LANGUAGE SQL STABLE; -- Bib-oriented defaults for search CREATE OR REPLACE FUNCTION asset.bib_source_default () RETURNS TEXT AS $f$ - SELECT '(' || ARRAY_TO_STRING(ARRAY_AGG(search.calculate_visibility_attribute(id, 'bib_source')),'|') || ')' + SELECT '(' || STRING_AGG(search.calculate_visibility_attribute(id, 'bib_source')::TEXT,'|') || ')' FROM config.bib_source WHERE transcendant; $f$ LANGUAGE SQL IMMUTABLE; @@ -381,20 +381,20 @@ $f$ LANGUAGE SQL STABLE; CREATE OR REPLACE FUNCTION asset.location_group_default () RETURNS TEXT AS $f$ SELECT '!()'::TEXT; -- For now, as there's no way to cause a location group to hide all copies. /* - SELECT '!(' || ARRAY_TO_STRING(ARRAY_AGG(search.calculate_visibility_attribute(id, 'location_group')),'|') || ')' + SELECT '!(' || STRING_AGG(search.calculate_visibility_attribute(id, 'location_group')::TEXT,'|') || ')' FROM asset.copy_location_group WHERE NOT opac_visible; */ $f$ LANGUAGE SQL IMMUTABLE; CREATE OR REPLACE FUNCTION asset.location_default () RETURNS TEXT AS $f$ - SELECT '!(' || ARRAY_TO_STRING(ARRAY_AGG(search.calculate_visibility_attribute(id, 'location')),'|') || ')' + SELECT '!(' || STRING_AGG(search.calculate_visibility_attribute(id, 'location')::TEXT,'|') || ')' FROM asset.copy_location WHERE NOT opac_visible; $f$ LANGUAGE SQL STABLE; CREATE OR REPLACE FUNCTION asset.status_default () RETURNS TEXT AS $f$ - SELECT '!(' || ARRAY_TO_STRING(ARRAY_AGG(search.calculate_visibility_attribute(id, 'status')),'|') || ')' + SELECT '!(' || STRING_AGG(search.calculate_visibility_attribute(id, 'status')::TEXT,'|') || ')' FROM config.copy_status WHERE NOT opac_visible; $f$ LANGUAGE SQL STABLE; diff --git a/Open-ILS/src/sql/Pg/990.schema.unapi.sql b/Open-ILS/src/sql/Pg/990.schema.unapi.sql index 241f82904b..167c42ae66 100644 --- a/Open-ILS/src/sql/Pg/990.schema.unapi.sql +++ b/Open-ILS/src/sql/Pg/990.schema.unapi.sql @@ -1711,7 +1711,7 @@ BEGIN parts := parts || xpath(sub_xpath, subrec.marc::XML)::TEXT[]; END LOOP; - SELECT ARRAY_TO_STRING( ARRAY_AGG( DISTINCT p ), '' )::XML INTO subxml FROM UNNEST(parts) p; + SELECT STRING_AGG( DISTINCT p , '' )::XML INTO subxml FROM UNNEST(parts) p; -- append data from the subordinate records to the -- main record document before applying the XSLT diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.function-fixes-for-string-agg.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.function-fixes-for-string-agg.sql new file mode 100644 index 0000000000..2078cd8d9d --- /dev/null +++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.function-fixes-for-string-agg.sql @@ -0,0 +1,219 @@ +BEGIN; + +--SELECT evergreen.upgrade_deps_block_check('XXXX', :eg_version); + +-- replace functions from 300.schema.staged_search.sql + +CREATE OR REPLACE FUNCTION asset.all_visible_flags () RETURNS TEXT AS $f$ + SELECT '(' || STRING_AGG(search.calculate_visibility_attribute(1 << x, 'copy_flags')::TEXT,'&') || ')' + FROM GENERATE_SERIES(0,0) AS x; -- increment as new flags are added. +$f$ LANGUAGE SQL STABLE; + +CREATE OR REPLACE FUNCTION asset.visible_orgs (otype TEXT) RETURNS TEXT AS $f$ + SELECT '(' || STRING_AGG(search.calculate_visibility_attribute(id, $1)::TEXT,'|') || ')' + FROM actor.org_unit + WHERE opac_visible; +$f$ LANGUAGE SQL STABLE; + +CREATE OR REPLACE FUNCTION asset.invisible_orgs (otype TEXT) RETURNS TEXT AS $f$ + SELECT '!(' || STRING_AGG(search.calculate_visibility_attribute(id, $1)::TEXT,'|') || ')' + FROM actor.org_unit + WHERE NOT opac_visible; +$f$ LANGUAGE SQL STABLE; + +CREATE OR REPLACE FUNCTION asset.bib_source_default () RETURNS TEXT AS $f$ + SELECT '(' || STRING_AGG(search.calculate_visibility_attribute(id, 'bib_source')::TEXT,'|') || ')' + FROM config.bib_source + WHERE transcendant; +$f$ LANGUAGE SQL IMMUTABLE; + +CREATE OR REPLACE FUNCTION asset.location_group_default () RETURNS TEXT AS $f$ + SELECT '!()'::TEXT; -- For now, as there's no way to cause a location group to hide all copies. +/* + SELECT '!(' || STRING_AGG(search.calculate_visibility_attribute(id, 'location_group')::TEXT,'|') || ')' + FROM asset.copy_location_group + WHERE NOT opac_visible; +*/ +$f$ LANGUAGE SQL IMMUTABLE; + +CREATE OR REPLACE FUNCTION asset.location_default () RETURNS TEXT AS $f$ + SELECT '!(' || STRING_AGG(search.calculate_visibility_attribute(id, 'location')::TEXT,'|') || ')' + FROM asset.copy_location + WHERE NOT opac_visible; +$f$ LANGUAGE SQL STABLE; + +CREATE OR REPLACE FUNCTION asset.status_default () RETURNS TEXT AS $f$ + SELECT '!(' || STRING_AGG(search.calculate_visibility_attribute(id, 'status')::TEXT,'|') || ')' + FROM config.copy_status + WHERE NOT opac_visible; +$f$ LANGUAGE SQL STABLE; + +-- replace unapi.mmr from 990.schema.unapi.sql + +CREATE OR REPLACE FUNCTION unapi.mmr ( + obj_id BIGINT, + format TEXT, + ename TEXT, + includes TEXT[], + org TEXT, + depth INT DEFAULT NULL, + slimit HSTORE DEFAULT NULL, + soffset HSTORE DEFAULT NULL, + include_xmlns BOOL DEFAULT TRUE, + pref_lib INT DEFAULT NULL +) +RETURNS XML AS $F$ +DECLARE + mmrec metabib.metarecord%ROWTYPE; + leadrec biblio.record_entry%ROWTYPE; + subrec biblio.record_entry%ROWTYPE; + layout unapi.bre_output_layout%ROWTYPE; + xfrm config.xml_transform%ROWTYPE; + ouid INT; + xml_buf TEXT; -- growing XML document + tmp_xml TEXT; -- single-use XML string + xml_frag TEXT; -- single-use XML fragment + top_el TEXT; + output XML; + hxml XML; + axml XML; + subxml XML; -- subordinate records elements + sub_xpath TEXT; + parts TEXT[]; +BEGIN + + -- xpath for extracting bre.marc values from subordinate records + -- so they may be appended to the MARC of the master record prior + -- to XSLT processing. + -- subjects, isbn, issn, upc -- anything else? + sub_xpath := + '//*[starts-with(@tag, "6") or @tag="020" or @tag="022" or @tag="024"]'; + + IF org = '-' OR org IS NULL THEN + SELECT shortname INTO org FROM evergreen.org_top(); + END IF; + + SELECT id INTO ouid FROM actor.org_unit WHERE shortname = org; + + IF ouid IS NULL THEN + RETURN NULL::XML; + END IF; + + SELECT INTO mmrec * FROM metabib.metarecord WHERE id = obj_id; + IF NOT FOUND THEN + RETURN NULL::XML; + END IF; + + -- TODO: aggregate holdings from constituent records + IF format = 'holdings_xml' THEN -- the special case + output := unapi.mmr_holdings_xml( + obj_id, ouid, org, depth, + evergreen.array_remove_item_by_value(includes,'holdings_xml'), + slimit, soffset, include_xmlns, pref_lib); + RETURN output; + END IF; + + SELECT * INTO layout FROM unapi.bre_output_layout WHERE name = format; + + IF layout.name IS NULL THEN + RETURN NULL::XML; + END IF; + + SELECT * INTO xfrm FROM config.xml_transform WHERE name = layout.transform; + + SELECT INTO leadrec * FROM biblio.record_entry WHERE id = mmrec.master_record; + + -- Grab distinct MVF for all records if requested + IF ('mra' = ANY (includes)) THEN + axml := unapi.mmr_mra(obj_id,NULL,NULL,NULL,org,depth,NULL,NULL,TRUE,pref_lib); + ELSE + axml := NULL::XML; + END IF; + + xml_buf = leadrec.marc; + + hxml := NULL::XML; + IF ('holdings_xml' = ANY (includes)) THEN + hxml := unapi.mmr_holdings_xml( + obj_id, ouid, org, depth, + evergreen.array_remove_item_by_value(includes,'holdings_xml'), + slimit, soffset, include_xmlns, pref_lib); + END IF; + + subxml := NULL::XML; + parts := '{}'::TEXT[]; + FOR subrec IN SELECT bre.* FROM biblio.record_entry bre + JOIN metabib.metarecord_source_map mmsm ON (mmsm.source = bre.id) + JOIN metabib.metarecord mmr ON (mmr.id = mmsm.metarecord) + WHERE mmr.id = obj_id AND NOT bre.deleted + ORDER BY CASE WHEN bre.id = mmr.master_record THEN 0 ELSE bre.id END + LIMIT COALESCE((slimit->'bre')::INT, 5) LOOP + + IF subrec.id = leadrec.id THEN CONTINUE; END IF; + -- Append choice data from the the non-lead records to the + -- the lead record document + + parts := parts || xpath(sub_xpath, subrec.marc::XML)::TEXT[]; + END LOOP; + + SELECT STRING_AGG( DISTINCT p , '' )::XML INTO subxml FROM UNNEST(parts) p; + + -- append data from the subordinate records to the + -- main record document before applying the XSLT + + IF subxml IS NOT NULL THEN + xml_buf := REGEXP_REPLACE(xml_buf, + '(.*?)$', subxml || '' || E'\\1'); + END IF; + + IF format = 'marcxml' THEN + -- If we're not using the prefixed namespace in + -- this record, then remove all declarations of it + IF xml_buf !~ E' mods) + top_el := REGEXP_REPLACE(xml_buf, E'^.*?<((?:\\S+:)?' || + layout.holdings_element || ').*$', E'\\1'); + + IF axml IS NOT NULL THEN + xml_buf := REGEXP_REPLACE(xml_buf, + '(.*?)$', axml || '\\1'); + END IF; + + IF hxml IS NOT NULL THEN + xml_buf := REGEXP_REPLACE(xml_buf, + '(.*?)$', hxml || '\\1'); + END IF; + + IF ('mmr.unapi' = ANY (includes)) THEN + output := REGEXP_REPLACE( + xml_buf, + '(.*?)', + XMLELEMENT( + name abbr, + XMLATTRIBUTES( + 'http://www.w3.org/1999/xhtml' AS xmlns, + 'unapi-id' AS class, + 'tag:open-ils.org:U2@mmr/' || obj_id || '/' || org AS title + ) + )::TEXT || '\\1' + ); + ELSE + output := xml_buf; + END IF; + + -- remove ignorable whitesace + output := REGEXP_REPLACE(output::TEXT,E'>\\s+<','><','gs')::XML; + RETURN output; +END; +$F$ LANGUAGE PLPGSQL STABLE; + +COMMIT; + -- 2.11.0