Skip to content

Commit

Permalink
Further test improvements. Small clean ups
Browse files Browse the repository at this point in the history
  • Loading branch information
@Flixtastic
Flixtastic committed Feb 24, 2025
1 parent dd7b2e0 commit 8b7921e
Show file tree
Hide file tree
Showing 6 changed files with 209 additions and 54 deletions.
5 changes: 2 additions & 3 deletions src/engine/TextIndexScanForEntity.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <string>

#include "engine/Operation.h"
#include "parser/MagicServiceQuery.h"
#include "parser/TextSearchQuery.h"

// This operation retrieves all text records and their corresponding
Expand Down Expand Up @@ -60,8 +59,6 @@ class TextIndexScanForEntity : public Operation {

vector<ColumnIndex> resultSortedOn() const override;

void setVariableToColumnMap();

VariableToColumnMap computeVariableToColumnMap() const override;

const TextIndexScanForEntityConfiguration& getConfig() const {
Expand All @@ -79,4 +76,6 @@ class TextIndexScanForEntity : public Operation {
ProtoResult computeResult([[maybe_unused]] bool requestLaziness) override;

vector<QueryExecutionTree*> getChildren() override { return {}; }

void setVariableToColumnMap();
};
2 changes: 2 additions & 0 deletions src/engine/TextIndexScanForWord.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ void TextIndexScanForWord::setVariableToColumnMap() {
"Text index scan for word shouldn't have a variable to bind match "
"defined when the word is not a prefix.");

Check warning on line 72 in src/engine/TextIndexScanForWord.cpp

View check run for this annotation

Codecov / codecov/patch

src/engine/TextIndexScanForWord.cpp#L70-L72 

Added lines #L70 - L72 were not covered by tests
}
AD_CORRECTNESS_CHECK(config_.isPrefix_ ||
!config_.varToBindMatch_.has_value());
if (config_.varToBindScore_.has_value()) {
config_.variableColumns_.value()[config_.varToBindScore_.value()] =
makeAlwaysDefinedColumn(index);
Expand Down
5 changes: 2 additions & 3 deletions src/engine/TextIndexScanForWord.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <string>

#include "engine/Operation.h"
#include "parser/MagicServiceQuery.h"
#include "parser/TextSearchQuery.h"

// This operation retrieves all text records from the fulltext index that
Expand Down Expand Up @@ -45,8 +44,6 @@ class TextIndexScanForWord : public Operation {

vector<ColumnIndex> resultSortedOn() const override;

void setVariableToColumnMap();

VariableToColumnMap computeVariableToColumnMap() const override;

const TextIndexScanForWordConfiguration& getConfig() const { return config_; }
Expand All @@ -59,4 +56,6 @@ class TextIndexScanForWord : public Operation {
ProtoResult computeResult([[maybe_unused]] bool requestLaziness) override;

vector<QueryExecutionTree*> getChildren() override { return {}; }

void setVariableToColumnMap();
};
91 changes: 73 additions & 18 deletions src/parser/TextSearchQuery.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,24 @@
#include "parser/MagicServiceIriConstants.h"
#include "util/http/HttpParser/AcceptHeaderQleverVisitor.h"

std::variant<Variable, FixedEntity> VarOrFixedEntity::makeEntityVariant(
const QueryExecutionContext* qec,
std::variant<Variable, std::string> entity) {
if (std::holds_alternative<std::string>(entity)) {
VocabIndex index;
std::string fixedEntity = std::move(std::get<std::string>(entity));
bool success = qec->getIndex().getVocab().getId(fixedEntity, &index);
if (!success) {
throw std::runtime_error(
"The entity " + fixedEntity +
" is not part of the underlying knowledge graph and can "
"therefore not be used as the object of ql:contains-entity");
}
return FixedEntity(std::move(fixedEntity), std::move(index));
}
return std::get<Variable>(entity);
};

namespace parsedQuery {

// ____________________________________________________________________________
Expand All @@ -17,7 +35,7 @@ void TextSearchQuery::throwSubjectVariableException(
if (!subject.isVariable()) {
throw TextSearchException(
absl::StrCat("The predicate <", predString,
"> needs a Variable as subject. The subject given was: ",
"> needs a variable as subject. The subject given was: ",
subject.toString()));
}
}
Expand All @@ -29,7 +47,7 @@ void TextSearchQuery::throwSubjectAndObjectVariableException(
if (!(subject.isVariable() && object.isVariable())) {
throw TextSearchException(absl::StrCat(
"The predicate <", predString,
"> needs a Variable as subject and one as object. The subject given "
"> needs a variable as subject and one as object. The subject given "
"was: ",
subject.toString(), ". The object given was: ", object.toString()));
}
Expand All @@ -51,7 +69,7 @@ void TextSearchQuery::throwObjectLiteralException(
if (!object.isLiteral()) {
throw TextSearchException(
absl::StrCat("The predicate <", predString,
"> needs a Literal as object. The object given was: ",
"> needs a literal as object. The object given was: ",
object.toString()));
}
}
Expand All @@ -62,7 +80,7 @@ void TextSearchQuery::predStringTextSearch(const Variable& subjectVar,
if (configVarToConfigs_[objectVar].textVar_.has_value()) {
throw TextSearchException(absl::StrCat(
"Each text search config should only be linked to a single text "
"Variable. The second text variable given was: ",
"variable. The second text variable given was: ",
subjectVar.name(), ". The config variable was: ", objectVar.name()));
}
configVarToConfigs_[objectVar].textVar_ = subjectVar;
Expand All @@ -75,7 +93,7 @@ void TextSearchQuery::predStringContainsWord(
std::string_view literal = asStringViewUnsafe(objectLiteral.getContent());
if (literal.empty()) {
throw TextSearchException(
"The predicate <contains-word> shouldn't have an empty Literal as "
"The predicate <contains-word> shouldn't have an empty literal as "
"object.");
}
configVarToConfigs_[subjectVar].word_ = literal;
Expand All @@ -95,8 +113,8 @@ void TextSearchQuery::predStringContainsEntity(const Variable& subjectVar,
object.getIri().toStringRepresentation();

Check warning on line 113 in src/parser/TextSearchQuery.cpp

View check run for this annotation

Codecov / codecov/patch

src/parser/TextSearchQuery.cpp#L112-L113 

Added lines #L112 - L113 were not covered by tests
} else {
throw TextSearchException(absl::StrCat(
"The predicate <contains-entity> needs a Variable as subject and an "
"IRI, Literal or Variable as object. The object given was: ",
"The predicate <contains-entity> needs a variable as subject and an "
"IRI, literal or variable as object. The object given was: ",
object.toString()));
}
}
Expand All @@ -105,8 +123,10 @@ void TextSearchQuery::predStringContainsEntity(const Variable& subjectVar,
void TextSearchQuery::predStringBindMatch(const Variable& subjectVar,
const Variable& objectVar) {
if (configVarToConfigs_[subjectVar].varToBindMatch_.has_value()) {
throw TextSearchException(
"A text search should only contain one <bind-match>.");
throw TextSearchException(absl::StrCat(
"Each text search config should only contain at most one <bind-match>. "
"The second match variable given was: ",
objectVar.name(), ". The config variable was: ", subjectVar.name()));
}
configVarToConfigs_[subjectVar].varToBindMatch_ = objectVar;
}
Expand All @@ -115,8 +135,10 @@ void TextSearchQuery::predStringBindMatch(const Variable& subjectVar,
void TextSearchQuery::predStringBindScore(const Variable& subjectVar,
const Variable& objectVar) {
if (configVarToConfigs_[subjectVar].varToBindScore_.has_value()) {
throw TextSearchException(
"A text search should only contain one <bind-score>.");
throw TextSearchException(absl::StrCat(
"Each text search config should only contain at most one <bind-score>. "
"The second match variable given was: ",
objectVar.name(), ". The config variable was: ", subjectVar.name()));
}
configVarToConfigs_[subjectVar].varToBindScore_ = objectVar;
}
Expand All @@ -129,7 +151,6 @@ void TextSearchQuery::addParameter(const SparqlTriple& triple) {
const TripleComponent& object = simpleTriple.o_;

auto predString = extractParameterName(predicate, TEXT_SEARCH_IRI);

if (predString == "text-search") {
throwSubjectAndObjectVariableException("text-search", subject, object);
predStringTextSearch(subject.getVariable(), object.getVariable());
Expand Down Expand Up @@ -162,15 +183,49 @@ TextSearchQuery::toConfigs(const QueryExecutionContext* qec) const {
ad_utility::HashMap<Variable, vector<string>> potentialTermsForCvar;
for (const auto& [var, conf] : configVarToConfigs_) {
if (!conf.isWordSearch_.has_value()) {
throw TextSearchException(
throw TextSearchException(absl::StrCat(
"Text search service needs configs with exactly one occurrence of "
"either <contains-word> or "
"<contains-entity>.");
"either <contains-word> or <contains-entity>. The config variable "
"was: ",
var.name()));
}
if (!conf.textVar_.has_value()) {
AD_FAIL();
// Last updated this error message 24.02.2025
throw TextSearchException(absl::StrCat(
"Text search service needs a text variable that is linked to one or "
"multiple text search config variables with the predicate "
"<text-search>. \n"
"The config variable can then be used with the predicates: "
"<contains-word>, <contains-entity>, <bind-match>, <bind-score>. \n"
"<contains-word>: This predicate needs a literal as object which has "
"one word with optionally a * at the end. This word or prefix is "
"then used to search the text index. \n"
"<contains-entity>: This predicate needs a variable, IRI or literal "
"as object. If a variable is given this variable can be used outside "
"of this service. If an IRI or literal is given the entity is fixed. "
"The entity given is then used to search the text index. \n"
"A config should contain exactly one occurrence of either "
"<contains-word> or <contains-entity>. \n"
"<bind-match>: This predicate should only be used in a text search "
"config with a word that is a prefix. The object should be a "
"variable. That variable specifies the column name for the column "
"with prefix matches of the word search.\n"
"<bind-score>: The object of this predicate should be a variable. "
"That variable specifies the column name for the column containing "
"the scores of the respective word or entity search. \n",
"The config variable was: ", var.name()));
}
if (conf.isWordSearch_.value()) {
if (conf.varToBindMatch_.has_value() &&
!conf.word_.value().ends_with("*")) {
throw TextSearchException(
absl::StrCat("The text search config shouldn't define a variable "
"for the prefix match column if the word isn't a "
"prefix. The config variable was: ",
var.name(), ". The word was: \"", conf.word_.value(),
"\". The text variable bound to was: ",
conf.textVar_.value().name()));
}
potentialTermsForCvar[conf.textVar_.value()].push_back(
conf.word_.value());
}
Expand All @@ -185,8 +240,8 @@ TextSearchQuery::toConfigs(const QueryExecutionContext* qec) const {
auto it = potentialTermsForCvar.find(conf.textVar_.value());
if (it == potentialTermsForCvar.end()) {
throw TextSearchException(absl::StrCat(
"Entity search has to happen on a text Variable that is also "
"contained in a word search. Text Variable: ",
"Entity search has to happen on a text variable that is also "
"contained in a word search. Text variable: ",
conf.textVar_.value().name(),
" is not contained in a word search."));
}
Expand Down
19 changes: 1 addition & 18 deletions src/parser/TextSearchQuery.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,29 +25,12 @@ struct VarOrFixedEntity {

static std::variant<Variable, FixedEntity> makeEntityVariant(
const QueryExecutionContext* qec,
std::variant<Variable, std::string> entity) {
if (std::holds_alternative<std::string>(entity)) {
VocabIndex index;
std::string fixedEntity = std::move(std::get<std::string>(entity));
bool success = qec->getIndex().getVocab().getId(fixedEntity, &index);
if (!success) {
throw std::runtime_error(
"The entity " + fixedEntity +
" is not part of the underlying knowledge graph and can "
"therefore not be used as the object of ql:contains-entity");
}
return FixedEntity(std::move(fixedEntity), std::move(index));
} else {
return std::get<Variable>(entity);
}
};
std::variant<Variable, std::string> entity);

VarOrFixedEntity(const QueryExecutionContext* qec,
std::variant<Variable, std::string> entity)
: entity_(makeEntityVariant(qec, std::move(entity))) {}

~VarOrFixedEntity() = default;

bool hasFixedEntity() const {
return std::holds_alternative<FixedEntity>(entity_);
}
Expand Down
Loading

0 comments on commit 8b7921e

Please sign in to comment.