Fix issue #936 HAVING clause with variable comparison not correctly evaluated

[devrb]

Modified the algebra.py file under sparql folder

Fixes #935, #936.

[coveralls]

Coverage remained the same at 88.521% when pulling 98900cc on devrb:master into aa33f9a on RDFLib:master.

[nicholascar]

@devrb can you please provide positive and negative tests to show how this PR solves the issue and doesn't introduce error? Thanks.

[white-gecko]

Merging master and #935 apparently does not fail for the GraphTestCase as introduced by #935 . No other test seem to fail. Hard to tell, if there are any side effects.

[nicholascar]

No other test seem to fail.

So do you recommend merging this or not? If your tests pass without the change, we shouldn't merge it, unless someone can show that it accomplishes something desirable

[ghost]

I rewrote the test as pytest-compatible:

import pytest
from rdflib import Graph

g = Graph()

data = """
<urn:a> <urn:p> 1 .
<urn:b> <urn:p> 3 .
<urn:c> <urn:q> 1 .
"""
g.parse(data=data, format="turtle")


def test_group_by():
    query = "SELECT ?p" "WHERE { ?s ?p ?o } " "GROUP BY ?p"
    qres = g.query(query)

    assert len(qres) == 2


def test_having_aggregate_eq_literal():
    query = (
        "SELECT ?p (avg(?o) as ?a) "
        "WHERE { ?s ?p ?o } "
        "GROUP BY ?p HAVING (avg(?o) = 2 )"
    )
    qres = g.query(query)

    assert len(qres) == 1


def test_having_primary_expression_var_neq_iri():
    query = "SELECT ?p " "WHERE { ?s ?p ?o } " "GROUP BY ?p HAVING (?p != <urn:foo> )"
    qres = g.query(query)

    assert len(qres) == 2

I was able to confirm that test_having_primary_expression_var_neq_iri fails without the change, passes with the change and the change cause no fails in the test suite:

6332 passed, 3 skipped, 229 xfailed, 2 xpassed, 52 warnings

However, the W3C SPARQL1.1 suite doesn't actually test HAVING clauses to any great degree of complexity:

$ grep -rni HAVING test/data/suites/w3c/sparql11/
test/data/suites/w3c/sparql11/aggregates/agg-avg-02.rq:7:HAVING (AVG(?o) <= 2.0)
test/data/suites/w3c/sparql11/aggregates/agg07.rq:6:HAVING ( COUNT(*) > 2 )
test/data/suites/w3c/sparql11/aggregates/agg03.rq:6:HAVING (COUNT(?O) > 2 )
test/data/suites/w3c/sparql11/aggregates/agg06.rq:5:HAVING (COUNT(*) > 0 )

[aucampia]

This looks okay to me, I think though that rdflib.plugins.sparql.algebra.traverse is a bit iffy, I think other places where it is used also has problems, but this change seems okay.