LP#1846354 various speed improvements

* Adjust upgrade script for speed

This commit makes minor adjustments to the upgrade script to speed it up
in the face of larger data sets.

NOTE: The DO blocks that provided assertion tests during the the upgrade
have been commented out, as they primarily test that the preceding
insert did not break in some way that would have inserted data from the
wrong table into the intermediate staging tables.  They take a very long
time to run in some cases, but are left here for testers to enable in
order to confirm that they would pass in production.

* Move the deleted filter out of the query for speed

* Rework aum and ausp to use the same sequence, and aump view to use UNION ALL

Signed-off-by: Mike Rylander <mrylander@gmail.com>
Signed-off-by: Jason Etheridge <jason@EquinoxInitiative.org>
Signed-off-by: Terran McCanna <tmccanna@georgialibraries.org>
Signed-off-by: Galen Charlton <gmc@equinoxinitiative.org>
Signed-off-by: Chris Sharp <csharp@georgialibraries.org>

diff --git a/Open-ILS/examples/fm_IDL.xml b/Open-ILS/examples/fm_IDL.xml
index dc8e55d..c3571d9 100644 (file)
--- a/Open-ILS/examples/fm_IDL.xml
+++ b/Open-ILS/examples/fm_IDL.xml
@@ -4681,7 +4681,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
         </permacrud>
        </class>
        <class id="ausp" controller="open-ils.cstore open-ils.pcrud" oils_obj:fieldmapper="actor::user_standing_penalty" oils_persist:tablename="actor.usr_standing_penalty" oils_persist:restrict_primary="100" reporter:label="User Standing Penalty">
-               <fields oils_persist:primary="id" oils_persist:sequence="actor.usr_standing_penalty_id_seq">
+               <fields oils_persist:primary="id" oils_persist:sequence="actor.usr_message_id_seq">
                        <field name="id" reporter:datatype="id" reporter:label="ID" />
                        <field name="set_date" reporter:datatype="timestamp" reporter:label="Set Date"/>
                        <field name="usr" reporter:datatype="link" reporter:label="User"/>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Actor.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Actor.pm
index 8681e5d..1464f11 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Actor.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Actor.pm
@@ -3383,18 +3383,21 @@ sub new_flesh_user {
     }
 
     if($fetch_notes) {
-        # grab undeleted notes (now actor.usr_message_penalty) that have not hit their stop_date
-        $user->notes(
-            $e->search_actor_usr_message_penalty([
+        # grab notes (now actor.usr_message_penalty) that have not hit their stop_date
+        # NOTE: This is a view that already filters out deleted messages that are not
+        # attached to a penalty, but the query is slow if we include deleted=f, so we
+        # post-filter that.  This counts both user messages and standing penalties, but
+        # linked ones are only counted once.
+        $user->notes([
+            grep { !$_->deleted or $_->deleted eq 'f' } @{ $e->search_actor_usr_message_penalty([
                 {   usr => $id,
-                    deleted => 'f',
                     '-or' => [
                         {stop_date => undef},
                         {stop_date => {'>' => 'now'}}
                     ],
                 }, {}
-            ])
-        );
+            ]) }
+        ]);
     }
 
     # retrieve the most recent usr_activity entry

diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.note_and_message_consolidation b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.note_and_message_consolidation
index 5da0cac..3eb444f 100644 (file)
--- a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.note_and_message_consolidation
+++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.note_and_message_consolidation
@@ -2,6 +2,15 @@ BEGIN;
 
 SELECT evergreen.upgrade_deps_block_check('XXXX', :eg_version);
 
+ALTER TABLE actor.usr_standing_penalty ALTER COLUMN id SET DEFAULT nextval('actor.usr_message_id_seq'::regclass);
+
+UPDATE actor.usr_standing_penalty SET id = id * -1; -- move them out of the way to avoid mid-statement collisions
+
+WITH messages AS ( SELECT MAX(id) AS max_id FROM actor.usr_message )
+UPDATE actor.usr_standing_penalty SET id = id * -1 + messages.max_id FROM messages;
+
+SELECT SETVAL('actor.usr_message_id_seq'::regclass, COALESCE((SELECT MAX(id) FROM actor.usr_standing_penalty) + 1, 1), FALSE);
+
 ALTER TABLE actor.usr_message ADD COLUMN pub BOOL NOT NULL DEFAULT FALSE;
 ALTER TABLE actor.usr_message ADD COLUMN stop_date TIMESTAMP WITH TIME ZONE;
 ALTER TABLE actor.usr_message ADD COLUMN editor        BIGINT REFERENCES actor.usr (id);
@@ -12,6 +21,7 @@ CREATE VIEW actor.usr_message_limited
 AS SELECT * FROM actor.usr_message WHERE pub AND NOT deleted;
 
 ALTER TABLE actor.usr_standing_penalty ADD COLUMN usr_message BIGINT REFERENCES actor.usr_message(id);
+CREATE INDEX usr_standing_penalty_usr_message_idx ON actor.usr_standing_penalty (usr_message);
 
 -- alright, let's set all existing user messages to public
 
@@ -27,7 +37,7 @@ CREATE TABLE actor.XXXX_penalty_notes AS
 -- here is our staging table which will be shaped exactly like
 -- actor.usr_message and use the same id sequence
 CREATE TABLE actor.XXXX_usr_message_for_penalty_notes (
-    LIKE actor.usr_message INCLUDING DEFAULTS EXCLUDING CONSTRAINTS
+    LIKE actor.usr_message INCLUDING DEFAULTS 
 );
 
 INSERT INTO actor.XXXX_usr_message_for_penalty_notes (
@@ -71,7 +81,7 @@ WHERE
 
 -- probably redundant here, but the spec calls for an assertion before removing
 -- the note column from actor.usr_standing_penalty, so being extra cautious:
-
+/*
 do $$ begin
     assert (
         select count(*)
@@ -81,29 +91,30 @@ do $$ begin
         )
     ) = 0, 'failed migrating to actor.usr_message';
 end; $$;
+*/
 
 ALTER TABLE actor.usr_standing_penalty DROP COLUMN note;
 
 -- combined view of actor.usr_standing_penalty and actor.usr_message for populating
 -- staff Notes (formerly Messages) interface
 
-CREATE VIEW actor.usr_message_penalty
-AS SELECT
-    COALESCE(ausp.id::TEXT,'') || ':' || COALESCE(aum.id::TEXT,'') AS "id",
+CREATE VIEW actor.usr_message_penalty AS
+SELECT -- ausp with or without messages
+    ausp.id AS "id",
     ausp.id AS "ausp_id",
     aum.id AS "aum_id",
-    COALESCE(ausp.org_unit,aum.sending_lib) AS "org_unit",
+    ausp.org_unit AS "org_unit",
     ausp.org_unit AS "ausp_org_unit",
     aum.sending_lib AS "aum_sending_lib",
-    COALESCE(ausp.usr,aum.usr) AS "usr",
+    ausp.usr AS "usr",
     ausp.usr as "ausp_usr",
     aum.usr as "aum_usr",
     ausp.standing_penalty AS "standing_penalty",
     ausp.staff AS "staff",
-    LEAST(ausp.set_date,aum.create_date) AS "create_date",
+    ausp.set_date AS "create_date",
     ausp.set_date AS "ausp_set_date",
     aum.create_date AS "aum_create_date",
-    LEAST(ausp.stop_date,aum.stop_date) AS "stop_date",
+    ausp.stop_date AS "stop_date",
     ausp.stop_date AS "ausp_stop_date",
     aum.stop_date AS "aum_stop_date",
     ausp.usr_message AS "ausp_usr_message",
@@ -116,13 +127,38 @@ AS SELECT
     aum.edit_date AS "edit_date"
 FROM
     actor.usr_standing_penalty ausp
-FULL OUTER JOIN
+    LEFT JOIN actor.usr_message aum ON (ausp.usr_message = aum.id AND NOT aum.deleted)
+        UNION ALL
+SELECT -- aum without penalties
+    aum.id AS "id",
+    NULL::INT AS "ausp_id",
+    aum.id AS "aum_id",
+    aum.sending_lib AS "org_unit",
+    NULL::INT AS "ausp_org_unit",
+    aum.sending_lib AS "aum_sending_lib",
+    aum.usr AS "usr",
+    NULL::INT as "ausp_usr",
+    aum.usr as "aum_usr",
+    NULL::INT AS "standing_penalty",
+    NULL::INT AS "staff",
+    aum.create_date AS "create_date",
+    NULL::TIMESTAMPTZ AS "ausp_set_date",
+    aum.create_date AS "aum_create_date",
+    aum.stop_date AS "stop_date",
+    NULL::TIMESTAMPTZ AS "ausp_stop_date",
+    aum.stop_date AS "aum_stop_date",
+    NULL::INT AS "ausp_usr_message",
+    aum.title AS "title",
+    aum.message AS "message",
+    aum.deleted AS "deleted",
+    aum.read_date AS "read_date",
+    aum.pub AS "pub",
+    aum.editor AS "editor",
+    aum.edit_date AS "edit_date"
+FROM
     actor.usr_message aum
-ON (
-    ausp.usr_message = aum.id
-)
-WHERE
-    NOT (ausp.id IS NULL AND aum.deleted);
+    LEFT JOIN actor.usr_standing_penalty ausp ON (ausp.usr_message = aum.id)
+WHERE NOT aum.deleted AND ausp.id IS NULL
 ;
 
 -- fun part where we migrate the following alert messages:
@@ -135,7 +171,7 @@ CREATE TABLE actor.XXXX_note_and_message_consolidation AS
 -- here is our staging table which will be shaped exactly like
 -- actor.usr_message and use the same id sequence
 CREATE TABLE actor.XXXX_usr_message (
-    LIKE actor.usr_message INCLUDING DEFAULTS EXCLUDING CONSTRAINTS
+    LIKE actor.usr_message INCLUDING DEFAULTS 
 );
 
 INSERT INTO actor.XXXX_usr_message (
@@ -158,7 +194,7 @@ FROM
 
 -- another staging table, but for actor.usr_standing_penalty
 CREATE TABLE actor.XXXX_usr_standing_penalty (
-    LIKE actor.usr_standing_penalty INCLUDING DEFAULTS EXCLUDING CONSTRAINTS
+    LIKE actor.usr_standing_penalty INCLUDING DEFAULTS 
 );
 
 INSERT INTO actor.XXXX_usr_standing_penalty (
@@ -188,7 +224,7 @@ INSERT INTO actor.usr_standing_penalty
 
 -- probably redundant here, but the spec calls for an assertion before removing
 -- the alert message column from actor.usr, so being extra cautious:
-
+/*
 do $$ begin
     assert (
         select count(*)
@@ -208,6 +244,7 @@ do $$ begin
         )
     ) = 0, 'failed migrating to actor.usr_standing_penalty';
 end; $$;
+*/
 
 -- WARNING: we're going to lose the history of alert_message
 ALTER TABLE actor.usr DROP COLUMN alert_message CASCADE;
@@ -220,7 +257,7 @@ SELECT auditor.update_auditors();
 -- for the note contents.
 
 CREATE TABLE actor.XXXX_usr_message_for_private_notes (
-    LIKE actor.usr_message INCLUDING DEFAULTS EXCLUDING CONSTRAINTS
+    LIKE actor.usr_message INCLUDING DEFAULTS 
 );
 
 INSERT INTO actor.XXXX_usr_message_for_private_notes (
@@ -244,7 +281,7 @@ WHERE
 ;
 
 CREATE TABLE actor.XXXX_usr_message_for_unmatched_public_notes (
-    LIKE actor.usr_message INCLUDING DEFAULTS EXCLUDING CONSTRAINTS
+    LIKE actor.usr_message INCLUDING DEFAULTS 
 );
 
 INSERT INTO actor.XXXX_usr_message_for_unmatched_public_notes (
@@ -275,7 +312,7 @@ WHERE
 --  3) usr_note and usr_message entries that can be matched
 
 CREATE TABLE actor.XXXX_usr_standing_penalties_for_notes (
-    LIKE actor.usr_standing_penalty INCLUDING DEFAULTS EXCLUDING CONSTRAINTS
+    LIKE actor.usr_standing_penalty INCLUDING DEFAULTS 
 );
 
 --  1) actor.XXXX_usr_message_for_private_notes and associated usr_note entries
@@ -344,14 +381,11 @@ INSERT INTO actor.XXXX_usr_standing_penalties_for_notes (
     m.stop_date,
     m.id
 FROM
-    actor.usr_note n,
-    actor.usr_message m
+    actor.usr_note n
+    JOIN actor.usr_message m ON (n.usr = m.usr AND n.create_date = m.create_date)
 WHERE
-    n.usr = m.usr AND n.create_date = m.create_date AND m.id NOT IN (
-        SELECT id FROM actor.XXXX_usr_message_for_private_notes
-        UNION
-        SELECT id FROM actor.XXXX_usr_message_for_unmatched_public_notes
-    )
+    NOT EXISTS ( SELECT 1 FROM actor.XXXX_usr_message_for_private_notes WHERE id = m.id )
+    AND NOT EXISTS ( SELECT 1 FROM actor.XXXX_usr_message_for_unmatched_public_notes WHERE id = m.id )
 ;
 
 -- so far so good, let's push these into production
@@ -364,7 +398,7 @@ INSERT INTO actor.usr_standing_penalty
 
 -- probably redundant here, but the spec calls for an assertion before dropping
 -- the actor.usr_note table, so being extra cautious:
-
+/*
 do $$ begin
     assert (
         select count(*)
@@ -374,8 +408,9 @@ do $$ begin
         )
     ) = 0, 'failed migrating to actor.usr_message';
 end; $$;
+*/
 
-DROP TABLE actor.usr_note;
+DROP TABLE actor.usr_note CASCADE;
 
 -- preserve would-be collisions for migrating
 -- ui.staff.require_initials.patron_info_notes

diff --git a/docs/RELEASE_NOTES_NEXT/Client/lp1846354_consolidate_patron_notes.adoc b/docs/RELEASE_NOTES_NEXT/Client/lp1846354_consolidate_patron_notes.adoc
index 0d2f9ca..9c54ec4 100644 (file)
--- a/docs/RELEASE_NOTES_NEXT/Client/lp1846354_consolidate_patron_notes.adoc
+++ b/docs/RELEASE_NOTES_NEXT/Client/lp1846354_consolidate_patron_notes.adoc
@@ -12,7 +12,7 @@ And for actor.usr_message, there is now both a pub column and a deleted column.
 
 Depending on privacy policies, system adminstrators may wish to set up a recurring process to truly delete older entries in actor.usr_message that have been flagged as deleted.
 
-WARNING: The upgrade script will remote the alert_message field from the auditor table, so if you care about preserving those you should run a query to create a backup.
+WARNING: The upgrade script will remove the alert_message field from the auditor table, so if you care about preserving those you should run a query to create a backup.
 
 For example: