8000 fixed issue #4185: On execution of FULLTEXT search / AQL query db is … · arangodb/arangodb@653f95f · GitHub
[go: up one dir, main page]
Skip to content

Commit 653f95f

Browse files
jsteemannjsteemann
authored
fixed issue #4185: On execution of FULLTEXT search / AQL query db is … (#4238)
1 parent 37da067 commit 653f95f

14 files changed

+129
-85
lines changed

‎arangod/Aql/OptimizerRules.cpp

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
#include "Graph/TraverserOptions.h"
5353
#include "Indexes/Index.h"
5454
#include "Transaction/Methods.h"
55+
#include "VocBase/Methods/Collections.h"
5556

5657
#include <boost/optional.hpp>
5758
#include <tuple>
@@ -4489,10 +4490,14 @@ void arangodb::aql::inlineSubqueriesRule(Optimizer* opt,
44894490
opt->addPlan(std::move(plan), rule, modified);
44904491
}
44914492

4492-
static bool isValueTypeString(AstNode* node) {
4493+
static bool isValueTypeString(AstNode const* node) {
44934494
return (node->type == NODE_TYPE_VALUE && node->value.type == VALUE_TYPE_STRING);
44944495
}
44954496

4497+
static bool isValueTypeCollection(AstNode const* node) {
4498+
return node->type == NODE_TYPE_COLLECTION || isValueTypeString(node);
4499+
}
4500+
44964501
static bool isValueTypeNumber(AstNode* node) {
44974502
return (node->type == NODE_TYPE_VALUE && (node->value.type == VALUE_TYPE_INT ||
44984503
node->value.type == VALUE_TYPE_DOUBLE));
@@ -4535,40 +4540,39 @@ static bool applyFulltextOptimization(EnumerateListNode* elnode,
45354540
AstNode* attrArg = fargs->getMember(1);
45364541
AstNode* queryArg = fargs->getMember(2);
45374542
AstNode* limitArg = fargs->numMembers() == 4 ? fargs->getMember(3) : nullptr;
4538-
if ((collArg->type != NODE_TYPE_COLLECTION && !isValueTypeString(collArg)) ||
4539-
!isValueTypeString(attrArg) || !isValueTypeString(queryArg) ||
4543+
if (!isValueTypeCollection(collArg) || !isValueTypeString(attrArg) ||
4544+
!isValueTypeString(queryArg) || // (... || queryArg->type == NODE_TYPE_REFERENCE)
45404545
(limitArg != nullptr && !isValueTypeNumber(limitArg))) {
45414546
return false;
45424547
}
45434548

45444549
std::string name = collArg->getString();
45454550
TRI_vocbase_t* vocbase = plan->getAst()->query()->vocbase();
4546-
aql::Collections* colls = plan->getAst()->query()->collections();
4547-
aql::Collection const* coll = colls->add(name, AccessMode::Type::READ);
45484551
std::vector<basics::AttributeName> field;
45494552
TRI_ParseAttributeString(attrArg->getString(), field, /*allowExpansion*/false);
45504553
if (field.empty()) {
45514554
return false;
45524555
}
45534556

4554-
//check for suitiable indexes
4555-
std::shared_ptr<LogicalCollection> logical = coll->getCollection();
4557+
// check for suitable indexes
45564558
std::shared_ptr<arangodb::Index> index;
4557-
for (auto idx : logical->getIndexes()) {
4558-
if (idx->type() == arangodb::Index::IndexType::TRI_IDX_TYPE_FULLTEXT_INDEX) {
4559-
TRI_ASSERT(idx->fields().size() == 1);
4560-
if (basics::AttributeName::isIdentical(idx->fields()[0], field, false)) {
4561-
index = idx;
4562-
break;
4559+
methods::Collections::lookup(vocbase, name, [&](LogicalCollection* logical) {
4560+
for (auto idx : logical->getIndexes()) {
4561+
if (idx->type() == arangodb::Index::IndexType::TRI_IDX_TYPE_FULLTEXT_INDEX) {
4562+
TRI_ASSERT(idx->fields().size() == 1);
4563+
if (basics::AttributeName::isIdentical(idx->fields()[0], field, false)) {
4564+
index = idx;
4565+
break;
4566+
}
45634567
}
45644568
}
4565-
}
4569+
});
45664570
if (!index) { // no index found
45674571
return false;
45684572
}
45694573

45704574
Ast* ast = plan->getAst();
4571-
AstNode* args = ast->createNodeArray(2);
4575+
AstNode* args = ast->createNodeArray(3 + (limitArg != nullptr ? 0 : 1));
45724576
args->addMember(ast->clone(collArg)); // only so createNodeFunctionCall doesn't throw
45734577
args->addMember(attrArg);
45744578
args->addMember(queryArg);
@@ -4581,6 +4585,19 @@ static bool applyFulltextOptimization(EnumerateListNode* elnode,
45814585
condition->andCombine(cond);
45824586
condition->normalize(plan);
45834587

4588+
// we assume by now that collection `name` exists
4589+
aql::Collections* colls = plan->getAst()->query()->collections();
4590+
aql::Collection* coll = colls->get(name);
4591+
if (coll == nullptr) { // TODO: cleanup this mess
4592+
coll = colls->add(name, AccessMode::Type::READ);
4593+
if (!ServerState::instance()->isCoordinator()) {
4594+
TRI_ASSERT(coll != nullptr);
4595+
coll->setCollection(vocbase->lookupCollection(name));
4596+
// FIXME: does this need to happen in the coordinator?
4597+
plan->getAst()->query()->trx()->addCollectionAtRuntime(name);
4598+
}
4599+
}
4600+
45844601
auto indexNode = new IndexNode(plan, plan->nextId(), vocbase,
45854602
coll, elnode->outVariable(),
45864603
std::vector<transaction::Methods::IndexHandle>{
@@ -4589,6 +4606,9 @@ static bool applyFulltextOptimization(EnumerateListNode* elnode,
45894606
plan->registerNode(indexNode);
45904607
condition.release();
45914608
plan->replaceNode(elnode, indexNode);
4609+
// mark removable, because it will not throw for most params
4610+
// FIXME: technically we need to validate the query parameter
4611+
calcNode->canRemoveIfThrows(true);
45924612

45934613
if (limitArg != nullptr && limitNode == nullptr) { // add LIMIT
45944614
size_t limit = static_cast<size_t>(limitArg->getIntValue());
@@ -4603,6 +4623,7 @@ static bool applyFulltextOptimization(EnumerateListNode* elnode,
46034623
}
46044624
limitNode->addDependency(indexNode);
46054625
}
4626+
46064627
return true;
46074628
}
46084629

‎arangod/MMFiles/MMFilesAqlFunctions.cpp

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -178,31 +178,27 @@ AqlValue MMFilesAqlFunctions::Fulltext(
178178

179179
AqlValue collectionValue = ExtractFunctionParameterValue(parameters, 0);
180180
if (!collectionValue.isString()) {
181-
RegisterWarning(query, "FULLTEXT", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH);
182-
return AqlValue(AqlValueHintNull());
181+
THROW_ARANGO_EXCEPTION_PARAMS(TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH, "FULLTEXT");
183182
}
184183
std::string const cname(collectionValue.slice().copyString());
185184

186185
AqlValue attribute = ExtractFunctionParameterValue(parameters, 1);
187186
if (!attribute.isString()) {
188-
RegisterWarning(query, "FULLTEXT", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH);
189-
return AqlValue(AqlValueHintNull());
187+
THROW_ARANGO_EXCEPTION_PARAMS(TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH, "FULLTEXT");
190188
}
191189
std::string const attributeName(attribute.slice().copyString());
192190

193191
AqlValue queryValue = ExtractFunctionParameterValue(parameters, 2);
194192
if (!queryValue.isString()) {
195-
RegisterWarning(query, "FULLTEXT", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH);
196-
return AqlValue(AqlValueHintNull());
193+
THROW_ARANGO_EXCEPTION_PARAMS(TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH, "FULLTEXT");
197194
}
198195
std::string const queryString = queryValue.slice().copyString();
199196

200197
size_t maxResults = 0; // 0 means "all results"
201198
if (parameters.size() >= 4) {
202199
AqlValue limit = ExtractFunctionParameterValue(parameters, 3);
203200
if (!limit.isNull(true) && !limit.isNumber()) {
204-
RegisterWarning(query, "FULLTEXT", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH);
205-
return AqlValue(AqlValueHintNull());
201+
THROW_ARANGO_EXCEPTION_PARAMS(TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH, "FULLTEXT");
206202
}
207203
if (limit.isNumber()) {
208204
int64_t value = limit.toInt64(trx);
@@ -212,15 +208,16 @@ AqlValue MMFilesAqlFunctions::Fulltext(
212208
}
213209
}
214210

215-
// add the collection to the query for proper cache handling
216-
query->collections()->add(cname, AccessMode::Type::READ);
217211
TRI_voc_cid_t cid = trx->resolver()->getCollectionIdLocal(cname);
218-
trx->addCollectionAtRuntime(cid, cname);
219-
LogicalCollection* collection = trx->documentCollection(cid);
220-
if (collection == nullptr) {
221-
RegisterWarning(query, "FULLTEXT", TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND);
222-
return AqlValue(AqlValueHintNull());
212+
if (cid == 0) {
213+
THROW_ARANGO_EXCEPTION_FORMAT(TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND, "%s",
214+
cname.c_str());
223215
}
216+
// add the collection to the query for proper cache handling
217+
query->collections()->add(cname, AccessMode::Type::READ);
218+
trx->addCollectionAtRuntime(cid, cname, AccessMode::Type::READ);
219+
LogicalCollection const* collection = trx->documentCollection(cid);
220+
TRI_ASSERT(collection != nullptr);
224221

225222
// split requested attribute name on '.' character to create a proper
226223
// vector of AttributeNames
@@ -251,8 +248,8 @@ AqlValue MMFilesAqlFunctions::Fulltext(
251248

252249
if (fulltextIndex == nullptr) {
253250
// fiddle collection name into error message
254-
RegisterWarning(query, "FULLTEXT", TRI_ERROR_QUERY_FULLTEXT_INDEX_MISSING);
255-
return AqlValue(AqlValueHintNull());
251+
THROW_ARANGO_EXCEPTION_PARAMS(TRI_ERROR_QUERY_FULLTEXT_INDEX_MISSING,
252+
cname.c_str());
256253
}
257254

258255
trx->pinData(cid);
@@ -261,8 +258,7 @@ AqlValue MMFilesAqlFunctions::Fulltext(
261258
TRI_CreateQueryMMFilesFulltextIndex(TRI_FULLTEXT_SEARCH_MAX_WORDS, maxResults);
262259

263260
if (ft == nullptr) {
264-
RegisterWarning(query, "FULLTEXT", TRI_ERROR_OUT_OF_MEMORY);
265-
return AqlValue(AqlValueHintNull());
261+
THROW_ARANGO_EXCEPTION(TRI_ERROR_OUT_OF_MEMORY);
266262
}
267263

268264
bool isSubstringQuery = false;
@@ -271,8 +267,7 @@ AqlValue MMFilesAqlFunctions::Fulltext(
271267

272268
if (res != TRI_ERROR_NO_ERROR) {
273269
TRI_FreeQueryMMFilesFulltextIndex(ft);
274-
RegisterWarning(query, "FULLTEXT", res);
275-
return AqlValue(AqlValueHintNull());
270+
THROW_ARANGO_EXCEPTION(res);
276271
}
277272

278273
// note: the following call will free "ft"!
@@ -412,7 +407,7 @@ void MMFilesAqlFunctions::registerResources() {
412407
TRI_ASSERT(functions != nullptr);
413408

414409
// fulltext functions
415-
functions->add({"FULLTEXT", ".h,.,.|.", true, false,
410+
functions->add({"FULLTEXT", ".h,.,.|.", false, true,
416411
false, true, &MMFilesAqlFunctions::Fulltext,
417412
NotInCoordinator});
418413
functions->add({"NEAR", ".h,.,.|.,.", false, true, false,

‎arangod/MMFiles/MMFilesCollection.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1977,7 +1977,7 @@ Result MMFilesCollection::read(transaction::Methods* trx, StringRef const& key,
19771977

19781978
bool MMFilesCollection::readDocument(transaction::Methods* trx,
19791979
LocalDocumentId const& documentId,
1980-
ManagedDocumentResult& result) {
1980+
ManagedDocumentResult& result) const {
19811981
uint8_t const* vpack = lookupDocumentVPack(documentId);
19821982
if (vpack != nullptr) {
19831983
result.setUnmanaged(vpack, documentId);
@@ -1988,7 +1988,7 @@ bool MMFilesCollection::readDocument(transaction::Methods* trx,
19881988

19891989
bool MMFilesCollection::readDocumentWithCallback(transaction::Methods* trx,
19901990
LocalDocumentId const& documentId,
1991-
IndexIterator::DocumentCallback const& cb) {
1991+
IndexIterator::DocumentCallback const& cb) const {
19921992
uint8_t const* vpack = lookupDocumentVPack(documentId);
19931993
if (vpack != nullptr) {
19941994
cb(documentId, VPackSlice(vpack));
@@ -1999,7 +1999,7 @@ bool MMFilesCollection::readDocumentWithCallback(transaction::Methods* trx,
19991999

20002000
size_t MMFilesCollection::readDocumentWithCallback(transaction::Methods* trx,
20012001
std::vector<std::pair<LocalDocumentId, uint8_t const*>>& documentIds,
2002-
IndexIterator::DocumentCallback const& cb) {
2002+
IndexIterator::DocumentCallback const& cb){
20032003
size_t count = 0;
20042004
batchLookupRevisionVPack(documentIds);
20052005
for (auto const& it : documentIds) {

‎arangod/MMFiles/MMFilesCollection.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -339,11 +339,11 @@ class MMFilesCollection final : public PhysicalCollection {
339339

340340
bool readDocument(transaction::Methods* trx,
341341
LocalDocumentId const& documentId,
342-
ManagedDocumentResult& result) override;
342+
ManagedDocumentResult& result) const override;
343343

344344
bool readDocumentWithCallback(transaction::Methods* trx,
345345
LocalDocumentId const& documentId,
346-
IndexIterator::DocumentCallback const& cb) override;
346+
IndexIterator::DocumentCallback const& cb) const override;
347347

348348
size_t readDocumentWithCallback(transaction::Methods* trx,
349349
std::vector<std::pair<LocalDocumentId, uint8_t const*>>& documentIds,

‎arangod/RocksDBEngine/RocksDBAqlFunctions.cpp

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -51,36 +51,37 @@ static ExecutionCondition const NotInCoordinator = [] {
5151
AqlValue RocksDBAqlFunctions::Fulltext(
5252
arangodb::aql::Query* query, transaction::Methods* trx,
5353
VPackFunctionParameters const& parameters) {
54+
TRI_ASSERT(!ServerState::instance()->isCoordinator());
5455
ValidateParameters(parameters, "FULLTEXT", 3, 4);
5556

5657
AqlValue collectionValue = ExtractFunctionParameterValue(parameters, 0);
5758
if (!collectionValue.isString()) {
58-
RegisterWarning(query, "FULLTEXT", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH);
59-
return AqlValue(AqlValueHintNull());
59+
THROW_ARANGO_EXCEPTION_PARAMS(
60+
TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH, "FULLTEXT");
6061
}
6162

6263
std::string const cname(collectionValue.slice().copyString());
6364

6465
AqlValue attribute = ExtractFunctionParameterValue(parameters, 1);
6566
if (!attribute.isString()) {
66-
RegisterWarning(query, "FULLTEXT", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH);
67-
return AqlValue(AqlValueHintNull());
67+
THROW_ARANGO_EXCEPTION_PARAMS(
68+
TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH, "FULLTEXT");
6869
}
6970
std::string const attributeName(attribute.slice().copyString());
7071

7172
AqlValue queryValue = ExtractFunctionParameterValue(parameters, 2);
7273
if (!queryValue.isString()) {
73-
RegisterWarning(query, "FULLTEXT", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH);
74-
return AqlValue(AqlValueHintNull());
74+
THROW_ARANGO_EXCEPTION_PARAMS(
75+
TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH, "FULLTEXT");
7576
}
7677
std::string const queryString = queryValue.slice().copyString();
7778

7879
size_t maxResults = 0; // 0 means "all results"
7980
if (parameters.size() >= 4) {
8081
AqlValue limit = ExtractFunctionParameterValue(parameters, 3);
8182
if (!limit.isNull(true) && !limit.isNumber()) {
82-
RegisterWarning(query, "FULLTEXT", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH);
83-
return AqlValue(AqlValueHintNull());
83+
THROW_ARANGO_EXCEPTION_PARAMS(
84+
TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH, "FULLTEXT");
8485
}
8586
if (limit.isNumber()) {
8687
int64_t value = limit.toInt64(trx);
@@ -90,15 +91,16 @@ AqlValue RocksDBAqlFunctions::Fulltext(
9091
}
9192
}
9293

94+
TRI_voc_cid_t cid = trx->resolver()->getCollectionIdLocal(cname);
95+
if (cid == 0) {
96+
THROW_ARANGO_EXCEPTION_FORMAT(TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND, "%s",
97+
cname.c_str());
98+
}
9399
// add the collection to the query for proper cache handling
94100
query->collections()->add(cname, AccessMode::Type::READ);
95-
TRI_voc_cid_t cid = trx->resolver()->getCollectionIdLocal(cname);
96101
trx->addCollectionAtRuntime(cid, cname);
97-
LogicalCollection* collection = trx->documentCollection(cid);
98-
if (collection == nullptr) {
99-
RegisterWarning(query, "FULLTEXT", TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND);
100-
return AqlValue(AqlValueHintNull());
101-
}
102+
LogicalCollection const* collection = trx->documentCollection(cid);
103+
TRI_ASSERT(collection != nullptr);
102104

103105
// NOTE: The shared_ptr is protected by trx lock.
104106
// It is save to use the raw pointer directly.
@@ -127,23 +129,21 @@ AqlValue RocksDBAqlFunctions::Fulltext(
127129

128130
if (fulltextIndex == nullptr) {
129131
// fiddle collection name into error message
130-
RegisterWarning(query, "FULLTEXT", TRI_ERROR_QUERY_FULLTEXT_INDEX_MISSING);
131-
return AqlValue(AqlValueHintNull());
132+
THROW_ARANGO_EXCEPTION_PARAMS(TRI_ERROR_QUERY_FULLTEXT_INDEX_MISSING,
133+
cname.c_str());
132134
}
133135
// do we need this in rocksdb?
134136
trx->pinData(cid);
135137

136138
FulltextQuery parsedQuery;
137139
Result res = fulltextIndex->parseQueryString(queryString, parsedQuery);
138140
if (res.fail()) {
139-
RegisterWarning(query, "FULLTEXT", res.errorNumber());
140-
return AqlValue(AqlValueHintNull());
141+
THROW_ARANGO_EXCEPTION(res);
141142
}
142143
std::set<LocalDocumentId> results;
143144
res = fulltextIndex->executeQuery(trx, parsedQuery, results);
144145
if (res.fail()) {
145-
RegisterWarning(query, "FULLTEXT", res.errorNumber());
146-
return AqlValue(AqlValueHintNull());
146+
THROW_ARANGO_EXCEPTION(res);
147147
}
148148

149149
PhysicalCollection* physical = collection->getPhysical();
@@ -414,7 +414,7 @@ void RocksDBAqlFunctions::registerResources() {
414414
TRI_ASSERT(functions != nullptr);
415415

416416
// fulltext functions
417-
functions->add({"FULLTEXT", ".h,.,.|.", true, false,
417+
functions->add({"FULLTEXT", ".h,.,.|.", false, true,
418418
false, true, &RocksDBAqlFunctions::Fulltext,
419419
NotInCoordinator});
420420
functions->add({"NEAR", ".h,.,.|.,.", false, true, false,

‎arangod/RocksDBEngine/RocksDBCollection.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,7 @@ Result RocksDBCollection::read(transaction::Methods* trx,
788788
// read using a token!
789789
bool RocksDBCollection::readDocument(transaction::Methods* trx,
790790
LocalDocumentId const& documentId,
791-
ManagedDocumentResult& result) {
791+
ManagedDocumentResult& result) const {
792792
if (documentId.isSet()) {
793793
auto res = lookupDocumentVPack(documentId, trx, result, true);
794794
return res.ok();
@@ -799,7 +799,7 @@ bool RocksDBCollection::readDocument(transaction::Methods* trx,
799799
// read using a token!
800800
bool RocksDBCollection::readDocumentWithCallback(
801801
transaction::Methods* trx, LocalDocumentId const& documentId,
802-
IndexIterator::DocumentCallback const& cb) {
802+
IndexIterator::DocumentCallback const& cb) const {
803803
if (documentId.isSet()) {
804804
auto res = lookupDocumentVPack(documentId, trx, cb, true);
805805
return res.ok();

‎arangod/RocksDBEngine/RocksDBCollection.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,11 @@ class RocksDBCollection final : public PhysicalCollection {
138138

139139
bool readDocument(transaction::Methods* trx,
140140
LocalDocumentId const& token,
141-
ManagedDocumentResult& result) override;
141+
ManagedDocumentResult& result) const override;
142142

143143
bool readDocumentWithCallback(
144144
transaction::Methods* trx, LocalDocumentId const& token,
145-
IndexIterator::DocumentCallback const& cb) override;
145+
IndexIterator::DocumentCallback const& cb) const override;
146146

147147
Result insert(arangodb::transaction::Methods* trx,
148148
arangodb::velocypack::Slice const newSlice,

0 commit comments

Comments
 (0)
0