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

prepared quey now carry the original (unparsed) parameters #565

Merged
merged 1 commit into from
Jan 29, 2016

Conversation

pchampin
Copy link
Contributor

Here is my use case: I use prepareQuery in my code when a query is used several times. It looks like this:

# some module
from rdflib.plugins.sparql.processor import prepareQuery
Q = prepareQuery("SELECT * { ?x <http://example.org/prop> ?y }")

def get_y(graph, x):
    return graph.query(Q, initBindings={"x": x})

This saves some time by parsing the query once an for all.

Except that... this works fine when graph uses the SPARQL processor shipped with RDFlib. But if I use a third-party Store, which has its own native implementation of SPARQL, Q is not usable anymore...

The solution I found is to add an attribute to the object produced by prepareQuery, containing the original query string, so that alternative Store implementations can get the original query string and pass it to their own parser. Granted, in that case, prepareQuery works for nothing, but at least the code above works regardless of the underlying store (and the spurious parsing is done only once, so it is not too bad).

@joernhees joernhees added enhancement New feature or request SPARQL labels Dec 29, 2015
@joernhees joernhees added this to the rdflib 4.2.2 milestone Dec 29, 2015
@joernhees
Copy link
Member

hmm, i'm not sure i fully understand what you're trying to do here... wouldn't the following just work?

Q = "SELECT * { ?x <http://example.org/prop> ?y }"

I'm not per se against adding an original_args attribute to prepared queries, but what you suggest slightly changes the "interface" of a query and everything treating them. Currently the query is either a query or a str. Introducing this would cause another case distinction for cases where one maybe could just not have prepared the query?

Some other considerations: while python certainly allows this kind of "just add an attribute to some instance", it's not necessarily the best thing to do... prepareQuery returns a Query object. By doing what you suggest, we'd somehow elevate those Query objects to have more attributes than others in a place that is remotely connected to the Query class, making it harder to grasp that this special attribute might need to be treated by implementations. Making a PreparedQuery(Query) subclass right below Query would IMO make this way more obvious... but if we want this, don't we actually want this for Query in general? Maybe we should "just" add __str__ and __repr__ to Query?

@gromgull
Copy link
Member

I am confused, why is:

But if I use a third-party Store, which has its own native implementation of SPARQL, Q is not usable anymore...

the case - that seems to be the bug here?

If some other SPARQL engine messes up the preparedQuery - that is the problem of that SPARQL engine, that's why I clone here: 6200934#diff-8939ddb2af331e50406fdd945bf5fb2cR424

So when I insert the bindings into the algebra, the original prepared query remain unchanged.

@pchampin
Copy link
Contributor Author

@joernhees of course, nothing forces me to prepare the query Q, but I would like to take advantage of the preparation (at least for stores that support it).

@gromgull sorry if I was not clear. I'm using an implementation of Store using Virtuoso as its backend, so I can not pass the prepared query to Virtuoso, I need the query as as SPARQL string. It is not about the store messing with the prepared query, only that it can't use it as is...

Granted, my solution is quick hack which I thought had minimal impact (if at all) on the rest of the lib. It is true, however, that the Query object could in theory be produced by other means that prepareQuery, in which case my hack would still not work.

I like @joernhees 's last proposal: having a way to reconstruct the SPARQL string for a Query object would solve the problem in a more robust and elegant way (I am not sure about __repr__, but either __str__ or a specific method sounds fine). I would have gone for that but I am not very familiar with the data structure in Query.algebra...

@gromgull
Copy link
Member

Right - now I understand!

The long-term solution is certainly being able to re-serialize a parsed query object, but this is probably a days work at least. (I wondered about this recently, when I then made this dirty hack: https://gist.github.com/gromgull/70e2d0500fbcb820dd99 instead)

In the mean time - this is probably ok.

Currently, there are no other ways to create a query object apart from through the parser logic - perhaps the parsed string should be saved there. (Perhaps it already is? :) )

@joernhees joernhees merged commit 6de4da0 into RDFLib:master Jan 29, 2016
joernhees added a commit that referenced this pull request Jan 29, 2016
* pull/565:
  indicate that original_args are private
  prepared quey now carry the original (unparsed) parameters
@joernhees
Copy link
Member

moving on... @pchampin note that i made original_args private in ff5ada5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants