Skip to content

Commit

Permalink
fix pagination with order_by with filter that returns empty pages for…
Browse files Browse the repository at this point in the history
… 4store
  • Loading branch information
syphax-bouazzouni committed Oct 19, 2023
1 parent 3163fc1 commit 4368235
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 12 deletions.
30 changes: 18 additions & 12 deletions lib/goo/sparql/query_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,15 @@ def build_select_query(ids, variables, graphs, patterns,
@klass, @unions, variables)

@order_by, variables, optional_patterns = init_order_by(@count, @klass, @order_by, optional_patterns, variables,patterns, query_options, graphs)
order_by_str, order_variables = order_by_string
variables, patterns = add_some_type_to_id(patterns, query_options, variables)

query_filter_str, patterns, optional_patterns, filter_variables =
filter_query_strings(@collection, graphs, internal_variables, @klass, optional_patterns, patterns, @query_filters)
filter_query_strings(@collection, graphs, @klass, optional_patterns, patterns, @query_filters)
variables = [] if @count
variables.delete :some_type

select_distinct(variables, aggregate_projections, filter_variables)
select_distinct(variables, aggregate_projections, filter_variables, order_variables)
.from(graphs)
.where(patterns)
.union_bind_in_where(properties_to_include)
Expand All @@ -54,7 +55,10 @@ def build_select_query(ids, variables, graphs, patterns,
@query.union(*@unions) unless @unions.empty?

ids_filter(ids) if ids
order_by if @order_by


@query.order_by(*order_by_str) if @order_by


put_query_aggregate_vars(aggregate_vars) if aggregate_vars
count if @count
Expand Down Expand Up @@ -116,16 +120,17 @@ def put_query_aggregate_vars(aggregate_vars)
self
end

def order_by
order_by_str = @order_by.map do |attr, order|
def order_by_string
order_variables = []
order_str = @order_by&.map do |attr, order|
if order.is_a?(Hash)
sub_attr, order = order.first
attr = @internal_variables_map[sub_attr]
sub_attr, order = attr.first
attr = @internal_variables_map.select{ |internal_var, attr_var| attr_var.eql?({attr => sub_attr}) || attr_var.eql?(sub_attr)}.keys.last
end
order_variables << attr
"#{order.to_s.upcase}(?#{attr})"
end
@query.order_by(*order_by_str)
self
[order_str,order_variables]
end

def from(graphs)
Expand All @@ -140,10 +145,11 @@ def from(graphs)
self
end

def select_distinct(variables, aggregate_projections, filter_variables)
def select_distinct(variables, aggregate_variables, filter_variables, order_variables)
select_vars = variables.dup
reject_aggregations_from_vars(select_vars, aggregate_projections) if aggregate_projections
select_vars = (select_vars + filter_variables).uniq if @page # Fix for 4store pagination with a filter
reject_aggregations_from_vars(select_vars, aggregate_variables) if aggregate_variables
# Fix for 4store pagination with a filter https://github.com/ontoportal-lirmm/ontologies_api/issues/25
select_vars = (select_vars + filter_variables + order_variables).uniq if @page
@query = @query.select(*select_vars).distinct(true)
self
end
Expand Down
11 changes: 11 additions & 0 deletions test/test_where.rb
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,17 @@ def test_embed_two_levels
end
end

def test_paging_with_filter_order
total_count = Student.where.count
page_1 = Student.where.page(1, total_count - 1).order_by(name: :asc).to_a
refute_empty page_1
assert page_1.next?
page_2 = Student.where.page(page_1.next_page, total_count - 1).order_by(name: :asc).to_a


refute_empty page_2
assert_equal total_count, page_1.size + page_2.size
end

def test_unique_object_references

Expand Down

0 comments on commit 4368235

Please sign in to comment.