Refine RewriteInputPropVisitor implementation (#347)

* Refine fixture of conftest.py in query folder

* Cleanup

* Restore capture of pytest

src/context/Symbols.h

@@ -9,6 +9,8 @@
 
 #include "util/ObjectPool.h"
 
+#include "common/datatypes/Value.h"
+
 namespace nebula {
 namespace graph {
 
@@ -118,6 +120,7 @@ private:
     // var name -> variable
     std::unordered_map<std::string, Variable*>                              vars_;
 };
+
 }  // namespace graph
 }  // namespace nebula
 #endif

src/validator/GoValidator.cpp

@@ -749,10 +749,7 @@ void GoValidator::extractPropExprs(const Expression* expr) {
 std::unique_ptr<Expression> GoValidator::rewriteToInputProp(Expression* expr) {
     RewriteInputPropVisitor visitor(propExprColMap_);
     const_cast<Expression*>(expr)->accept(&visitor);
-    if (visitor.ok()) {
-        return std::move(visitor).result();
-    }
-    return nullptr;
+    return std::move(visitor).result();
 }
 
 Status GoValidator::buildColumns() {

src/visitor/RewriteInputPropVisitor.cpp

@@ -121,36 +121,34 @@ void RewriteInputPropVisitor::visit(VariablePropertyExpression* expr) {
 }
 
 void RewriteInputPropVisitor::visit(ListExpression* expr) {
-    auto items = std::move(*expr).get();
-    for (auto iter = items.begin(); iter < items.end(); ++iter) {
-        iter->get()->accept(this);
+    const auto& items = expr->items();
+    for (size_t i = 0; i < items.size(); ++i) {
+        items[i]->accept(this);
         if (ok()) {
-            *iter = std::move(result_);
+            expr->setItem(i, std::move(result_));
         }
     }
-    expr->setItems(std::move(items));
 }
 
 void RewriteInputPropVisitor::visit(SetExpression* expr) {
-    auto items = std::move(*expr).get();
-    for (auto iter = items.begin(); iter < items.end(); ++iter) {
-        iter->get()->accept(this);
+    const auto& items = expr->items();
+    for (size_t i = 0; i < items.size(); ++i) {
+        items[i]->accept(this);
         if (ok()) {
-            *iter = std::move(result_);
+            expr->setItem(i, std::move(result_));
         }
     }
-    expr->setItems(std::move(items));
 }
 
 void RewriteInputPropVisitor::visit(MapExpression* expr) {
-    auto items = std::move(*expr).get();
-    for (auto iter = items.begin(); iter < items.end(); ++iter) {
-        iter->second.get()->accept(this);
+    const auto& items = expr->items();
+    for (size_t i = 0; i < items.size(); ++i) {
+        items[i].second->accept(this);
         if (ok()) {
-            *iter = std::make_pair(std::move(iter->first), std::move(result_));
+            auto key = std::make_unique<std::string>(*items[i].first);
+            expr->setItem(i, {std::move(key), std::move(result_)});
         }
     }
-    expr->setItems(std::move(items));
 }
 
 void RewriteInputPropVisitor::visit(FunctionCallExpression* expr) {
@@ -189,49 +187,7 @@ void RewriteInputPropVisitor::visitUnaryExpr(UnaryExpression* expr) {
 }
 
 void RewriteInputPropVisitor::visitVertexEdgePropExpr(PropertyExpression* expr) {
-    PropertyExpression* propExpr = nullptr;
-    switch (expr->kind()) {
-        case Expression::Kind::kTagProperty: {
-            propExpr = static_cast<TagPropertyExpression*>(expr);
-            break;
-        }
-        case Expression::Kind::kSrcProperty: {
-            propExpr = static_cast<SourcePropertyExpression*>(expr);
-            break;
-        }
-        case Expression::Kind::kDstProperty: {
-            propExpr = static_cast<DestPropertyExpression*>(expr);
-            break;
-        }
-        case Expression::Kind::kEdgeProperty: {
-            propExpr = static_cast<EdgePropertyExpression*>(expr);
-            break;
-        }
-        case Expression::Kind::kEdgeSrc: {
-            propExpr = static_cast<EdgeSrcIdExpression*>(expr);
-            break;
-        }
-        case Expression::Kind::kEdgeType: {
-            propExpr = static_cast<EdgeTypeExpression*>(expr);
-            break;
-        }
-        case Expression::Kind::kEdgeRank: {
-            propExpr = static_cast<EdgeRankExpression*>(expr);
-            break;
-        }
-        case Expression::Kind::kEdgeDst: {
-            propExpr = static_cast<EdgeDstIdExpression*>(expr);
-            break;
-        }
-        case Expression::Kind::kVarProperty: {
-            propExpr = static_cast<VariablePropertyExpression*>(expr);
-            break;
-        }
-        default: {
-            LOG(FATAL) << "Invalid Kind " << expr->kind();
-        }
-    }
-    auto found = propExprColMap_.find(propExpr->toString());
+    auto found = propExprColMap_.find(expr->toString());
     DCHECK(found != propExprColMap_.end());
     auto alias = new std::string(*(found->second->alias()));
     result_ = std::make_unique<InputPropertyExpression>(alias);

src/visitor/RewriteInputPropVisitor.h

@@ -15,7 +15,8 @@ namespace graph {
 
 class RewriteInputPropVisitor final : public ExprVisitor {
 public:
-    explicit RewriteInputPropVisitor(std::unordered_map<std::string, YieldColumn *> &propExprColMap)
+    explicit RewriteInputPropVisitor(
+        const std::unordered_map<std::string, YieldColumn *> &propExprColMap)
         : propExprColMap_(propExprColMap) {}
     ~RewriteInputPropVisitor() = default;
 
@@ -75,7 +76,7 @@ private:
     void reportError(const Expression *);
 
 private:
-    std::unordered_map<std::string, YieldColumn *>& propExprColMap_;
+    const std::unordered_map<std::string, YieldColumn *> &propExprColMap_;
 
     std::unique_ptr<Expression> result_;
     Status status_;

tests/pytest.ini

 [pytest]
-addopts = -r xfE -v --tb=short --showlocals -s
+addopts = -r xfE -v --tb=short --showlocals
 junit_family=legacy

tests/query/v1/test_find_path.py

@@ -5,12 +5,9 @@
 # This source code is licensed under Apache 2.0 License,
 # attached with Common Clause Condition 1.0, found in the LICENSES directory.
 
-import pytest
-
 from tests.common.nebula_test_suite import NebulaTestSuite
 
 
-@pytest.mark.usefixtures('set_vertices_and_edges')
 class TestFindPath(NebulaTestSuite):
     @classmethod
     def prepare(self):

tests/query/v2/test_match.py

@@ -5,19 +5,17 @@
 # This source code is licensed under Apache 2.0 License,
 # attached with Common Clause Condition 1.0, found in the LICENSES directory.
 
-from tests.common.nebula_test_suite import NebulaTestSuite
-from tests.common.nebula_test_suite import T_NULL, T_EMPTY
 import pytest
 
+from tests.common.nebula_test_suite import NebulaTestSuite
+
+
 @pytest.mark.usefixtures('set_vertices_and_edges')
 class TestMatch(NebulaTestSuite):
     @classmethod
     def prepare(self):
         self.use_nba()
 
-    def cleanup():
-        pass
-
     def test_single_node(self):
         VERTICES = self.VERTEXS
         stmt = 'MATCH (v:player {name: "Yao Ming"}) RETURN v'
@@ -93,10 +91,7 @@ class TestMatch(NebulaTestSuite):
         self.check_column_names(resp, expected['column_names'])
         self.check_out_of_order_result(resp, expected['rows'])
 
-
     def test_one_step(self):
-        VERTICES = self.VERTEXS
-
         stmt = 'MATCH (v1:player{name: "LeBron James"}) -[r]-> (v2) RETURN type(r) AS Type, v2.name AS Name'
         resp = self.execute_query(stmt)
         self.check_resp_succeeded(resp)
@@ -164,8 +159,6 @@ class TestMatch(NebulaTestSuite):
         self.check_out_of_order_result(resp, expected['rows'])
 
     def test_two_steps(self):
-        VERTICES = self.VERTEXS
-
         stmt = '''
                   MATCH (v1:player{age: 28}) -[:like]-> (v2) -[:like]-> (v3)
                   RETURN v1.name AS Player, v2.name AS Friend, v3.name AS FoF