Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixing issue #563 #564

Merged
merged 2 commits into from
Jan 30, 2016
Merged

fixing issue #563 #564

merged 2 commits into from
Jan 30, 2016

Conversation

pchampin
Copy link
Contributor

No description provided.

@joernhees joernhees added bug Something isn't working in-resolution SPARQL labels Dec 23, 2015
@joernhees joernhees added this to the rdflib 4.2.2 milestone Dec 23, 2015
bindings[a.res] = val
break
else:
bindings[a.res] = Literal(0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning a Literal(0) where we can't find anything seems wrong to me. Could we return None instead?

sample query on virtuoso

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sample of nothing should be an error:

http://www.w3.org/TR/sparql11-query/#defn_aggSample

Sample({}) = error

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the problem is that there are various meanings of "error"... IMO they don't mean to remove the whole result row / cancel the whole query, but just the one binding within the row, as shown here:
http://www.w3.org/TR/sparql11-query/#aggregateExample2

So if we encounter such an error, i think it's correct to somehow return the row with a placeholder indicating an unbound variable (which i thought None was), rather than removing the result row or simply cancelling the whole query.

At least that's what Virtuoso does in the sample query above, so it must be true ;)

@joernhees
Copy link
Member

@gromgull except for the Literal(0) this seems legit to me, any thoughts?

@pchampin
Copy link
Contributor Author

I'm terribly sorry about this mess with Literal(0). I totally agree that this seems wrong, I did it because I couldn't find the part of the spec where it was specified (thx @gromgull for pointing it out), and I thought that Virtuoso and Corese both did that. Obviously, they do not, they both return NULL instead, I still don't know how I messed up to get this impression :-/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working in-resolution SPARQL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants