tursom/memgraph
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Map indexing added

Browse Source 
Reviewers: mislav.bradac, buda

Reviewed By: mislav.bradac

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D739
This commit is contained in:
florijan
2017-09-02 14:30:05 +02:00
parent 9eac85c9fb
commit f68bac922f
10 changed files with 98 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -8,6 +8,7 @@
* `rand` function added.
* Maps can now be stored as vertex/edge properties.
* `collect` aggregation now supports Map collection.
* Map indexing supported.
### Bug Fixes and Other Changes

2
src/query/frontend/ast/ast.hpp
View File

@@ -417,7 +417,7 @@ class InListOperator : public BinaryOperator {
using BinaryOperator::BinaryOperator;
};
class ListIndexingOperator : public BinaryOperator {
class ListMapIndexingOperator : public BinaryOperator {
friend class AstTreeStorage;
public:

6
src/query/frontend/ast/ast_visitor.hpp
View File

@@ -45,7 +45,7 @@ class GreaterOperator;
class LessEqualOperator;
class GreaterEqualOperator;
class InListOperator;
class ListIndexingOperator;
class ListMapIndexingOperator;
class ListSlicingOperator;
class IfOperator;
class Delete;
@@ -64,7 +64,7 @@ using TreeCompositeVisitor = ::utils::CompositeVisitor<
FilterAndOperator, NotOperator, AdditionOperator, SubtractionOperator,
MultiplicationOperator, DivisionOperator, ModOperator, NotEqualOperator,
EqualOperator, LessOperator, GreaterOperator, LessEqualOperator,
GreaterEqualOperator, InListOperator, ListIndexingOperator,
GreaterEqualOperator, InListOperator, ListMapIndexingOperator,
ListSlicingOperator, IfOperator, UnaryPlusOperator, UnaryMinusOperator,
IsNullOperator, ListLiteral, MapLiteral, PropertyLookup, LabelsTest,
EdgeTypeTest, Aggregation, Function, All, Create, Match, Return, With,
@@ -89,7 +89,7 @@ using TreeVisitor = ::utils::Visitor<
FilterAndOperator, NotOperator, AdditionOperator, SubtractionOperator,
MultiplicationOperator, DivisionOperator, ModOperator, NotEqualOperator,
EqualOperator, LessOperator, GreaterOperator, LessEqualOperator,
GreaterEqualOperator, InListOperator, ListIndexingOperator,
GreaterEqualOperator, InListOperator, ListMapIndexingOperator,
ListSlicingOperator, IfOperator, UnaryPlusOperator, UnaryMinusOperator,
IsNullOperator, ListLiteral, MapLiteral, PropertyLookup, LabelsTest,
EdgeTypeTest, Aggregation, Function, All, Create, Match, Return, With,

2
src/query/frontend/ast/cypher_main_visitor.cpp
View File

@@ -714,7 +714,7 @@ antlrcpp::Any CypherMainVisitor::visitExpression3b(
for (auto *list_op : ctx->listIndexingOrSlicing()) {
if (list_op->getTokens(kDotsTokenId).size() == 0U) {
// If there is no '..' then we need to create list indexing operator.
expression = storage_.Create<ListIndexingOperator>(
expression = storage_.Create<ListMapIndexingOperator>(
expression, list_op->expression()[0]->accept(this));
} else if (!list_op->lower_bound && !list_op->upper_bound) {
throw SemanticException(

55
src/query/interpret/eval.hpp
View File

@@ -170,33 +170,40 @@ class ExpressionEvaluator : public TreeVisitor<TypedValue> {
return false;
}
TypedValue Visit(ListIndexingOperator &list_indexing) override {
// TODO: implement this for maps
auto _list = list_indexing.expression1_->Accept(*this);
if (_list.type() != TypedValue::Type::List &&
_list.type() != TypedValue::Type::Null) {
TypedValue Visit(ListMapIndexingOperator &list_indexing) override {
auto lhs = list_indexing.expression1_->Accept(*this);
auto index = list_indexing.expression2_->Accept(*this);
if (!lhs.IsList() && !lhs.IsMap() && !lhs.IsNull())
throw QueryRuntimeException(
"Expected a list to index with '[]', but got {}", _list.type());
"Expected a list or map to index with '[]', but got {}", lhs.type());
if (lhs.IsNull() || index.IsNull()) return TypedValue::Null;
if (lhs.IsList()) {
if (!index.IsInt())
throw QueryRuntimeException(
"Expected an int as a list index, but got {}", index.type());
auto index_int = index.Value<int64_t>();
const auto &list = lhs.Value<std::vector<TypedValue>>();
if (index_int < 0) {
index_int += static_cast<int64_t>(list.size());
}
if (index_int >= static_cast<int64_t>(list.size()) || index_int < 0)
return TypedValue::Null;
return list[index_int];
}
auto _index = list_indexing.expression2_->Accept(*this);
if (_index.type() != TypedValue::Type::Int &&
_index.type() != TypedValue::Type::Null) {
throw QueryRuntimeException("Expected an int as a list index, but got {}",
_index.type());
if (lhs.IsMap()) {
if (!index.IsString())
throw QueryRuntimeException(
"Expected a string as a map index, but got {}", index.type());
const auto &map = lhs.Value<std::map<std::string, TypedValue>>();
auto found = map.find(index.Value<std::string>());
if (found == map.end()) return TypedValue::Null;
return found->second;
}
if (_index.type() == TypedValue::Type::Null ||
_list.type() == TypedValue::Type::Null) {
return TypedValue::Null;
}
auto index = _index.Value<int64_t>();
const auto &list = _list.Value<std::vector<TypedValue>>();
if (index < 0) {
index = static_cast<int64_t>(list.size()) + index;
}
if (index >= static_cast<int64_t>(list.size()) || index < 0) {
return TypedValue::Null;
}
return list[index];
// lhs is Null
return TypedValue::Null;
}
TypedValue Visit(ListSlicingOperator &op) override {

2
src/query/plan/rule_based_planner.cpp
View File

@@ -344,7 +344,7 @@ class ReturnBodyContext : public HierarchicalTreeVisitor {
VISIT_BINARY_OPERATOR(LessEqualOperator)
VISIT_BINARY_OPERATOR(GreaterEqualOperator)
VISIT_BINARY_OPERATOR(InListOperator)
VISIT_BINARY_OPERATOR(ListIndexingOperator)
VISIT_BINARY_OPERATOR(ListMapIndexingOperator)
#undef VISIT_BINARY_OPERATOR

10
tests/qa/tck_engine/tests/memgraph_V1/features/map_operations.feature
View File

@@ -33,3 +33,13 @@ Feature: Map operators
Then the result should be:
| x.a | x.c.d |
| 1 | 42 |
Scenario: Map indexing
When executing query:
"""
WITH {a: 1, b: 'bla', c: {d: 42}} AS x RETURN x["a"] as xa, x["c"]["d"] as xcd, x["z"] as xz
"""
Then the result should be:
| xa | xcd | xz |
| 1 | 42 | null |

6
tests/unit/cypher_main_visitor.cpp
View File

@@ -473,11 +473,11 @@ TYPED_TEST(CypherMainVisitorTest, ComparisonOperators) {
#undef CHECK_COMPARISON
TYPED_TEST(CypherMainVisitorTest, ListIndexingOperator) {
TYPED_TEST(CypherMainVisitorTest, ListIndexing) {
TypeParam ast_generator("RETURN [1,2,3] [ 2 ]");
auto *query = ast_generator.query_;
auto *return_clause = dynamic_cast<Return *>(query->clauses_[0]);
auto *list_index_op = dynamic_cast<ListIndexingOperator *>(
auto *list_index_op = dynamic_cast<ListMapIndexingOperator *>(
return_clause->body_.named_expressions[0]->expression_);
ASSERT_TRUE(list_index_op);
auto *list = dynamic_cast<ListLiteral *>(list_index_op->expression1_);
@@ -532,7 +532,7 @@ TYPED_TEST(CypherMainVisitorTest, InWithListIndexing) {
ASSERT_TRUE(literal);
EXPECT_EQ(literal->value_.Value<int64_t>(), 1);
auto *list_indexing =
dynamic_cast<ListIndexingOperator *>(in_list_operator->expression2_);
dynamic_cast<ListMapIndexingOperator *>(in_list_operator->expression2_);
ASSERT_TRUE(list_indexing);
auto *list = dynamic_cast<ListLiteral *>(list_indexing->expression1_);
EXPECT_TRUE(list);

53
tests/unit/query_expression_evaluator.cpp
View File

@@ -313,7 +313,7 @@ TEST(ExpressionEvaluator, InListOperator) {
}
}
TEST(ExpressionEvaluator, ListIndexingOperator) {
TEST(ExpressionEvaluator, ListMapIndexingOperator) {
AstTreeStorage storage;
NoContextExpressionEvaluator eval;
auto *list_literal = storage.Create<ListLiteral>(std::vector<Expression *>{
@@ -322,35 +322,35 @@ TEST(ExpressionEvaluator, ListIndexingOperator) {
storage.Create<PrimitiveLiteral>(4)});
{
// Legal indexing.
auto *op = storage.Create<ListIndexingOperator>(
auto *op = storage.Create<ListMapIndexingOperator>(
list_literal, storage.Create<PrimitiveLiteral>(2));
auto value = op->Accept(eval.eval);
EXPECT_EQ(value.Value<int64_t>(), 3);
}
{
// Out of bounds indexing.
auto *op = storage.Create<ListIndexingOperator>(
auto *op = storage.Create<ListMapIndexingOperator>(
list_literal, storage.Create<PrimitiveLiteral>(4));
auto value = op->Accept(eval.eval);
EXPECT_EQ(value.type(), TypedValue::Type::Null);
}
{
// Out of bounds indexing with negative bound.
auto *op = storage.Create<ListIndexingOperator>(
auto *op = storage.Create<ListMapIndexingOperator>(
list_literal, storage.Create<PrimitiveLiteral>(-100));
auto value = op->Accept(eval.eval);
EXPECT_EQ(value.type(), TypedValue::Type::Null);
}
{
// Legal indexing with negative index.
auto *op = storage.Create<ListIndexingOperator>(
auto *op = storage.Create<ListMapIndexingOperator>(
list_literal, storage.Create<PrimitiveLiteral>(-2));
auto value = op->Accept(eval.eval);
EXPECT_EQ(value.Value<int64_t>(), 3);
}
{
// Indexing with one operator being null.
auto *op = storage.Create<ListIndexingOperator>(
auto *op = storage.Create<ListMapIndexingOperator>(
storage.Create<PrimitiveLiteral>(TypedValue::Null),
storage.Create<PrimitiveLiteral>(-2));
auto value = op->Accept(eval.eval);
@@ -358,13 +358,52 @@ TEST(ExpressionEvaluator, ListIndexingOperator) {
}
{
// Indexing with incompatible type.
auto *op = storage.Create<ListIndexingOperator>(
auto *op = storage.Create<ListMapIndexingOperator>(
storage.Create<PrimitiveLiteral>(2),
storage.Create<PrimitiveLiteral>(TypedValue::Null));
EXPECT_THROW(op->Accept(eval.eval), QueryRuntimeException);
}
}
TEST(ExpressionEvaluator, MapIndexing) {
AstTreeStorage storage;
NoContextExpressionEvaluator eval;
Dbms dbms;
auto dba = dbms.active();
auto *map_literal = storage.Create<MapLiteral>(
std::map<std::pair<std::string, GraphDbTypes::Property>, Expression *>{
{PROPERTY_PAIR("a"), storage.Create<PrimitiveLiteral>(1)},
{PROPERTY_PAIR("b"), storage.Create<PrimitiveLiteral>(2)},
{PROPERTY_PAIR("c"), storage.Create<PrimitiveLiteral>(3)}});
{
// Legal indexing.
auto *op = storage.Create<ListMapIndexingOperator>(
map_literal, storage.Create<PrimitiveLiteral>("b"));
auto value = op->Accept(eval.eval);
EXPECT_EQ(value.Value<int64_t>(), 2);
}
{
// Legal indexing, non-existing key.
auto *op = storage.Create<ListMapIndexingOperator>(
map_literal, storage.Create<PrimitiveLiteral>("z"));
auto value = op->Accept(eval.eval);
EXPECT_TRUE(value.IsNull());
}
{
// Wrong key type.
auto *op = storage.Create<ListMapIndexingOperator>(
map_literal, storage.Create<PrimitiveLiteral>(42));
EXPECT_THROW(op->Accept(eval.eval), QueryRuntimeException);
}
{
// Indexing with Null.
auto *op = storage.Create<ListMapIndexingOperator>(
map_literal, storage.Create<PrimitiveLiteral>(TypedValue::Null));
auto value = op->Accept(eval.eval);
EXPECT_TRUE(value.IsNull());
}
}
TEST(ExpressionEvaluator, ListSlicingOperator) {
AstTreeStorage storage;
NoContextExpressionEvaluator eval;

2
tests/unit/query_planner.cpp
View File

@@ -985,7 +985,7 @@ TEST(TestLogicalPlanner, EmptyListIndexAggregation) {
auto sum = SUM(LITERAL(2));
auto empty_list = LIST();
auto group_by_literal = LITERAL(42);
QUERY(RETURN(storage.Create<query::ListIndexingOperator>(empty_list, sum),
QUERY(RETURN(storage.Create<query::ListMapIndexingOperator>(empty_list, sum),
AS("result"), group_by_literal, AS("group_by")));
// We expect to group by '42' and the empty list, because it is a
// sub-expression of a binary operator which contains an aggregation. This is