1
0
Fork 0

fixes for non-array IN lookups, added tests

This commit is contained in:
jsteemann 2017-04-25 01:47:07 +02:00
parent ca5d54aa58
commit 4300c77d3e
12 changed files with 107 additions and 46 deletions

View File

@ -764,22 +764,30 @@ void Index::expandInSearchValues(VPackSlice const base,
VPackSlice current = oneLookup.at(i);
if (current.hasKey(StaticStrings::IndexIn)) {
VPackSlice inList = current.get(StaticStrings::IndexIn);
if (!inList.isArray()) {
// IN value is a non-array
result.clear();
result.openArray();
return;
}
TRI_ASSERT(inList.isArray());
VPackValueLength nList = inList.length();
if (nList == 0) {
// Empty Array. short circuit, no matches possible
result.clear();
result.openArray();
return;
}
std::unordered_set<VPackSlice,
arangodb::basics::VelocyPackHelper::VPackHash,
arangodb::basics::VelocyPackHelper::VPackEqual>
tmp(static_cast<size_t>(inList.length()),
tmp(static_cast<size_t>(nList),
arangodb::basics::VelocyPackHelper::VPackHash(),
arangodb::basics::VelocyPackHelper::VPackEqual());
TRI_ASSERT(inList.isArray());
if (inList.length() == 0) {
// Empty Array. short circuit, no matches possible
result.clear();
result.openArray();
result.close();
return;
}
for (auto const& el : VPackArrayIterator(inList)) {
tmp.emplace(el);
}

View File

@ -58,7 +58,6 @@ class LogicalCollection;
namespace transaction {
class Methods;
}
;
/// @brief a base class to iterate over the index. An iterator is requested
/// at the index itself
@ -102,23 +101,23 @@ class IndexIterator {
/// @brief Special iterator if the condition cannot have any result
class EmptyIndexIterator final : public IndexIterator {
public:
EmptyIndexIterator(LogicalCollection* collection, transaction::Methods* trx, ManagedDocumentResult* mmdr, arangodb::Index const* index)
: IndexIterator(collection, trx, mmdr, index) {}
public:
EmptyIndexIterator(LogicalCollection* collection, transaction::Methods* trx, ManagedDocumentResult* mmdr, arangodb::Index const* index)
: IndexIterator(collection, trx, mmdr, index) {}
~EmptyIndexIterator() {}
~EmptyIndexIterator() {}
char const* typeName() const override { return "empty-index-iterator"; }
char const* typeName() const override { return "empty-index-iterator"; }
bool next(TokenCallback const&, size_t) override {
return false;
}
bool next(TokenCallback const&, size_t) override {
return false;
}
void reset() override {}
void reset() override {}
void skip(uint64_t, uint64_t& skipped) override {
skipped = 0;
}
void skip(uint64_t, uint64_t& skipped) override {
skipped = 0;
}
};
/// @brief a wrapper class to iterate over several IndexIterators.

View File

@ -448,14 +448,15 @@ IndexIterator* MMFilesEdgeIndex::iteratorForCondition(
if (comp->type == aql::NODE_TYPE_OPERATOR_BINARY_IN) {
// a.b IN values
if (!valNode->isArray()) {
return nullptr;
// a.b IN non-array
return new EmptyIndexIterator(_collection, trx, mmdr, this);
}
return createInIterator(trx, mmdr, attrNode, valNode);
}
// operator type unsupported
return nullptr;
return new EmptyIndexIterator(_collection, trx, mmdr, this);
}
/// @brief specializes the condition for use with the index

View File

@ -120,8 +120,10 @@ MMFilesHashIndexLookupBuilder::MMFilesHashIndexLookupBuilder(
TRI_IF_FAILURE("Index::permutationIN") {
THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
}
for (auto const& value : VPackArrayIterator(values)) {
tmp.emplace(value);
if (values.isArray()) {
for (auto const& value : VPackArrayIterator(values)) {
tmp.emplace(value);
}
}
if (tmp.empty()) {
// IN [] short-circuit, cannot be fullfilled;

View File

@ -471,14 +471,15 @@ IndexIterator* MMFilesPrimaryIndex::iteratorForCondition(
} else if (comp->type == aql::NODE_TYPE_OPERATOR_BINARY_IN) {
// a.b IN values
if (!valNode->isArray()) {
return nullptr;
// a.b IN non-array
return new EmptyIndexIterator(_collection, trx, mmdr, this);
}
return createInIterator(trx, mmdr, attrNode, valNode);
}
// operator type unsupported
return nullptr;
return new EmptyIndexIterator(_collection, trx, mmdr, this);
}
/// @brief specializes the condition for use with the index

View File

@ -464,10 +464,11 @@ void RocksDBCollection::truncate(transaction::Methods* trx,
// TODO maybe we could also reuse Index::drop, if we ensure the
// implementations
// don't do anything beyond deleting their contents
RocksDBKeyBounds indexBounds =
RocksDBKeyBounds::PrimaryIndex(42); // default constructor?
for (std::shared_ptr<Index> const& index : _indexes) {
RocksDBIndex* rindex = static_cast<RocksDBIndex*>(index.get());
RocksDBKeyBounds indexBounds =
RocksDBKeyBounds::Empty();
switch (rindex->type()) {
case RocksDBIndex::TRI_IDX_TYPE_PRIMARY_INDEX:
indexBounds = RocksDBKeyBounds::PrimaryIndex(rindex->objectId());

View File

@ -104,7 +104,10 @@ bool RocksDBEdgeIndexIterator::next(TokenCallback const& cb, size_t limit) {
// acquire rocksdb collection
auto rocksColl = RocksDBCollection::toRocksDBCollection(_collection);
while (limit > 0) {
while (true) {
TRI_ASSERT(limit > 0);
while (_iterator->Valid() &&
(_index->_cmp->Compare(_iterator->key(), _bounds.end()) < 0)) {
StringRef edgeKey = RocksDBKey::primaryKey(_iterator->key());
@ -121,14 +124,12 @@ bool RocksDBEdgeIndexIterator::next(TokenCallback const& cb, size_t limit) {
}
} // TODO do we need to handle failed lookups here?
}
if (limit > 0) {
_keysIterator.next();
if (!updateBounds()) {
return false;
}
_keysIterator.next();
if (!updateBounds()) {
return false;
}
}
return true;
}
void RocksDBEdgeIndexIterator::reset() {
@ -323,14 +324,15 @@ IndexIterator* RocksDBEdgeIndex::iteratorForCondition(
if (comp->type == aql::NODE_TYPE_OPERATOR_BINARY_IN) {
// a.b IN values
if (!valNode->isArray()) {
return nullptr;
// a.b IN non-array
return new EmptyIndexIterator(_collection, trx, mmdr, this);
}
return createInIterator(trx, mmdr, attrNode, valNode);
}
// operator type unsupported
return nullptr;
return new EmptyIndexIterator(_collection, trx, mmdr, this);
}
/// @brief specializes the condition for use with the index

View File

@ -35,6 +35,10 @@ using namespace arangodb::velocypack;
const char RocksDBKeyBounds::_stringSeparator = '\0';
RocksDBKeyBounds RocksDBKeyBounds::Empty() {
return RocksDBKeyBounds();
}
RocksDBKeyBounds RocksDBKeyBounds::Databases() {
return RocksDBKeyBounds(RocksDBEntryType::Database);
}
@ -98,6 +102,11 @@ rocksdb::Slice const RocksDBKeyBounds::end() const {
return rocksdb::Slice(_endBuffer);
}
// constructor for an empty bound. do not use for anything but to
// default-construct a key bound!
RocksDBKeyBounds::RocksDBKeyBounds()
: _type(RocksDBEntryType::Database), _startBuffer(), _endBuffer() {}
RocksDBKeyBounds::RocksDBKeyBounds(RocksDBEntryType type)
: _type(type), _startBuffer(), _endBuffer() {
switch (_type) {

View File

@ -38,8 +38,11 @@ namespace arangodb {
class RocksDBKeyBounds {
public:
RocksDBKeyBounds() = delete;
//////////////////////////////////////////////////////////////////////////////
/// @brief empty bounds
//////////////////////////////////////////////////////////////////////////////
static RocksDBKeyBounds Empty();
//////////////////////////////////////////////////////////////////////////////
/// @brief Bounds for list of all databases
//////////////////////////////////////////////////////////////////////////////
@ -125,7 +128,8 @@ class RocksDBKeyBounds {
rocksdb::Slice const end() const;
private:
RocksDBKeyBounds(RocksDBEntryType type);
RocksDBKeyBounds();
explicit RocksDBKeyBounds(RocksDBEntryType type);
RocksDBKeyBounds(RocksDBEntryType type, uint64_t first);
RocksDBKeyBounds(RocksDBEntryType type, uint64_t first,
std::string const& second);

View File

@ -502,14 +502,15 @@ IndexIterator* RocksDBPrimaryIndex::iteratorForCondition(
} else if (comp->type == aql::NODE_TYPE_OPERATOR_BINARY_IN) {
// a.b IN values
if (!valNode->isArray()) {
return nullptr;
// a.b IN non-array
return new EmptyIndexIterator(_collection, trx, mmdr, this);
}
return createInIterator(trx, mmdr, attrNode, valNode);
}
// operator type unsupported
return nullptr;
return new EmptyIndexIterator(_collection, trx, mmdr, this);
}
/// @brief specializes the condition for use with the index

View File

@ -1147,7 +1147,7 @@ IndexIterator* RocksDBVPackIndex::iteratorForCondition(
// unsupported right now. Should have been rejected by
// supportsFilterCondition
TRI_ASSERT(false);
return nullptr;
return new EmptyIndexIterator(_collection, trx, mmdr, this);
}
value->toVelocyPackValue(searchValues);
}

View File

@ -227,6 +227,28 @@ function optimizerIndexesTestSuite () {
assertEqual(0, results.stats.scannedIndex);
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test _id
////////////////////////////////////////////////////////////////////////////////
testUsePrimaryKeyInAttributeAccess : function () {
var values = [ "test1", "test2", "test21", "test30" ];
var query = "LET data = NOOPT({ ids : " + JSON.stringify(values) + " }) FOR i IN " + c.name() + " FILTER i._key IN data.ids RETURN i.value";
var plan = AQL_EXPLAIN(query).plan;
var nodeTypes = plan.nodes.map(function(node) {
return node.type;
});
assertEqual("SingletonNode", nodeTypes[0], query);
assertNotEqual(-1, nodeTypes.indexOf("IndexNode"), query);
var results = AQL_EXECUTE(query);
assertEqual([ 1, 2, 21, 30 ], results.json.sort(), query);
assertEqual(0, results.stats.scannedFull);
assertEqual(4, results.stats.scannedIndex);
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test _key
////////////////////////////////////////////////////////////////////////////////
@ -2560,7 +2582,18 @@ function optimizerIndexesTestSuite () {
[ "FOR i IN " + c.name() + " FILTER i._key IN ['test23', 'test42'] RETURN i._key", [ 'test23', 'test42' ] ],
[ "LET a = PASSTHRU([]) FOR i IN " + c.name() + " FILTER i._key IN a RETURN i._key", [ ] ],
[ "LET a = PASSTHRU(['test23']) FOR i IN " + c.name() + " FILTER i._key IN a RETURN i._key", [ 'test23' ] ],
[ "LET a = PASSTHRU(['test23', 'test42']) FOR i IN " + c.name() + " FILTER i._key IN a RETURN i._key", [ 'test23', 'test42' ] ]
[ "LET a = PASSTHRU(['test23', 'test42']) FOR i IN " + c.name() + " FILTER i._key IN a RETURN i._key", [ 'test23', 'test42' ] ],
[ "LET a = PASSTHRU({ ids: ['test23', 'test42'] }) FOR i IN " + c.name() + " FILTER i._key IN a.ids RETURN i._key", [ 'test23', 'test42' ] ],
[ "LET a = PASSTHRU({ ids: [23, 42] }) FOR i IN " + c.name() + " FILTER i.value2 IN a.ids RETURN i.value2", [ 23, 42 ] ],
[ "LET a = PASSTHRU({ ids: [23, 42] }) FOR i IN " + c.name() + " FILTER i.value3 IN a.ids RETURN i.value2", [ 23, 42 ] ],
// non-arrays. should not fail but return no results
[ "LET a = PASSTHRU({}) FOR i IN " + c.name() + " FILTER i._key IN a RETURN i._key", [ ] ],
[ "LET a = PASSTHRU({}) FOR i IN " + c.name() + " FILTER i.value2 IN a RETURN i.value2", [ ] ],
[ "LET a = PASSTHRU({}) FOR i IN " + c.name() + " FILTER i.value3 IN a RETURN i.value2", [ ] ]
];
queries.forEach(function(query) {