[open-ils-commits] [GIT] Evergreen ILS branch master updated. d46585ed0e2816cd641242bfb292f0869f01950c
Evergreen Git
git at git.evergreen-ils.org
Fri Apr 19 16:20:18 EDT 2013
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Evergreen ILS".
The branch, master has been updated
via d46585ed0e2816cd641242bfb292f0869f01950c (commit)
via 036232ce17a3ff957cd33e77f664e02f61f2c80d (commit)
from ec205de7f1c4d703331ac5deb1401e45b9b2f020 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit d46585ed0e2816cd641242bfb292f0869f01950c
Author: Mike Rylander <mrylander at gmail.com>
Date: Fri Apr 19 16:19:11 2013 -0400
Stamping best-hold-order update script
Signed-off-by: Mike Rylander <mrylander at gmail.com>
diff --git a/Open-ILS/src/sql/Pg/002.schema.config.sql b/Open-ILS/src/sql/Pg/002.schema.config.sql
index b9068db..5a955d4 100644
--- a/Open-ILS/src/sql/Pg/002.schema.config.sql
+++ b/Open-ILS/src/sql/Pg/002.schema.config.sql
@@ -91,7 +91,7 @@ CREATE TRIGGER no_overlapping_deps
BEFORE INSERT OR UPDATE ON config.db_patch_dependencies
FOR EACH ROW EXECUTE PROCEDURE evergreen.array_overlap_check ('deprecates');
-INSERT INTO config.upgrade_log (version, applied_to) VALUES ('0791', :eg_version); -- senator/miker
+INSERT INTO config.upgrade_log (version, applied_to) VALUES ('0793', :eg_version); -- senator/miker
CREATE TABLE config.bib_source (
id SERIAL PRIMARY KEY,
diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.data.best-hold-order-traditional-approx.sql b/Open-ILS/src/sql/Pg/upgrade/0793.data.best-hold-order-traditional-approx.sql
similarity index 94%
rename from Open-ILS/src/sql/Pg/upgrade/XXXX.data.best-hold-order-traditional-approx.sql
rename to Open-ILS/src/sql/Pg/upgrade/0793.data.best-hold-order-traditional-approx.sql
index ddd525d..01db997 100644
--- a/Open-ILS/src/sql/Pg/upgrade/XXXX.data.best-hold-order-traditional-approx.sql
+++ b/Open-ILS/src/sql/Pg/upgrade/0793.data.best-hold-order-traditional-approx.sql
@@ -1,6 +1,6 @@
BEGIN;
--- INSERT INTO config.upgrade_log (version) VALUES ('XXXX');
+SELECT evergreen.upgrade_deps_block_check('0792', :eg_version);
UPDATE config.best_hold_order
SET
commit 036232ce17a3ff957cd33e77f664e02f61f2c80d
Author: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
Date: Wed Apr 3 16:35:51 2013 -0400
Fix various Traditional and holds-go-home best-hold sort orders
Use copy's call number's owning_lib instead of copy's circ_lib
Should compare checkin lib to copy's (call number's) owning_lib, not
hold request lib.
You might think the comparison should be to acp.circ_lib, but that
doesn't work with floating copies (for non-floaters, acp.circ_lib
should be equal to acp.call_number.owning_lib).
approx is a more correct first determinant to give the behavior sites
are used to.
hprox can cause copies to be too eager to go home when
there are holds with that copy's circ lib as its request lib (if that's
what you want, then you do pick or create a sort-order with hprox near
the top).
Address a problem in the copy_has_not_been_home CTE.
This expression was always meant to provide a TRUE or FALSE value as its
lone result, but would return NULL in cases where copies had no transit
history.
Use pickup_lib, not request_lib, as the determinant of
nearness-to-home. request_lib was used with the thinking that an item's
"owning" patrons should have their wishes favored at holds-go-home time,
even if where they wanted to send the copy was not actually home, but
that's neither necessarily desired nor very intuitive.
Clear up holds-go-home logic with better code AND add TechRef
documentation with diagram in attempt to be as clear as possible.
Signed-off-by: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
Signed-off-by: Mike Rylander <mrylander at gmail.com>
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm
index 9ba8272..fc43e1d 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm
@@ -3007,6 +3007,10 @@ sub find_nearest_permitted_hold {
my $fifo = $U->ou_ancestor_setting_value($user->ws_ou, 'circ.holds_fifo');
+ # the nearest_hold API call now needs this
+ $copy->call_number($editor->retrieve_asset_call_number($copy->call_number))
+ unless ref $copy->call_number;
+
# search for what should be the best holds for this copy to fulfill
my $best_holds = $U->storagereq(
"open-ils.storage.action.hold_request.nearest_hold.atomic",
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm
index baa7098..f18a254 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm
@@ -17,10 +17,12 @@ use OpenILS::Application::Circ::CircCommon;
use OpenILS::Application::AppUtils;
my $U = "OpenILS::Application::AppUtils";
-# used in build_hold_sort_clause()
+# Used in build_hold_sort_clause(). See the hash %order_by_sprintf_args in
+# that sub to confirm what gets used to replace the formatters, and see
+# nearest_hold() for the main body of the SQL query that these go into.
my %HOLD_SORT_ORDER_BY = (
pprox => 'p.prox',
- hprox => 'actor.org_unit_proximity(%d, h.request_lib)', # $cp->circ_lib
+ hprox => 'actor.org_unit_proximity(%d, h.pickup_lib)', # $cp->call_number->owning_lib
aprox => 'COALESCE(hm.proximity, p.prox)',
approx => 'action.hold_copy_calculated_proximity(h.id, %d, %d)', # $cp,$here
priority => 'pgt.hold_priority',
@@ -29,18 +31,20 @@ my %HOLD_SORT_ORDER_BY = (
rtime => 'h.request_time',
htime => q!
CASE WHEN
+ last_event_on_copy.place <> %d AND
copy_has_not_been_home.result
- THEN actor.org_unit_proximity(%d, h.request_lib)
+ THEN actor.org_unit_proximity(%d, h.pickup_lib)
ELSE 999
END
- !,
+ !, # $cp->call_number->owning_lib x 2
shtime => q!
CASE WHEN
+ last_event_on_copy.place <> %d AND
copy_has_not_been_home_even_to_idle.result
- THEN actor.org_unit_proximity(%d, h.request_lib)
+ THEN actor.org_unit_proximity(%d, h.pickup_lib)
ELSE 999
END
- !,
+ !, # $cp->call_number->owning_lib x 2
);
@@ -349,10 +353,10 @@ sub build_hold_sort_clause {
my ($columns, $cp, $here) = @_;
my %order_by_sprintf_args = (
- hprox => [$cp->circ_lib],
+ hprox => [$cp->call_number->owning_lib],
approx => [$cp->id, $here],
- htime => [$cp->circ_lib],
- shtime => [$cp->circ_lib]
+ htime => [$cp->call_number->owning_lib, $cp->call_number->owning_lib],
+ shtime => [$cp->call_number->owning_lib, $cp->call_number->owning_lib]
);
my @clauses;
@@ -374,16 +378,69 @@ sub build_hold_sort_clause {
# more order-by clauses after that.
}
- my ($ctes, $joins);
+ my ($ctes, $joins) = ("", "");
if ($ctes_needed >= 1) {
- # For our first auxiliary query, the question we seek to answer is, "has
- # our copy been circulating away from home too long?" Two parts to
- # answer this question.
+ # Each CTE serves the next. The first is one version or another
+ # of last_event_on_copy, which is described in holds-go-home.txt
+ # TechRef, but it essentially returns place and time of the most
+ # recent transit or circ to do with a copy, and failing that it
+ # returns a synthetic event that means "here" and "now".
+
+ if ($ctes_needed == 2) {
+ $ctes .= sprintf(q!
+, last_event_on_copy AS ( -- combined circ and transit version
+ SELECT *
+ FROM (
+ ( SELECT
+ TRUE AS concrete,
+ dest AS place,
+ COALESCE(dest_recv_time, source_send_time) AS moment
+ FROM action.transit_copy
+ WHERE target_copy = %d
+ ORDER BY moment DESC LIMIT 1
+ ) UNION (
+ SELECT
+ TRUE AS concrete,
+ COALESCE(checkin_lib, circ_lib) AS place,
+ COALESCE(checkin_time, xact_start) AS moment
+ FROM action.circulation
+ WHERE target_copy = %d
+ ORDER BY moment DESC LIMIT 1
+ ) UNION
+ SELECT
+ FALSE AS concrete,
+ %d AS place,
+ NOW() AS moment
+ ) x ORDER BY concrete DESC, moment DESC LIMIT 1
+) !, $cp->id, $cp->id, $cp->call_number->owning_lib);
+ } else {
+ $ctes .= sprintf(q!
+, last_event_on_copy AS ( -- circ only version
+ SELECT * FROM (
+ ( SELECT
+ TRUE AS concrete,
+ COALESCE(checkin_lib, circ_lib) AS place,
+ COALESCE(checkin_time, xact_start) AS moment
+ FROM action.circulation
+ WHERE target_copy = %d
+ ORDER BY moment DESC LIMIT 1
+ ) UNION SELECT
+ FALSE AS concrete,
+ %d AS place,
+ NOW() AS moment
+ ) x ORDER BY concrete DESC, moment DESC LIMIT 1
+) !, $cp->id, $cp->call_number->owning_lib);
+ }
+
+ $joins .= q!
+ JOIN last_event_on_copy ON (true)
+ !;
+
+ # For our next auxiliary query, the question we seek to answer is,
+ # "has our copy been circulating away from home too long?"
#
- # part 1: Have their been no checkouts at the copy's circ_lib since the
+ # Have there been no checkouts at the copy's circ_lib since the
# beginning of our go-home interval?
- # part 2: Was the last transit to affect our copy before the beginning
- # of our go-home interval an outbound transit? i.e. away from circ-lib
# [We use sprintf because the outer function that's going to send one
# big query through DBI is blind to our process of dynamically building
@@ -400,56 +457,33 @@ sub build_hold_sort_clause {
circ.target_copy = %d AND
circ.circ_lib = %d AND
circ.xact_start >= NOW() - go_home_interval.value
- ) IS NULL AND (
- -- part 2
- SELECT atc.dest <> %d FROM action.transit_copy atc
- JOIN go_home_interval ON (true)
- WHERE
- atc.id = (
- SELECT MAX(id) FROM action.transit_copy atc_inner
- WHERE
- atc_inner.target_copy = %d AND
- atc_inner.source_send_time < NOW() - go_home_interval.value
- )
- ) AS result
-) !, $cp->id, $cp->circ_lib, $cp->circ_lib, $cp->id);
- $joins .= " JOIN copy_has_not_been_home ON (true) ";
+ ) IS NULL AS result
+) !, $cp->id, $cp->circ_lib);
+
+ $joins .= q!
+ JOIN copy_has_not_been_home ON (true)
+ !;
}
if ($ctes_needed == 2) {
- # In this auxiliary query, we ask the question, "has our copy come home
- # by any means that we can determine, even if it didn't circulate once
- # it came home, in the time defined by the go-home-interval?"
- # answer this question. Two parts to this too (besides including the
- # previous auxiliary query).
+ # By this query, we mean to determine that the copy hasn't landed at
+ # home by means of transit during the go-home interval (in addition
+ # to not having circulated from home in the same time frame).
#
- # 1: there have been no homebound transits for this copy since the
- # beginning of the go-home interval.
- # 2: there have been no checkins at home since the beginning of
- # the go-home interval for this copy
+ # There have been no homebound transits that arrived for this copy
+ # since the beginning of the go-home interval.
$ctes .= sprintf(q!
, copy_has_not_been_home_even_to_idle AS (
- SELECT
- copy_has_not_been_home.response AND (
- -- part 1
- SELECT MIN(atc.id) FROM action.transit_copy atc
- JOIN go_home_interval ON (true)
- WHERE
- atc.target_copy = %d AND
- atc.dest = %d AND
- atc.dest_recv_time >= NOW() - go_home_interval.value
- ) IS NULL AND (
- -- part 2
- SELECT MIN(circ.id) FROM action.circulation circ
- JOIN go_home_interval ON (true)
- WHERE
- circ.target_copy = %d AND
- circ.checkin_lib = %d AND
- circ.checkin_time >= NOW() - go_home_interval.value
- ) IS NULL
- AS result
-) !, $cp->id, $cp->circ_lib, $cp->id, $cp->circ_lib);
+ SELECT result AND NOT (
+ SELECT COUNT(*)::INT::BOOL
+ FROM action.transit_copy atc
+ WHERE
+ atc.target_copy = %d AND
+ (atc.dest = %d OR atc.source = %d) AND
+ atc.dest_recv_time >= NOW() - (SELECT value FROM go_home_interval)
+ ) AS result FROM copy_has_not_been_home
+) !, $cp->id, $cp->circ_lib, $cp->circ_lib);
$joins .= " JOIN copy_has_not_been_home_even_to_idle ON (true) ";
}
@@ -464,7 +498,8 @@ sub nearest_hold {
my $self = shift;
my $client = shift;
my $here = shift; # just the ID
- my $cp = shift; # now an object, formerly just the ID
+ my $cp = shift; # now an object with call_number fleshed,
+ # formerly just copy ID
my $limit = int(shift()) || 10;
my $age = shift() || '0 seconds';
my $fifo = shift();
diff --git a/Open-ILS/src/sql/Pg/950.data.seed-values.sql b/Open-ILS/src/sql/Pg/950.data.seed-values.sql
index d693e6c..a8ddf6a 100644
--- a/Open-ILS/src/sql/Pg/950.data.seed-values.sql
+++ b/Open-ILS/src/sql/Pg/950.data.seed-values.sql
@@ -12489,15 +12489,15 @@ INSERT INTO config.org_unit_setting_type (
INSERT INTO config.best_hold_order (
name,
- pprox, aprox, priority, cut, depth, rtime, htime, hprox
+ approx, pprox, aprox, priority, cut, depth, rtime
) VALUES (
'Traditional',
- 1, 2, 3, 4, 5, 6, 7, 8
+ 1, 2, 3, 4, 5, 6, 7
);
INSERT INTO config.best_hold_order (
name,
- hprox, pprox, aprox, priority, cut, depth, rtime, htime
+ hprox, approx, pprox, aprox, priority, cut, depth, rtime
) VALUES (
'Traditional with Holds-always-go-home',
1, 2, 3, 4, 5, 6, 7, 8
@@ -12505,7 +12505,7 @@ INSERT INTO config.best_hold_order (
INSERT INTO config.best_hold_order (
name,
- htime, hprox, pprox, aprox, priority, cut, depth, rtime
+ htime, approx, pprox, aprox, priority, cut, depth, rtime
) VALUES (
'Traditional with Holds-go-home',
1, 2, 3, 4, 5, 6, 7, 8
diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.data.best-hold-order-traditional-approx.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.data.best-hold-order-traditional-approx.sql
new file mode 100644
index 0000000..ddd525d
--- /dev/null
+++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.data.best-hold-order-traditional-approx.sql
@@ -0,0 +1,67 @@
+BEGIN;
+
+-- INSERT INTO config.upgrade_log (version) VALUES ('XXXX');
+
+UPDATE config.best_hold_order
+SET
+ approx = 1,
+ pprox = 2,
+ aprox = 3,
+ priority = 4,
+ cut = 5,
+ depth = 6,
+ rtime = 7,
+ hprox = NULL,
+ htime = NULL
+WHERE name = 'Traditional' AND
+ pprox = 1 AND
+ aprox = 2 AND
+ priority = 3 AND
+ cut = 4 AND
+ depth = 5 AND
+ rtime = 6 ;
+
+UPDATE config.best_hold_order
+SET
+ hprox = 1,
+ approx = 2,
+ pprox = 3,
+ aprox = 4,
+ priority = 5,
+ cut = 6,
+ depth = 7,
+ rtime = 8,
+ htime = NULL
+WHERE name = 'Traditional with Holds-always-go-home' AND
+ hprox = 1 AND
+ pprox = 2 AND
+ aprox = 3 AND
+ priority = 4 AND
+ cut = 5 AND
+ depth = 6 AND
+ rtime = 7 AND
+ htime = 8;
+
+UPDATE config.best_hold_order
+SET
+ htime = 1,
+ approx = 2,
+ pprox = 3,
+ aprox = 4,
+ priority = 5,
+ cut = 6,
+ depth = 7,
+ rtime = 8,
+ hprox = NULL
+WHERE name = 'Traditional with Holds-go-home' AND
+ htime = 1 AND
+ hprox = 2 AND
+ pprox = 3 AND
+ aprox = 4 AND
+ priority = 5 AND
+ cut = 6 AND
+ depth = 7 AND
+ rtime = 8 ;
+
+
+COMMIT;
diff --git a/docs/TechRef/Circ/holds-go-home.txt b/docs/TechRef/Circ/holds-go-home.txt
new file mode 100644
index 0000000..6816574
--- /dev/null
+++ b/docs/TechRef/Circ/holds-go-home.txt
@@ -0,0 +1,93 @@
+Holds Go Home
+=============
+
+Outline
+-------
+
+A copy prefers to fulfill a hold near its home when:
+
+ - The last event for a copy was NOT at home *and* ...
+ - The copy has not circulated from home within the defined period *and* ...
+ - The copy has neither departed from home by transit nor arrived at home
+ by transit within the defined period.
+
+Definitions
+-----------
+
+In the preceding section, some terms are used that want explanation.
+
+_Event_ refers to either a circulation or a transit related to a copy. An
+event has only two qualities we care about: moment and place.
+
+ - When the event comes from a circulation, _moment_ is checkin_time if
+ we have it, otherwise xact_start. When the event comes from a transit,
+ moment is dest_recv_time if we have it, otherwise source_send_time.
+
+ - When the event comes from a circulation, _place_ is checkin_lib if we
+ have it, otherwise circ_lib. When the event comes from a transit,
+ place is dest, *always*.
+
+ - When the copy in question has neither any transit history nor any
+ circulation history, we produce a synthetic event with _moment_ equal
+ to the present time, and _place_ equal to the copy's call number's
+ owning_lib (see 'home' below).
+
+_Home_ is the value of the copy's call number's owning_lib field, which
+incidentally is usually equal to the copy's circ_lib field except where
+floating is in use.
+
+_The defined period_ is the time between the present moment (_NOW()_
+to the database) and the present moment less the value of the interval
+defined in the _circ.hold_go_home_interval_ org unit setting at a scope
+of the copy's _home_. E.g., if the setting contains the string "6 months",
+the defined period is the last 6 months before the present moment, or
+anything greater than _NOW() - '6 months'::INTERVAL_.
+
+Logic
+-----
+
+............................................
+
+ -------
+| Event |
+ -------
+ |
+ |
+ v
+ ------------------------- -----------------------
+| Was last event at home? | -----Yes.-----> | Don't try to go home. |
+ ------------------------- -----------------------
+ | ^ ^
+ | No. | |
+ v | |
+ ------------------------------------------------ | |
+| Did copy circ from home during defined period? |--Yes.--/ |
+ ------------------------------------------------ |
+ | |
+ | No. |
+ | |
+ v |
+ ------------------------------------------------------ |
+| Did copy leave or arrive home during defined period? |--Yes.---/
+ ------------------------------------------------------
+ |
+ | No.
+ |
+ v
+ ----------
+| Go home. |
+ ----------
+
+............................................
+
+
+Implications in Best-Hold Selection Sort Order
+----------------------------------------------
+
+The calculations described above are all embodied in the best-hold selection
+sort order determinant _shtime_.
+
+_htime_ is a simpler version of the same, with all reference to transits
+removed, considering only circulations. This means events become thin
+circulations, the "Did a transit bring copy home..." step in the flow chart
+goes away, etc. etc.
-----------------------------------------------------------------------
Summary of changes:
.../perlmods/lib/OpenILS/Application/Circ/Holds.pm | 4 +
.../Application/Storage/Publisher/action.pm | 155 ++++++++++++--------
Open-ILS/src/sql/Pg/002.schema.config.sql | 2 +-
Open-ILS/src/sql/Pg/950.data.seed-values.sql | 8 +-
...793.data.best-hold-order-traditional-approx.sql | 67 +++++++++
docs/TechRef/Circ/holds-go-home.txt | 93 ++++++++++++
6 files changed, 264 insertions(+), 65 deletions(-)
create mode 100644 Open-ILS/src/sql/Pg/upgrade/0793.data.best-hold-order-traditional-approx.sql
create mode 100644 docs/TechRef/Circ/holds-go-home.txt
hooks/post-receive
--
Evergreen ILS
More information about the open-ils-commits
mailing list