-
Notifications
You must be signed in to change notification settings - Fork 10
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
fix: remove use of JSON LD from CQL JSON parsing #316
Conversation
… numeric and string comparisons. e.g. FILTER(?var = 2000) will match on 2000, "2000"^^xsd:decimal etc. whereas VALUES ?var { 2000 } will only match on the exact same value. Use a triple pattern match for "=" filtering where the RHS operand is a URI. This should be more performant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks ok to me. admittedly, a bit hard to really understand all the changes. but it works locally for me and the tests are passing.
just needs to be rebased before merge.
Unfortunately the code touches a lot of places. In short, previously the CQL JSON was converted to JSON LD by injecting a JSON LD context, in order to convert "non semantic" JSON (CQL JSON) into RDF. The thinking was this would make interpreting/using the CQL JSON easier in Prez; in some ways it does, but ultimately it created more issues than it solved, so this change removes the injection of the JSON LD context. |
…ofile in order to support geometries that are specified on named node sequence paths (rather than the common specification of geometries using blank nodes).
…l-remove-context # Conflicts: # poetry.lock
🎉 This PR is included in version 4.2.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Removes the use of JSON LD with CQL JSON to simplify parsing/processing of CQL JSON.
In order to do this, CQL JSON properties that begin with "http" are interpreted as URIs. This is a reasonable but not foolproof assumption that may need revision.
Tests that previously only passed with Fuseki now work with the inclusion of GeoSPARQL support in Oxigraph and have been uncommented.