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.

Sign up for GitHub

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

Jump to bottom

Add support for prefixes #530

Merged
merged 4 commits into from
Jan 3, 2022
Merged

Add support for prefixes #530

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d22df50
Add support for prefixes
RobinTF Jan 2, 2022
291859c
Fix formatting
RobinTF Jan 3, 2022
0a22ae9
Fix tests
RobinTF Jan 3, 2022
42a89ac
Added a small comment
joka921 Jan 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/parser/SparqlParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,12 @@ ParsedQuery SparqlParser::parse() {
void SparqlParser::parseQuery(ParsedQuery* query, QueryType queryType) {
if (queryType == CONSTRUCT_QUERY) {
auto str = _lexer.getUnconsumedInput();
auto parseResult = sparqlParserHelpers::parseConstructTemplate(str);
SparqlQleverVisitor::PrefixMap prefixes;
for (const auto& prefix : query->_prefixes) {
prefixes[prefix._prefix] = prefix._uri;
}
auto parseResult =
sparqlParserHelpers::parseConstructTemplate(str, std::move(prefixes));
Comment on lines +45 to +50
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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

query->_clause = std::move(parseResult._resultOfParse);
_lexer.reset(std::move(parseResult._remainingText));
_lexer.expect("where");
Expand Down
14 changes: 8 additions & 6 deletions src/parser/SparqlParserHelpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include "SparqlParserHelpers.h"

#include "../util/antlr/ThrowingErrorStrategy.h"
#include "sparqlParser/SparqlQleverVisitor.h"
#include "sparqlParser/generated/SparqlAutomaticLexer.h"

namespace sparqlParserHelpers {
Expand All @@ -21,7 +20,9 @@ struct ParserAndVisitor {
public:
SparqlAutomaticParser _parser{&_tokens};
SparqlQleverVisitor _visitor;
explicit ParserAndVisitor(string input) : _input{std::move(input)} {
explicit ParserAndVisitor(string input,
SparqlQleverVisitor::PrefixMap prefixes)
: _input{std::move(input)}, _visitor{std::move(prefixes)} {
_parser.setErrorHandler(std::make_shared<ThrowingErrorStrategy>());
}

Expand All @@ -46,7 +47,7 @@ struct ParserAndVisitor {
// ____________________________________________________________________________
ResultOfParseAndRemainingText<sparqlExpression::SparqlExpressionPimpl>
parseExpression(const std::string& input) {
ParserAndVisitor p{input};
ParserAndVisitor p{input, {}};
auto resultOfParseAndRemainingText =
p.parse<sparqlExpression::SparqlExpression::Ptr>(
input, "expression", &SparqlAutomaticParser::expression);
Expand All @@ -60,15 +61,16 @@ parseExpression(const std::string& input) {
// ____________________________________________________________________________
ResultOfParseAndRemainingText<ParsedQuery::Alias> parseAlias(
const std::string& input) {
ParserAndVisitor p{input};
ParserAndVisitor p{input, {}};
return p.parse<ParsedQuery::Alias>(
input, "alias", &SparqlAutomaticParser::aliasWithouBrackes);
}
// _____________________________________________________________________________

ResultOfParseAndRemainingText<std::vector<std::array<VarOrTerm, 3>>>
parseConstructTemplate(const std::string& input) {
ParserAndVisitor p{input};
parseConstructTemplate(const std::string& input,
SparqlQleverVisitor::PrefixMap prefixes) {
ParserAndVisitor p{input, std::move(prefixes)};
return p.parse<std::vector<std::array<VarOrTerm, 3>>>(
input, "construct template", &SparqlAutomaticParser::constructTemplate);
}
Expand Down
4 changes: 3 additions & 1 deletion src/parser/SparqlParserHelpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "../engine/sparqlExpressions/SparqlExpressionPimpl.h"
#include "./ParsedQuery.h"
#include "sparqlParser/SparqlQleverVisitor.h"

namespace sparqlParserHelpers {

Expand All @@ -32,7 +33,8 @@ ResultOfParseAndRemainingText<ParsedQuery::Alias> parseAlias(
const std::string& input);

ResultOfParseAndRemainingText<std::vector<std::array<VarOrTerm, 3>>>
parseConstructTemplate(const std::string& input);
parseConstructTemplate(const std::string& input,
SparqlQleverVisitor::PrefixMap prefixes);
} // namespace sparqlParserHelpers

#endif // QLEVER_SPARQLPARSERHELPERS_H
5 changes: 3 additions & 2 deletions src/parser/sparqlParser/SparqlQleverVisitor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

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

Was this a bug already before or is this specific for the CONSTRUCT clause?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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 pnLocal = text.substr(pos + 1);
if (!_prefixMap.contains(pnameNS)) {
// TODO<joka921> : proper name
Expand All @@ -1189,7 +1189,8 @@ class SparqlQleverVisitor : public SparqlAutomaticVisitor {

antlrcpp::Any visitPnameNs(
SparqlAutomaticParser::PnameNsContext* ctx) override {
auto prefix = ctx->getText();
auto text = ctx->getText();
auto prefix = text.substr(0, text.length() - 1);
Comment on lines +1194 to +1195
Copy link
Member

Choose a reason for hiding this comment

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

Same question as before.

if (!_prefixMap.contains(prefix)) {
// TODO<joka921> : proper name
throw SparqlParseException{
Expand Down