Add support for prefixes #530
Conversation
|
@RobinTF Thanks, Robin, the following command, which failed so far, now works with this PR.
|
| @@ -1172,7 +1172,7 @@ class SparqlQleverVisitor : public SparqlAutomaticVisitor { | |||
| SparqlAutomaticParser::PnameLnContext* ctx) override { | |||
| string text = ctx->getText(); | |||
| auto pos = text.find(':'); | |||
| auto pnameNS = text.substr(0, pos + 1); | |||
| auto pnameNS = text.substr(0, pos); | |||
There was a problem hiding this comment.
Was this a bug already before or is this specific for the CONSTRUCT clause?
There was a problem hiding this comment.
I wouldn't consider it a bug, but rather an implementation specific oddity.
Basically the prefix map was storing the trailing colon : along with the prefix name, but the non-antlr parser already stripped those colons from the prefix.
That's why previously the hash-map mapped key: to value instead of key to value and probably also the reason why the tests are currently failing
| auto text = ctx->getText(); | ||
| auto prefix = text.substr(0, text.length() - 1); |
| SparqlQleverVisitor::PrefixMap prefixes; | ||
| for (const auto& prefix : query->_prefixes) { | ||
| prefixes[prefix._prefix] = prefix._uri; | ||
| } | ||
| auto parseResult = | ||
| sparqlParserHelpers::parseConstructTemplate(str, std::move(prefixes)); |
There was a problem hiding this comment.
Are the prefixes now parsed twice (for the CONSTRUCT clause and the WHERE clause)? I am asking because prefixes did work already for the WHERE clause.
There was a problem hiding this comment.
They are not, the weirdness comes from the parser-duality that currently exists within the codebase.
Basically the existing parser (the non-antlr) version currently parses all the prefixes, but the parsing of the construct clause is done using antlr (which requires the parsed prefixes being passed here), which then delegates back to the original parser to parse the where clause
Fixes a bug that was mentioned in #528 (comment)
Prefix IRIs were not parsed correctly