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

Starting 'Select *' implementation #546

Merged
merged 14 commits into from
Jan 27, 2022
Merged
45 changes: 27 additions & 18 deletions src/engine/QueryExecutionTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,28 +99,30 @@ QueryExecutionTree::generateResults(const SelectedVarsOrAsterisk & selectedVarsO
LOG(DEBUG) << "Resolving strings for finished binary result...\n";
vector<std::optional<pair<size_t, ResultTable::ResultType>>> validIndices;
if(selectedVarsOrAsterisk.isAsterisk()) {
list<string> orderedVariables = selectedVarsOrAsterisk.retrieveOrder();
auto allVars = getVariableColumns();
for (const auto& var : orderedVariables) {
auto it = allVars.find(var);
if (it != allVars.end()) {
auto orderedVariablesFromQuery = selectedVarsOrAsterisk.retrieveOrder();
auto variablesFromExecutionTree = getVariableColumns();
for (const auto& variableFromQuery : orderedVariablesFromQuery) {
auto it = variablesFromExecutionTree.find(variableFromQuery);
if (it != variablesFromExecutionTree.end()) {
validIndices.emplace_back(pair<size_t, ResultTable::ResultType>(
it->second, resultTable->getResultType(it->second)));
allVars.erase(it);
variablesFromExecutionTree.erase(it);
} else {
Copy link
Member

Choose a reason for hiding this comment

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

This else branch should also warn:
OG(WARN) << "The variable "" << variableFromQuery <<
"" was found in the original, query, but not in the execution tree. "
"This is likely a bug\n";

validIndices.emplace_back(std::nullopt);
}
}
for(const auto& var : allVars){
LOG(DEBUG) << "Variable " << var.first << " was not parsed!! \n";
for(const auto& variableFromQuery : variablesFromExecutionTree){
LOG(WARN) << "The variable \"" << variableFromQuery.first <<
"\" was found in the execution tree, but not in the original query. "
"This is likely a bug\n";
}
}
else {
for (auto var : selectedVarsOrAsterisk.getSelectVariables()) {
if (ad_utility::startsWith(var, "TEXT(")) {
var = var.substr(5, var.rfind(')') - 5);
for (auto variableFromQuery : selectedVarsOrAsterisk.getSelectVariables()) {
if (ad_utility::startsWith(variableFromQuery, "TEXT(")) {
variableFromQuery = variableFromQuery.substr(5, variableFromQuery.rfind(')') - 5);
}
auto it = getVariableColumns().find(var);
auto it = getVariableColumns().find(variableFromQuery);
if (it != getVariableColumns().end()) {
validIndices.emplace_back(pair<size_t, ResultTable::ResultType>(
it->second, resultTable->getResultType(it->second)));
Expand Down Expand Up @@ -156,19 +158,26 @@ template ad_utility::stream_generator::stream_generator QueryExecutionTree::
// ___________________________________________________________________________
QueryExecutionTree::ColumnIndicesAndTypes
QueryExecutionTree::selectedVariablesToColumnIndices(
SelectedVarsOrAsterisk selectedVarsOrAsterisk,
const SelectedVarsOrAsterisk & selectedVarsOrAsterisk,
const ResultTable& resultTable) const {
ColumnIndicesAndTypes exportColumns;
if(selectedVarsOrAsterisk.isAsterisk()) {
for(auto var: selectedVarsOrAsterisk.retrieveOrder()){
if (getVariableColumns().contains(var)) {
auto columnIndex = getVariableColumns().at(var);
auto variablesFromExecutionTree = getVariableColumns();
for(auto variableFromQuery: selectedVarsOrAsterisk.retrieveOrder()){
if (getVariableColumns().contains(variableFromQuery)) {
auto columnIndex = getVariableColumns().at(variableFromQuery);
exportColumns.push_back(VariableAndColumnIndex{
var, columnIndex, resultTable.getResultType(columnIndex)});
variableFromQuery, columnIndex, resultTable.getResultType(columnIndex)});
variablesFromExecutionTree.erase(variableFromQuery);
} else {
exportColumns.emplace_back(std::nullopt);
Copy link
Member

Choose a reason for hiding this comment

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

Also add the complementary warning here.

}
}
for(const auto& variableFromQuery : variablesFromExecutionTree){
LOG(WARN) << "The variable \"" << variableFromQuery.first <<
"\" was found in the execution tree, but not in the original query. "
"This is likely a bug\n";
}
}
else {
for (auto var : selectedVarsOrAsterisk.getSelectVariables()) {
Expand Down Expand Up @@ -285,7 +294,7 @@ nlohmann::json QueryExecutionTree::writeResultAsSparqlJson(
};

for (size_t rowIndex = offset; rowIndex < upperBound; ++rowIndex) {
// Due to be an 'nlohmann' object, object keys are alphabetically sorted!
// TODO: ordered_json` entries are ordered alphabetically, but insertion order would be preferable.
nlohmann::ordered_json binding;
for (const auto& column : columns) {
const auto& currentId = idTable(rowIndex, column->_columnIndex);
Expand Down
2 changes: 1 addition & 1 deletion src/engine/QueryExecutionTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class QueryExecutionTree {
// Returns a vector where the i-th element contains the column index and
// `ResultType` of the i-th `selectVariable` in the `resultTable`
ColumnIndicesAndTypes selectedVariablesToColumnIndices(
SelectedVarsOrAsterisk selectedVarsOrAsterisk,
const SelectedVarsOrAsterisk & selectedVarsOrAsterisk,
const ResultTable& resultTable) const;

template <ExportSubFormat format>
Expand Down
4 changes: 1 addition & 3 deletions src/engine/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,7 @@ Awaitable<json> Server::composeResponseQleverJson(
j["warnings"] = qet.collectWarnings();
if(query.hasSelectClause()){
if(query.selectClause()._varsOrAsterisk.isAsterisk()) {
auto list = query.selectClause()._varsOrAsterisk.retrieveOrder();
std::vector<string> result{ list.begin(),list.end() };
j["selected"] = result;
j["selected"] = query.selectClause()._varsOrAsterisk.retrieveOrder();
}
else {
j["selected"] = query.selectClause()._varsOrAsterisk.getSelectVariables();
Expand Down
2 changes: 1 addition & 1 deletion src/parser/ParsedQuery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ string ParsedQuery::asString() const {
os << "\nSELECT: {\n\t";
if(usesAsterisk) {
auto list = selectClause._varsOrAsterisk.retrieveOrder();
std::list<string>::iterator it;
std::vector<string>::iterator it;
Copy link
Member

Choose a reason for hiding this comment

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

still auto

Copy link
Member

Choose a reason for hiding this comment

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

(I'm sorry, I think one of my comments got lost here).

for (it = list.begin(); it != list.end(); ){
os << it->c_str();
if (++it != list.end()) {
Expand Down
40 changes: 10 additions & 30 deletions src/parser/ParsedQuery.h
Original file line number Diff line number Diff line change
Expand Up @@ -308,24 +308,14 @@ class ParsedQuery {
};

typedef std::variant<vector<string>,char> SelectVarsOrAsterisk;
// Representation of the 'Selector: All (*)' xor 'Selector: (VarName)+'
// template <typename T>
// Represents either "all Variables" (Select *) or a list of explicitly
// selected Variables (Select ?a ?b).
struct SelectedVarsOrAsterisk {
private:
SelectVarsOrAsterisk _varsOrAsterisk;
std::list<string> _variablesOrder;

void setAsterisk() {
_varsOrAsterisk = '*';
}
std::vector<string> _variablesOrder;
Copy link
Member

Choose a reason for hiding this comment

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

Call this member _variablesFromQueryBody


public:
/*
// Clone of the private variable typed 'variant' (vector)
[[nodiscard]] SelectVarsOrAsterisk get() const {
return SelectVarsOrAsterisk{_varsOrAsterisk};
}
*/

[[nodiscard]] bool isAsterisk() const {
return std::holds_alternative<char>(_varsOrAsterisk);
Expand All @@ -337,15 +327,8 @@ class ParsedQuery {

// Sets the Selector to 'All' (*) only if the Selector is still undefined
// Returned value maybe unused due to Syntax Check
[[maybe_unused]] bool setsAsterisk() {
/*
* Needs (std::monostate_t) but unnecessary due to Syntax Check
*/
// if (!isAsterisk() && !isVariables()) {
setAsterisk();
return true;
// }
// else return false;
void setsAsterisk() {
_varsOrAsterisk = '*';
}
joka921 marked this conversation as resolved.
Show resolved Hide resolved

[[nodiscard]] char getAsterisk() const {
Expand All @@ -360,28 +343,25 @@ class ParsedQuery {
return std::get<std::vector<string>>(_varsOrAsterisk);
}

void pushVariablesOrder(const string& variable) {
// Add a variable, that was found in the query body. The added variables
// will only be used if `isAsterisk` is true.
void addVariableFromQueryBody (const string& variable) {
if(!(std::find(_variablesOrder.begin(),
joka921 marked this conversation as resolved.
Show resolved Hide resolved
_variablesOrder.end(),
variable) != _variablesOrder.end())) {
_variablesOrder.emplace_back(variable);
}
}


[[nodiscard]] std::list<string>& retrieveOrder() {
return _variablesOrder;
}

[[nodiscard]] std::list<string> retrieveOrder() const {
[[nodiscard]] auto retrieveOrder() const {
Copy link
Member

Choose a reason for hiding this comment

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

Here one of my comments disappeared:
Call this method orderedVariablesFromQueryBody and comment it:
// get the variables which addVariableFromQueryBody` was previously called. The result contains no duplicates and is ordered by the first appearance in the query body.

return _variablesOrder;
}
};

joka921 marked this conversation as resolved.
Show resolved Hide resolved
// Represents the Select Clause with all the possible outcomes
struct SelectClause {
// `_aliases` will be empty if Selector '*' is present.
// Essentially, there is a `SELECT *` clause in the query.
// This means, that there is a `SELECT *` clause in the query.
std::vector<Alias> _aliases;
SelectedVarsOrAsterisk _varsOrAsterisk;
bool _reduced = false;
Expand Down
16 changes: 8 additions & 8 deletions src/parser/SparqlParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ void SparqlParser::parseSelect(ParsedQuery* query) {
if (_lexer.accept(SparqlToken::Type::VARIABLE)) {
// Exception avoided due to previous Syntax Check of Selector '*'
selectClause._varsOrAsterisk.getSelectVariables().push_back(_lexer.current().raw);
selectClause._varsOrAsterisk.pushVariablesOrder(_lexer.current().raw);
selectClause._varsOrAsterisk.addVariableFromQueryBody(_lexer.current().raw);
} else if (_lexer.accept("text")) {
_lexer.expect("(");
std::ostringstream s;
Expand All @@ -198,7 +198,7 @@ void SparqlParser::parseSelect(ParsedQuery* query) {
ParsedQuery::Alias a = parseAliasWithAntlr();
selectClause._aliases.push_back(a);
selectClause._varsOrAsterisk.getSelectVariables().emplace_back(a._outVarName);
selectClause._varsOrAsterisk.pushVariablesOrder(a._outVarName);
selectClause._varsOrAsterisk.addVariableFromQueryBody(a._outVarName);
_lexer.expect(")");
} else {
_lexer.accept();
Expand Down Expand Up @@ -287,7 +287,7 @@ void SparqlParser::parseWhere(ParsedQuery* query,
GraphPatternOperation::Bind bind{parseExpressionWithAntlr()};
_lexer.expect("as");
_lexer.expect(SparqlToken::Type::VARIABLE);
query->selectClause()._varsOrAsterisk.pushVariablesOrder(_lexer.current().raw);
query->selectClause()._varsOrAsterisk.addVariableFromQueryBody(_lexer.current().raw);
bind._target = _lexer.current().raw;
_lexer.expect(")");
currentPattern->_children.emplace_back(std::move(bind));
Expand Down Expand Up @@ -346,7 +346,7 @@ void SparqlParser::parseWhere(ParsedQuery* query,
// values with several variables
while (_lexer.accept(SparqlToken::Type::VARIABLE)) {
values._variables.push_back(_lexer.current().raw);
query->selectClause()._varsOrAsterisk.pushVariablesOrder(_lexer.current().raw);
query->selectClause()._varsOrAsterisk.addVariableFromQueryBody(_lexer.current().raw);
}
_lexer.expect(")");
_lexer.expect("{");
Expand All @@ -364,7 +364,7 @@ void SparqlParser::parseWhere(ParsedQuery* query,
} else if (_lexer.accept(SparqlToken::Type::VARIABLE)) {
// values with a single variable
values._variables.push_back(_lexer.current().raw);
query->selectClause()._varsOrAsterisk.pushVariablesOrder(_lexer.current().raw);
query->selectClause()._varsOrAsterisk.addVariableFromQueryBody(_lexer.current().raw);
_lexer.expect("{");
while (_lexer.accept(SparqlToken::Type::IRI) ||
_lexer.accept(SparqlToken::Type::RDFLITERAL)) {
Expand All @@ -385,7 +385,7 @@ void SparqlParser::parseWhere(ParsedQuery* query,
if (lastSubject.empty()) {
if (_lexer.accept(SparqlToken::Type::VARIABLE)) {
subject = _lexer.current().raw;
query->selectClause()._varsOrAsterisk.pushVariablesOrder(_lexer.current().raw);
query->selectClause()._varsOrAsterisk.addVariableFromQueryBody(_lexer.current().raw);
} else if (_lexer.accept(SparqlToken::Type::RDFLITERAL)) {
subject = parseLiteral(_lexer.current().raw, true);
} else {
Expand All @@ -401,7 +401,7 @@ void SparqlParser::parseWhere(ParsedQuery* query,
if (lastPredicate.empty()) {
if (_lexer.accept(SparqlToken::Type::VARIABLE)) {
predicate = _lexer.current().raw;
query->selectClause()._varsOrAsterisk.pushVariablesOrder(_lexer.current().raw);
query->selectClause()._varsOrAsterisk.addVariableFromQueryBody(_lexer.current().raw);
} else if (_lexer.accept(SparqlToken::Type::RDFLITERAL)) {
predicate = parseLiteral(_lexer.current().raw, true);
} else {
Expand All @@ -419,7 +419,7 @@ void SparqlParser::parseWhere(ParsedQuery* query,
std::string object;
if (_lexer.accept(SparqlToken::Type::VARIABLE)) {
object = _lexer.current().raw;
query->selectClause()._varsOrAsterisk.pushVariablesOrder(_lexer.current().raw);
query->selectClause()._varsOrAsterisk.addVariableFromQueryBody(_lexer.current().raw);
} else if (_lexer.accept(SparqlToken::Type::RDFLITERAL)) {
object = parseLiteral(_lexer.current().raw, true);
} else {
Expand Down