From 2a433735de937c1dbde2dde7a2f6a9e8526a4edc Mon Sep 17 00:00:00 2001 From: Lebbeous Fogle-Weekley Date: Fri, 14 Jun 2013 18:22:58 -0400 Subject: [PATCH] OPAC Browse: Put best-matched browse entry in middle of result set Signed-off-by: Lebbeous Fogle-Weekley --- .../perlmods/lib/OpenILS/WWW/EGCatLoader/Browse.pm | 109 +++---------- Open-ILS/src/sql/Pg/030.schema.metabib.sql | 181 ++++++++++++++------- .../sql/Pg/upgrade/YYYY.schema.bib-auth-browse.sql | 180 +++++++++++++------- Open-ILS/src/templates/opac/browse.tt2 | 19 +-- 4 files changed, 278 insertions(+), 211 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Browse.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Browse.pm index 62b0c4a3d6..5431774cd5 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Browse.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Browse.pm @@ -54,16 +54,7 @@ sub prepare_browse_parameters { no warnings 'uninitialized'; - # XXX TODO add config.global_flag rows for browse limit-limit and - # browse offset-limit? - - my $limit = int( - $self->cgi->param('blimit') || - $self->ctx->{opac_hits_per_page} || - 10 - ); - my $offset = int($self->cgi->param('boffset') || 0); - my $force_backward = scalar($self->cgi->param('bback')); + # XXX TODO add config.global_flag rows for browse limit-limit ? my @params = ( scalar($self->cgi->param('qtype')), @@ -73,19 +64,15 @@ sub prepare_browse_parameters { $self->ctx->{copy_location_group}, $self->ctx->{is_staff} ? 't' : 'f', scalar($self->cgi->param('bpivot')), - $force_backward ? 't' : 'f' + int( + $self->cgi->param('blimit') || + $self->ctx->{opac_hits_per_page} || 10 + ) ); - # We do need $limit, $offset, and $force_backward as part of the - # cache key, but we also need to keep them separate from other - # parameters for purposes of paging link generation. return ( - "oils_browse_" . md5_hex( - OpenSRF::Utils::JSON->perl2JSON( - [@params, $limit, $offset, $force_backward] - ) - ), - $limit, $offset, $force_backward, @params + "oils_browse_" . md5_hex(OpenSRF::Utils::JSON->perl2JSON(\@params)), + @params ); } @@ -278,15 +265,10 @@ sub flesh_browse_results { } sub load_browse_impl { - my ($self, $limit, $offset, $force_backward, @params) = @_; - - my $inner_limit = ($offset >= 0 and not $force_backward) ? - $limit + 1 : $limit; + my ($self, @params) = @_; my $results = $self->editor->json_query({ - from => [ - "metabib.browse", (@params, $inner_limit, $offset) - ] + from => [ "metabib.browse", @params ] }); if (not $results) { # DB error, not empty result set. @@ -308,50 +290,23 @@ sub load_browse_impl { return $results; } -# $results can be modified by this function. This would be simpler -# but for the moving pivot concept that helps us avoid paging with -# large offsets (slow). +# Find paging information, put it into $self->ctx, and return "real" +# rows from $results, excluding those that contain only paging +# information. sub infer_browse_paging { - my ($self, $results, $limit, $offset, $force_backward) = @_; - - # (All these comments assume a default limit of 10). For typical - # not-backwards requests not at the end of the result set, we - # should have an eleventh result that tells us what's next. - while (scalar @$results > $limit) { - $self->ctx->{forward_pivot} = (pop @$results)->{browse_entry}; - $self->ctx->{more_forward} = 1; - } - - # If we're going backwards by pivot id, we don't have an eleventh - # result to tell us we can page forward, but we can assume we can - # go forward because duh, we followed a link backward to get here. - if ($force_backward and $self->cgi->param('bpivot')) { - $self->ctx->{forward_pivot} = scalar($self->cgi->param('bpivot')); - $self->ctx->{more_forward} = 1; - } - - # The pivot that the user can use for going backwards is the first - # of the result set. - if (@$results) { - $self->ctx->{back_pivot} = $results->[0]->{browse_entry}; - } - - # The result of these tests relate to basic limit/offset paging. + my ($self, $results) = @_; - # This comparison for setting more_forward does not fold into - # those for setting more_back. - if ($offset < 0 || $force_backward) { - $self->ctx->{more_forward} = 1; + foreach (@$results) { + if ($_->{pivot_point}) { + if ($_->{row_number} < 0) { # sic + $self->ctx->{forward_pivot} = $_->{pivot_point}; + } else { + $self->ctx->{back_pivot} = $_->{pivot_point}; + } + } } - if ($offset > 0 or (!$force_backward and $self->cgi->param('bpivot'))) { - $self->ctx->{more_back} = 1; - } elsif (scalar @$results < $limit) { - $self->ctx->{more_back} = 0; - } elsif (not($self->cgi->param('bterm') eq '0' and - not defined $self->cgi->param('bpivot'))) { # paging links - $self->ctx->{more_back} = 1; - } + return [ grep { not defined $_->{pivot_point} } @$results ]; } sub leading_article_test { @@ -390,8 +345,8 @@ sub load_browse { _init_browse_cache(); - # This is just so we can access a user settings IFF the user is logged in - # (opac.hits_per_page). + # If there's a user logged in, flesh extended user info so we can get + # her opac.hits_per_page setting, if any. if ($self->ctx->{user}) { $self->prepare_extended_user_info('settings'); if (my $setting = first { $_->name eq 'opac.hits_per_page' } @@ -401,10 +356,6 @@ sub load_browse { } } - - $self->ctx->{more_forward} = 0; - $self->ctx->{more_back} = 0; - if ($self->cgi->param('qtype') and defined $self->cgi->param('bterm')) { $self->leading_article_test( @@ -412,24 +363,18 @@ sub load_browse { $self->cgi->param('bterm') ); - my ($cache_key, $limit, $offset, $force_backward, @params) = - $self->prepare_browse_parameters; + my ($cache_key, @params) = $self->prepare_browse_parameters; my $results = $browse_cache->get_cache($cache_key); if (not $results) { - $results = $self->load_browse_impl( - $limit, $offset, $force_backward, @params - ); + $results = $self->load_browse_impl(@params); if ($results) { $browse_cache->put_cache($cache_key, $results, $browse_timeout); } } if ($results) { - $self->infer_browse_paging( - $results, $limit, $offset, $force_backward - ); - $self->ctx->{browse_results} = $results; + $self->ctx->{browse_results} = $self->infer_browse_paging($results); } # We don't need an else clause to send the user a 5XX error or diff --git a/Open-ILS/src/sql/Pg/030.schema.metabib.sql b/Open-ILS/src/sql/Pg/030.schema.metabib.sql index 2755d224b3..0fa82bea1e 100644 --- a/Open-ILS/src/sql/Pg/030.schema.metabib.sql +++ b/Open-ILS/src/sql/Pg/030.schema.metabib.sql @@ -1747,9 +1747,10 @@ CREATE TYPE metabib.flat_browse_entry_appearance AS ( authorities TEXT, sources INT, -- visible ones, that is row_number INT, -- internal use, sort of - accurate BOOL -- Count in sources field is accurate? Not + accurate BOOL, -- Count in sources field is accurate? Not -- if we had more than a browse superpage -- of records to look at. + pivot_point BIGINT ); @@ -1773,42 +1774,55 @@ END; $p$ LANGUAGE PLPGSQL; CREATE OR REPLACE FUNCTION metabib.staged_browse( - core_query TEXT, + query TEXT, context_org INT, context_locations INT[], staff BOOL, browse_superpage_size INT, + count_up_from_zero BOOL, -- if false, count down from -1 result_limit INT, - use_offset INT + next_pivot_pos INT ) RETURNS SETOF metabib.flat_browse_entry_appearance AS $p$ DECLARE - core_cursor REFCURSOR; - core_record RECORD; + curs REFCURSOR; + rec RECORD; qpfts_query TEXT; result_row metabib.flat_browse_entry_appearance%ROWTYPE; results_skipped INT := 0; - results_returned INT := 0; + row_counter INT := 0; + row_number INT; slice_start INT; slice_end INT; full_end INT; superpage_of_records BIGINT[]; superpage_size INT; BEGIN - OPEN core_cursor FOR EXECUTE core_query; + IF count_up_from_zero THEN + row_number := 0; + ELSE + row_number := -1; + END IF; + + OPEN curs FOR EXECUTE query; LOOP - FETCH core_cursor INTO core_record; - EXIT WHEN NOT FOUND; + FETCH curs INTO rec; + IF NOT FOUND THEN + IF result_row.pivot_point IS NOT NULL THEN + RETURN NEXT result_row; + END IF; + RETURN; + END IF; result_row.sources := 0; - full_end := ARRAY_LENGTH(core_record.records, 1); + full_end := ARRAY_LENGTH(rec.records, 1); superpage_size := COALESCE(browse_superpage_size, full_end); slice_start := 1; slice_end := superpage_size; WHILE result_row.sources = 0 AND slice_start <= full_end LOOP - superpage_of_records := core_record.records[slice_start:slice_end]; + superpage_of_records := rec.records[slice_start:slice_end]; qpfts_query := 'SELECT NULL::BIGINT AS id, ARRAY[r] AS records, ' || '1::INT AS rel FROM (SELECT UNNEST(' || @@ -1836,30 +1850,57 @@ BEGIN browse_superpage_size >= full_end; IF result_row.sources > 0 THEN - IF results_skipped < use_offset THEN - results_skipped := results_skipped + 1; - CONTINUE; - END IF; + -- We've got a browse entry with visible holdings. Yay. + - result_row.browse_entry := core_record.id; - result_row.authorities := core_record.authorities; - result_row.fields := core_record.fields; - result_row.value := core_record.value; + -- The function that calls this function needs row_number in order + -- to correctly order results from two different runs of this + -- functions. + result_row.row_number := row_number; - -- This is needed so our caller can flip it and reverse it. - result_row.row_number := results_returned; + -- Now, if row_counter is still less than limit, return a row. If + -- not, but it is less than next_pivot_pos, continue on without + -- returning actual result rows until we find + -- that next pivot, and return it. - RETURN NEXT result_row; + IF row_counter < result_limit THEN + result_row.browse_entry := rec.id; + result_row.authorities := rec.authorities; + result_row.fields := rec.fields; + result_row.value := rec.value; - results_returned := results_returned + 1; + RETURN NEXT result_row; + ELSE + result_row.browse_entry := NULL; + result_row.authorities := NULL; + result_row.fields := NULL; + result_row.value := NULL; + result_row.sources := NULL; + result_row.accurate := NULL; + result_row.pivot_point := rec.id; + + IF row_counter >= next_pivot_pos THEN + RETURN NEXT result_row; + RETURN; + END IF; + END IF; - EXIT WHEN results_returned >= result_limit; + IF count_up_from_zero THEN + row_number := row_number + 1; + ELSE + row_number := row_number - 1; + END IF; + + -- row_counter is different from row_number. + -- It simply counts up from zero so that we know when + -- we've reached our limit. + row_counter := row_counter + 1; END IF; END LOOP; END; $p$ LANGUAGE PLPGSQL; --- This is optimized to be fast for values of result_offset near zero. + CREATE OR REPLACE FUNCTION metabib.browse( search_field INT[], browse_term TEXT, @@ -1867,20 +1908,23 @@ CREATE OR REPLACE FUNCTION metabib.browse( context_loc_group INT DEFAULT NULL, staff BOOL DEFAULT FALSE, pivot_id BIGINT DEFAULT NULL, - force_backward BOOL DEFAULT FALSE, - result_limit INT DEFAULT 10, - result_offset INT DEFAULT 0 -- Can be negative! + result_limit INT DEFAULT 10 ) RETURNS SETOF metabib.flat_browse_entry_appearance AS $p$ DECLARE core_query TEXT; - whole_query TEXT; + back_query TEXT; + forward_query TEXT; pivot_sort_value TEXT; pivot_sort_fallback TEXT; context_locations INT[]; - use_offset INT; browse_superpage_size INT; results_skipped INT := 0; + back_limit INT; + back_to_pivot INT; + forward_limit INT; + forward_to_pivot INT; BEGIN + -- First, find the pivot if we were given a browse term but not a pivot. IF pivot_id IS NULL THEN pivot_id := metabib.browse_pivot(search_field, browse_term); END IF; @@ -1888,20 +1932,43 @@ BEGIN SELECT INTO pivot_sort_value, pivot_sort_fallback sort_value, value FROM metabib.browse_entry WHERE id = pivot_id; + -- Bail if we couldn't find a pivot. IF pivot_sort_value IS NULL THEN RETURN; END IF; + -- Transform the context_loc_group argument (if any) (logc at the + -- TPAC layer) into a form we'll be able to use. IF context_loc_group IS NOT NULL THEN SELECT INTO context_locations ARRAY_AGG(location) FROM asset.copy_location_group_map WHERE lgroup = context_loc_group; END IF; + -- Get the configured size of browse superpages. SELECT INTO browse_superpage_size value -- NULL ok FROM config.global_flag WHERE enabled AND name = 'opac.browse.holdings_visibility_test_limit'; + -- First we're going to search backward from the pivot, then we're going + -- to search forward. In each direction, we need two limits. At the + -- lesser of the two limits, we delineate the edge of the result set + -- we're going to return. At the greater of the two limits, we find the + -- pivot value that would represent an offset from the current pivot + -- at a distance of one "page" in either direction, where a "page" is a + -- result set of the size specified in the "result_limit" argument. + -- + -- The two limits in each direction make four derived values in total, + -- and we calculate them now. + back_limit := CEIL(result_limit::FLOAT / 2); + back_to_pivot := result_limit; + forward_limit := result_limit / 2; + forward_to_pivot := result_limit - 1; + + -- This is the meat of the SQL query that finds browse entries. We'll + -- pass this to a function which uses it with a cursor, so that individual + -- rows may be fetched in a loop until some condition is satisfied, without + -- waiting for a result set of fixed size to be collected all at once. core_query := ' SELECT mbe.id, @@ -1923,30 +1990,29 @@ BEGIN ) WHERE '; - -- PostgreSQL is not magic. We can't actually pass a negative offset. - IF result_offset >= 0 AND NOT force_backward THEN - use_offset := result_offset; - core_query := core_query || - ' mbe.sort_value >= ' || quote_literal(pivot_sort_value) || - ' GROUP BY 1,2,3 ORDER BY mbe.sort_value, mbe.value '; - - RETURN QUERY SELECT * FROM metabib.staged_browse( - core_query, context_org, context_locations, - staff, browse_superpage_size, result_limit, use_offset - ); - ELSE - -- Part 1 of 2 to deliver what the user wants with a negative offset: - core_query := core_query || - ' mbe.sort_value < ' || quote_literal(pivot_sort_value) || - ' GROUP BY 1,2,3 ORDER BY mbe.sort_value DESC, mbe.value DESC '; - - -- Part 2 of 2 to deliver what the user wants with a negative offset: - RETURN QUERY SELECT * FROM (SELECT * FROM metabib.staged_browse( - core_query, context_org, context_locations, - staff, browse_superpage_size, result_limit, use_offset - )) sb ORDER BY row_number DESC; + -- This is the variant of the query for browsing backward. + back_query := core_query || + ' mbe.sort_value <= ' || quote_literal(pivot_sort_value) || + ' GROUP BY 1,2,3 ORDER BY mbe.sort_value DESC, mbe.value DESC '; + + -- This variant browses forward. + forward_query := core_query || + ' mbe.sort_value > ' || quote_literal(pivot_sort_value) || + ' GROUP BY 1,2,3 ORDER BY mbe.sort_value, mbe.value '; + + -- We now call the function which applies a cursor to the provided + -- queries, stopping at the appropriate limits and also giving us + -- the next page's pivot. + RETURN QUERY + SELECT * FROM metabib.staged_browse( + back_query, context_org, context_locations, + staff, browse_superpage_size, TRUE, back_limit, back_to_pivot + ) UNION + SELECT * FROM metabib.staged_browse( + forward_query, context_org, context_locations, + staff, browse_superpage_size, FALSE, forward_limit, forward_to_pivot + ) ORDER BY row_number DESC; - END IF; END; $p$ LANGUAGE PLPGSQL; @@ -1957,9 +2023,7 @@ CREATE OR REPLACE FUNCTION metabib.browse( context_loc_group INT DEFAULT NULL, staff BOOL DEFAULT FALSE, pivot_id BIGINT DEFAULT NULL, - force_backward BOOL DEFAULT FALSE, - result_limit INT DEFAULT 10, - result_offset INT DEFAULT 0 -- Can be negative, implying backward! + result_limit INT DEFAULT 10 ) RETURNS SETOF metabib.flat_browse_entry_appearance AS $p$ BEGIN RETURN QUERY SELECT * FROM metabib.browse( @@ -1970,12 +2034,9 @@ BEGIN context_loc_group, staff, pivot_id, - force_backward, - result_limit, - result_offset + result_limit ); END; $p$ LANGUAGE PLPGSQL; - COMMIT; diff --git a/Open-ILS/src/sql/Pg/upgrade/YYYY.schema.bib-auth-browse.sql b/Open-ILS/src/sql/Pg/upgrade/YYYY.schema.bib-auth-browse.sql index e441641053..2077f9c9da 100644 --- a/Open-ILS/src/sql/Pg/upgrade/YYYY.schema.bib-auth-browse.sql +++ b/Open-ILS/src/sql/Pg/upgrade/YYYY.schema.bib-auth-browse.sql @@ -7148,9 +7148,10 @@ CREATE TYPE metabib.flat_browse_entry_appearance AS ( authorities TEXT, sources INT, -- visible ones, that is row_number INT, -- internal use, sort of - accurate BOOL -- Count in sources field is accurate? Not + accurate BOOL, -- Count in sources field is accurate? Not -- if we had more than a browse superpage -- of records to look at. + pivot_point BIGINT ); @@ -7174,42 +7175,55 @@ END; $p$ LANGUAGE PLPGSQL; CREATE OR REPLACE FUNCTION metabib.staged_browse( - core_query TEXT, + query TEXT, context_org INT, context_locations INT[], staff BOOL, browse_superpage_size INT, + count_up_from_zero BOOL, -- if false, count down from -1 result_limit INT, - use_offset INT + next_pivot_pos INT ) RETURNS SETOF metabib.flat_browse_entry_appearance AS $p$ DECLARE - core_cursor REFCURSOR; - core_record RECORD; + curs REFCURSOR; + rec RECORD; qpfts_query TEXT; result_row metabib.flat_browse_entry_appearance%ROWTYPE; results_skipped INT := 0; - results_returned INT := 0; + row_counter INT := 0; + row_number INT; slice_start INT; slice_end INT; full_end INT; superpage_of_records BIGINT[]; superpage_size INT; BEGIN - OPEN core_cursor FOR EXECUTE core_query; + IF count_up_from_zero THEN + row_number := 0; + ELSE + row_number := -1; + END IF; + + OPEN curs FOR EXECUTE query; LOOP - FETCH core_cursor INTO core_record; - EXIT WHEN NOT FOUND; + FETCH curs INTO rec; + IF NOT FOUND THEN + IF result_row.pivot_point IS NOT NULL THEN + RETURN NEXT result_row; + END IF; + RETURN; + END IF; result_row.sources := 0; - full_end := ARRAY_LENGTH(core_record.records, 1); + full_end := ARRAY_LENGTH(rec.records, 1); superpage_size := COALESCE(browse_superpage_size, full_end); slice_start := 1; slice_end := superpage_size; WHILE result_row.sources = 0 AND slice_start <= full_end LOOP - superpage_of_records := core_record.records[slice_start:slice_end]; + superpage_of_records := rec.records[slice_start:slice_end]; qpfts_query := 'SELECT NULL::BIGINT AS id, ARRAY[r] AS records, ' || '1::INT AS rel FROM (SELECT UNNEST(' || @@ -7237,30 +7251,57 @@ BEGIN browse_superpage_size >= full_end; IF result_row.sources > 0 THEN - IF results_skipped < use_offset THEN - results_skipped := results_skipped + 1; - CONTINUE; - END IF; + -- We've got a browse entry with visible holdings. Yay. - result_row.browse_entry := core_record.id; - result_row.authorities := core_record.authorities; - result_row.fields := core_record.fields; - result_row.value := core_record.value; - -- This is needed so our caller can flip it and reverse it. - result_row.row_number := results_returned; + -- The function that calls this function needs row_number in order + -- to correctly order results from two different runs of this + -- functions. + result_row.row_number := row_number; - RETURN NEXT result_row; + -- Now, if row_counter is still less than limit, return a row. If + -- not, but it is less than next_pivot_pos, continue on without + -- returning actual result rows until we find + -- that next pivot, and return it. - results_returned := results_returned + 1; + IF row_counter < result_limit THEN + result_row.browse_entry := rec.id; + result_row.authorities := rec.authorities; + result_row.fields := rec.fields; + result_row.value := rec.value; - EXIT WHEN results_returned >= result_limit; + RETURN NEXT result_row; + ELSE + result_row.browse_entry := NULL; + result_row.authorities := NULL; + result_row.fields := NULL; + result_row.value := NULL; + result_row.sources := NULL; + result_row.accurate := NULL; + result_row.pivot_point := rec.id; + + IF row_counter >= next_pivot_pos THEN + RETURN NEXT result_row; + RETURN; + END IF; + END IF; + + IF count_up_from_zero THEN + row_number := row_number + 1; + ELSE + row_number := row_number - 1; + END IF; + + -- row_counter is different from row_number. + -- It simply counts up from zero so that we know when + -- we've reached our limit. + row_counter := row_counter + 1; END IF; END LOOP; END; $p$ LANGUAGE PLPGSQL; --- This is optimized to be fast for values of result_offset near zero. + CREATE OR REPLACE FUNCTION metabib.browse( search_field INT[], browse_term TEXT, @@ -7268,20 +7309,23 @@ CREATE OR REPLACE FUNCTION metabib.browse( context_loc_group INT DEFAULT NULL, staff BOOL DEFAULT FALSE, pivot_id BIGINT DEFAULT NULL, - force_backward BOOL DEFAULT FALSE, - result_limit INT DEFAULT 10, - result_offset INT DEFAULT 0 -- Can be negative! + result_limit INT DEFAULT 10 ) RETURNS SETOF metabib.flat_browse_entry_appearance AS $p$ DECLARE core_query TEXT; - whole_query TEXT; + back_query TEXT; + forward_query TEXT; pivot_sort_value TEXT; pivot_sort_fallback TEXT; context_locations INT[]; - use_offset INT; browse_superpage_size INT; results_skipped INT := 0; + back_limit INT; + back_to_pivot INT; + forward_limit INT; + forward_to_pivot INT; BEGIN + -- First, find the pivot if we were given a browse term but not a pivot. IF pivot_id IS NULL THEN pivot_id := metabib.browse_pivot(search_field, browse_term); END IF; @@ -7289,20 +7333,43 @@ BEGIN SELECT INTO pivot_sort_value, pivot_sort_fallback sort_value, value FROM metabib.browse_entry WHERE id = pivot_id; + -- Bail if we couldn't find a pivot. IF pivot_sort_value IS NULL THEN RETURN; END IF; + -- Transform the context_loc_group argument (if any) (logc at the + -- TPAC layer) into a form we'll be able to use. IF context_loc_group IS NOT NULL THEN SELECT INTO context_locations ARRAY_AGG(location) FROM asset.copy_location_group_map WHERE lgroup = context_loc_group; END IF; + -- Get the configured size of browse superpages. SELECT INTO browse_superpage_size value -- NULL ok FROM config.global_flag WHERE enabled AND name = 'opac.browse.holdings_visibility_test_limit'; + -- First we're going to search backward from the pivot, then we're going + -- to search forward. In each direction, we need two limits. At the + -- lesser of the two limits, we delineate the edge of the result set + -- we're going to return. At the greater of the two limits, we find the + -- pivot value that would represent an offset from the current pivot + -- at a distance of one "page" in either direction, where a "page" is a + -- result set of the size specified in the "result_limit" argument. + -- + -- The two limits in each direction make four derived values in total, + -- and we calculate them now. + back_limit := CEIL(result_limit::FLOAT / 2); + back_to_pivot := result_limit; + forward_limit := result_limit / 2; + forward_to_pivot := result_limit - 1; + + -- This is the meat of the SQL query that finds browse entries. We'll + -- pass this to a function which uses it with a cursor, so that individual + -- rows may be fetched in a loop until some condition is satisfied, without + -- waiting for a result set of fixed size to be collected all at once. core_query := ' SELECT mbe.id, @@ -7324,30 +7391,29 @@ BEGIN ) WHERE '; - -- PostgreSQL is not magic. We can't actually pass a negative offset. - IF result_offset >= 0 AND NOT force_backward THEN - use_offset := result_offset; - core_query := core_query || - ' mbe.sort_value >= ' || quote_literal(pivot_sort_value) || - ' GROUP BY 1,2,3 ORDER BY mbe.sort_value, mbe.value '; - - RETURN QUERY SELECT * FROM metabib.staged_browse( - core_query, context_org, context_locations, - staff, browse_superpage_size, result_limit, use_offset - ); - ELSE - -- Part 1 of 2 to deliver what the user wants with a negative offset: - core_query := core_query || - ' mbe.sort_value < ' || quote_literal(pivot_sort_value) || - ' GROUP BY 1,2,3 ORDER BY mbe.sort_value DESC, mbe.value DESC '; - - -- Part 2 of 2 to deliver what the user wants with a negative offset: - RETURN QUERY SELECT * FROM (SELECT * FROM metabib.staged_browse( - core_query, context_org, context_locations, - staff, browse_superpage_size, result_limit, use_offset - )) sb ORDER BY row_number DESC; + -- This is the variant of the query for browsing backward. + back_query := core_query || + ' mbe.sort_value <= ' || quote_literal(pivot_sort_value) || + ' GROUP BY 1,2,3 ORDER BY mbe.sort_value DESC, mbe.value DESC '; + + -- This variant browses forward. + forward_query := core_query || + ' mbe.sort_value > ' || quote_literal(pivot_sort_value) || + ' GROUP BY 1,2,3 ORDER BY mbe.sort_value, mbe.value '; + + -- We now call the function which applies a cursor to the provided + -- queries, stopping at the appropriate limits and also giving us + -- the next page's pivot. + RETURN QUERY + SELECT * FROM metabib.staged_browse( + back_query, context_org, context_locations, + staff, browse_superpage_size, TRUE, back_limit, back_to_pivot + ) UNION + SELECT * FROM metabib.staged_browse( + forward_query, context_org, context_locations, + staff, browse_superpage_size, FALSE, forward_limit, forward_to_pivot + ) ORDER BY row_number DESC; - END IF; END; $p$ LANGUAGE PLPGSQL; @@ -7358,9 +7424,7 @@ CREATE OR REPLACE FUNCTION metabib.browse( context_loc_group INT DEFAULT NULL, staff BOOL DEFAULT FALSE, pivot_id BIGINT DEFAULT NULL, - force_backward BOOL DEFAULT FALSE, - result_limit INT DEFAULT 10, - result_offset INT DEFAULT 0 -- Can be negative, implying backward! + result_limit INT DEFAULT 10 ) RETURNS SETOF metabib.flat_browse_entry_appearance AS $p$ BEGIN RETURN QUERY SELECT * FROM metabib.browse( @@ -7371,9 +7435,7 @@ BEGIN context_loc_group, staff, pivot_id, - force_backward, - result_limit, - result_offset + result_limit ); END; $p$ LANGUAGE PLPGSQL; diff --git a/Open-ILS/src/templates/opac/browse.tt2 b/Open-ILS/src/templates/opac/browse.tt2 index 0bb4931f42..df0e7fc02e 100644 --- a/Open-ILS/src/templates/opac/browse.tt2 +++ b/Open-ILS/src/templates/opac/browse.tt2 @@ -8,9 +8,8 @@ ctx.page_title = l("Browse the Catalog"); blimit = CGI.param('blimit') || ctx.opac_hits_per_page || 10; - boffset = CGI.param('boffset') || 0; - depart_list = ['blimit', 'bterm', 'boffset', 'bpivot', 'bback']; + depart_list = ['blimit', 'bterm', 'bpivot']; %]
@@ -48,21 +47,21 @@ [% BLOCK browse_pager %]
- [% IF ctx.more_back %] - ← [%l ('Back') %] + [% IF ctx.back_pivot %] + ← [%l ('Back') %] [% END %] [% IF browse.english_pager; # XXX how to apply i18n here? current_qtype = CGI.param('qtype') || 'title' %] - 0-9 + 0-9 [% FOR letter IN ['A'..'Z'] %] - [% letter %] + [% letter %] [% END %] [% END %] - [% IF ctx.more_forward %] - [%l ('Next') %] → + [% IF ctx.forward_pivot %] + [%l ('Next') %] → [% END %] @@ -85,7 +84,7 @@ [% IF ctx.browse_leading_article_alternative %]

[% alternative_link = BLOCK %] - [% ctx.browse_leading_article_alternative | html %] + [% ctx.browse_leading_article_alternative | html %] [% END; # alternative_link BLOCK l("Did you mean [_1]?", alternative_link); END # IF %] @@ -169,7 +168,7 @@ mkurl_args.locg = piece.org_id; END; %] - [% piece.heading | html %] + [% piece.heading | html %] [% ELSIF piece.institution %] [% piece.institution | html %] -- 2.11.0