ruby-rdf / sparql

Ruby SPARQL library
http://rubygems.org/gems/sparql
The Unlicense
88 stars 14 forks source link

Only the first ORDER operand value is taken into account #15

Closed michivi closed 9 years ago

michivi commented 9 years ago

Hello,

I believe the current implementation of the ORDER operator only takes into account the first ordering command.

I've made a small script to try and reveal the issue:

require 'rdf'
require 'sparql'

query = "select *
where
{
  values ?a { 3 1 2 }
  values ?b { 5 3 4 }
}
order by desc(?a) asc(?b)"

sse = SPARQL::Grammar.parse(query);
puts "
  Query:
    #{query}

  SXP:
    #{sse.to_sxp}

  Expected:
    a: 3, b: 3
    a: 3, b: 4
    a: 3, b: 5
    a: 2, b: 3
    a: 2, b: 4
    a: 2, b: 5
    a: 1, b: 3
    a: 1, b: 4
    a: 1, b: 5

  Actual:
"
repo = RDF::Repository.new
sse.execute(repo) { |solution| puts "    a: #{solution.a}, b: #{solution.b}" }

When executing this script, I get this:

  Query:
    select *
where
{
  values ?a { 3 1 2 }
  values ?b { 5 3 4 }
}
order by desc(?a) asc(?b)

  SXP:
    (order ((desc ?a) (asc ?b)) (join (table (vars ?a) (row (?a 3)) (row (?a 1)) (row (?a 2))) (table (vars ?b) (row (?b 5)) (row (?b 3)) (row (?b 4)))))

  Expected:
    a: 3, b: 3
    a: 3, b: 4
    a: 3, b: 5
    a: 2, b: 3
    a: 2, b: 4
    a: 2, b: 5
    a: 1, b: 3
    a: 1, b: 4
    a: 1, b: 5

  Actual:
    a: 3, b: 5
    a: 3, b: 3
    a: 3, b: 4
    a: 2, b: 3
    a: 2, b: 5
    a: 2, b: 4
    a: 1, b: 4
    a: 1, b: 5
    a: 1, b: 3

The actual values show that the ASC(?b) order seems to be ignored.

Looking at the code in lib/sparql/algebra/operator/order.rb, I believe the error is at the lines 37-54:

          operand(0).inject(false) do |memo, op|
            debug(options) {"(order) #{op.inspect}"}
            memo ||= begin
              # CODE
              comp == 0 ? false : comp
            end
           end || 0  # They compare equivalently if there are no matches

Shouldn't it be:

          operand(0).inject(0) do |memo, op|
            debug(options) {"(order) #{op.inspect}"}
            memo = begin
              # CODE
              comp
            end
          end if memo == 0

Does the false value have a specific meaning for comp? (it is used as the initial value and the value used in case of equality in the block, but overwritten by the || operator)