Skip to content

Commit

Permalink
Implement CONSTRUCT WHERE correctly (#1757)
Browse files Browse the repository at this point in the history
Queries such as ` CONSTRUCT WHERE { ?s ?p ?o}` are now correctly treated equivalently to `CONSTRUCT { ?s ?p ?o} WHERE { ?s ?p ?o}`. Previously they returned an empty result because they were evaluated like `CONSTRUCT {} WHERE {?s ?p ?o}`
Fixes #1349
  • Loading branch information
RobinTF authored Feb 6, 2025
1 parent 098b79c commit a307781
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 3 deletions.
49 changes: 49 additions & 0 deletions src/parser/sparqlParser/SparqlQleverVisitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,47 @@ Alias Visitor::visit(Parser::AliasWithoutBracketsContext* ctx) {
return {visitExpressionPimpl(ctx->expression()), visit(ctx->var())};
}

// ____________________________________________________________________________________
parsedQuery::BasicGraphPattern Visitor::toGraphPattern(
const ad_utility::sparql_types::Triples& triples) {
parsedQuery::BasicGraphPattern pattern{};
pattern._triples.reserve(triples.size());
auto toTripleComponent = []<typename T>(const T& item) {
namespace tc = ad_utility::triple_component;
if constexpr (ad_utility::isSimilar<T, Variable>) {
return TripleComponent{item};
} else if constexpr (ad_utility::isSimilar<T, BlankNode>) {
// Blank Nodes in the pattern are to be treated as internal variables
// inside WHERE.
return TripleComponent{
ParsedQuery::blankNodeToInternalVariable(item.toSparql())};
} else {
static_assert(ad_utility::SimilarToAny<T, Literal, Iri>);
return RdfStringParser<TurtleParser<Tokenizer>>::parseTripleObject(
item.toSparql());
}
};
auto toPropertyPath = []<typename T>(const T& item) -> PropertyPath {
if constexpr (ad_utility::isSimilar<T, Variable>) {
return PropertyPath::fromVariable(item);
} else if constexpr (ad_utility::isSimilar<T, Iri>) {
return PropertyPath::fromIri(item.toSparql());
} else {
static_assert(ad_utility::SimilarToAny<T, Literal, BlankNode>);
// This case can only happen if there's a bug in the SPARQL parser.
AD_THROW("Literals or blank nodes are not valid predicates.");
}
};
for (const auto& triple : triples) {
auto subject = std::visit(toTripleComponent, triple.at(0));
auto predicate = std::visit(toPropertyPath, triple.at(1));
auto object = std::visit(toTripleComponent, triple.at(2));
pattern._triples.emplace_back(std::move(subject), std::move(predicate),
std::move(object));
}
return pattern;
}

// ____________________________________________________________________________________
ParsedQuery Visitor::visit(Parser::ConstructQueryContext* ctx) {
ParsedQuery query;
Expand All @@ -288,8 +329,16 @@ ParsedQuery Visitor::visit(Parser::ConstructQueryContext* ctx) {
.value_or(parsedQuery::ConstructClause{});
visitWhereClause(ctx->whereClause(), query);
} else {
// For `CONSTRUCT WHERE`, the CONSTRUCT template and the WHERE clause are
// syntactically the same, so we set the flag to true to keep the blank
// nodes, and convert them into variables during `toGraphPattern`.
isInsideConstructTriples_ = true;
auto cleanup =
absl::Cleanup{[this]() { isInsideConstructTriples_ = false; }};
query._clause = parsedQuery::ConstructClause{
visitOptional(ctx->triplesTemplate()).value_or(Triples{})};
query._rootGraphPattern._graphPatterns.emplace_back(
toGraphPattern(query.constructClause().triples_));
}
query.addSolutionModifiers(visit(ctx->solutionModifier()));

Expand Down
11 changes: 11 additions & 0 deletions src/parser/sparqlParser/SparqlQleverVisitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#pragma once

#include <antlr4-runtime.h>
#include <gtest/gtest_prod.h>

#include "engine/sparqlExpressions/AggregateExpression.h"
#include "engine/sparqlExpressions/NaryExpression.h"
Expand Down Expand Up @@ -598,4 +599,14 @@ class SparqlQleverVisitor {
void warnOrThrowIfUnboundVariables(auto* ctx,
const SparqlExpressionPimpl& expression,
std::string_view clauseName);

// Convert an instance of `Triples` to a `BasicGraphPattern` so it can be used
// just like a WHERE clause. Most of the time this just changes the type and
// stays semantically the same, but for blank nodes, this step converts them
// into internal variables so they are interpreted correctly by the query
// planner.
static parsedQuery::BasicGraphPattern toGraphPattern(
const ad_utility::sparql_types::Triples& triples);

FRIEND_TEST(SparqlParser, ensureExceptionOnInvalidGraphTerm);
};
37 changes: 34 additions & 3 deletions test/SparqlAntlrParserTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1215,16 +1215,47 @@ TEST(SparqlParser, ConstructQuery) {
m::pq::LimitOffset({10}), m::pq::OrderKeys({{Var{"?a"}, false}})));
// This case of the grammar is not useful without Datasets, but we still
// support it.
expectConstructQuery("CONSTRUCT WHERE { ?a <foo> ?b }",
m::ConstructQuery({{Var{"?a"}, Iri{"<foo>"}, Var{"?b"}}},
m::GraphPattern()));
expectConstructQuery(
"CONSTRUCT WHERE { ?a <foo> ?b }",
m::ConstructQuery(
{{Var{"?a"}, Iri{"<foo>"}, Var{"?b"}}},
m::GraphPattern(m::Triples({{Var{"?a"}, "<foo>", Var{"?b"}}}))));

// Blank nodes turn into variables inside WHERE.
expectConstructQuery(
"CONSTRUCT WHERE { [] <foo> ?b }",
m::ConstructQuery(
{{BlankNode{true, "0"}, Iri{"<foo>"}, Var{"?b"}}},
m::GraphPattern(m::Triples(
{{Var{absl::StrCat(QLEVER_INTERNAL_BLANKNODE_VARIABLE_PREFIX,
"g_0")},
"<foo>", Var{"?b"}}}))));

// Test another variant to cover all cases.
expectConstructQuery(
"CONSTRUCT WHERE { <bar> ?foo \"Abc\"@en }",
m::ConstructQuery(
{{Iri{"<bar>"}, Var{"?foo"}, Literal{"\"Abc\"@en"}}},
m::GraphPattern(m::Triples(
{{iri("<bar>"), PropertyPath::fromVariable(Var{"?foo"}),
lit("\"Abc\"", "@en")}}))));
// CONSTRUCT with datasets.
expectConstructQuery(
"CONSTRUCT { } FROM <foo> FROM NAMED <foo2> FROM NAMED <foo3> WHERE { }",
m::ConstructQuery({}, m::GraphPattern(), m::Graphs{iri("<foo>")},
m::Graphs{iri("<foo2>"), iri("<foo3>")}));
}

// _____________________________________________________________________________
TEST(SparqlParser, ensureExceptionOnInvalidGraphTerm) {
EXPECT_THROW(SparqlQleverVisitor::toGraphPattern(
{{Var{"?a"}, BlankNode{true, "0"}, Var{"?b"}}}),
ad_utility::Exception);
EXPECT_THROW(SparqlQleverVisitor::toGraphPattern(
{{Var{"?a"}, Literal{"\"Abc\""}, Var{"?b"}}}),
ad_utility::Exception);
}

// Test that ASK queries are parsed as they should.
TEST(SparqlParser, AskQuery) {
// Some helper functions and abbreviations.
Expand Down

0 comments on commit a307781

Please sign in to comment.