kuzudb · andyfengHKU · Jul 15, 2023 · Jul 14, 2023
diff --git a/src/binder/bind/bind_graph_pattern.cpp b/src/binder/bind/bind_graph_pattern.cpp
@@ -49,7 +49,7 @@ std::unique_ptr<QueryGraph> Binder::bindPatternElement(
     if (patternElement.hasPathName()) {
         auto pathName = patternElement.getPathName();
         auto pathExpression = createPathExpression(pathName, nodeAndRels);
-        variableScope->addExpression(pathName, pathExpression);
+        scope->addExpression(pathName, pathExpression);
     }
     return queryGraph;
 }
@@ -180,9 +180,12 @@ std::shared_ptr<RelExpression> Binder::bindQueryRel(const RelPattern& relPattern
     const std::shared_ptr<NodeExpression>& rightNode, QueryGraph& queryGraph,
     PropertyKeyValCollection& collection) {
     auto parsedName = relPattern.getVariableName();
-    if (variableScope->contains(parsedName)) {
-        auto prevVariable = variableScope->getExpression(parsedName);
-        ExpressionBinder::validateExpectedDataType(*prevVariable, LogicalTypeID::REL);
+    if (scope->contains(parsedName)) {
+        auto prevVariable = scope->getExpression(parsedName);
+        auto expectedDataType = QueryRelTypeUtils::isRecursive(relPattern.getRelType()) ?
+                                    LogicalTypeID::RECURSIVE_REL :
+                                    LogicalTypeID::REL;
+        ExpressionBinder::validateExpectedDataType(*prevVariable, expectedDataType);
         throw BinderException("Bind relationship " + parsedName +
                               " to relationship with same name is not supported.");
     }
@@ -237,7 +240,7 @@ std::shared_ptr<RelExpression> Binder::bindQueryRel(const RelPattern& relPattern
     }
     queryRel->setAlias(parsedName);
     if (!parsedName.empty()) {
-        variableScope->addExpression(parsedName, queryRel);
+        scope->addExpression(parsedName, queryRel);
     }
     queryGraph.addQueryRel(queryRel);
     return queryRel;
@@ -286,10 +289,10 @@ std::shared_ptr<RelExpression> Binder::createRecursiveQueryRel(const parser::Rel
         InternalKeyword::ANONYMOUS, std::vector<common::table_id_t>{recursiveNodeTableIDs.begin(),
                                         recursiveNodeTableIDs.end()});
     auto prevScope = saveScope();
-    variableScope->clear();
+    scope->clear();
     auto tmpRel = createNonRecursiveQueryRel(
         recursiveRelPatternInfo->relName, tableIDs, nullptr, nullptr, directionType);
-    variableScope->addExpression(tmpRel->toString(), tmpRel);
+    scope->addExpression(tmpRel->toString(), tmpRel);
     expression_vector predicates;
     for (auto& [propertyName, rhs] : relPattern.getPropertyKeyVals()) {
         auto boundLhs = expressionBinder.bindRelPropertyExpression(*tmpRel, propertyName);
@@ -356,8 +359,8 @@ std::shared_ptr<NodeExpression> Binder::bindQueryNode(
     const NodePattern& nodePattern, QueryGraph& queryGraph, PropertyKeyValCollection& collection) {
     auto parsedName = nodePattern.getVariableName();
     std::shared_ptr<NodeExpression> queryNode;
-    if (variableScope->contains(parsedName)) { // bind to node in scope
-        auto prevVariable = variableScope->getExpression(parsedName);
+    if (scope->contains(parsedName)) { // bind to node in scope
+        auto prevVariable = scope->getExpression(parsedName);
         ExpressionBinder::validateExpectedDataType(*prevVariable, LogicalTypeID::NODE);
         queryNode = static_pointer_cast<NodeExpression>(prevVariable);
         // E.g. MATCH (a:person) MATCH (a:organisation)
@@ -369,7 +372,7 @@ std::shared_ptr<NodeExpression> Binder::bindQueryNode(
     } else {
         queryNode = createQueryNode(nodePattern);
         if (!parsedName.empty()) {
-            variableScope->addExpression(parsedName, queryNode);
+            scope->addExpression(parsedName, queryNode);
         }
     }
     for (auto& [propertyName, rhs] : nodePattern.getPropertyKeyVals()) {

diff --git a/src/binder/bind/bind_projection_clause.cpp b/src/binder/bind/bind_projection_clause.cpp
@@ -37,7 +37,7 @@ std::unique_ptr<BoundWithClause> Binder::bindWithClause(const WithClause& withCl
     }
     auto boundProjectionBody = bindProjectionBody(*projectionBody, newProjectionExpressions);
     validateOrderByFollowedBySkipOrLimitInWithClause(*boundProjectionBody);
-    variableScope->clear();
+    scope->clear();
     addExpressionsToScope(projectionExpressions);
     auto boundWithClause = std::make_unique<BoundWithClause>(std::move(boundProjectionBody));
     if (withClause.hasWhereExpression()) {
@@ -60,7 +60,7 @@ std::unique_ptr<BoundReturnClause> Binder::bindReturnClause(const ReturnClause&
 }
 
 static bool isAggregateExpression(
-    const std::shared_ptr<Expression>& expression, VariableScope* scope) {
+    const std::shared_ptr<Expression>& expression, BinderScope* scope) {
     if (expression->hasAlias() && scope->contains(expression->getAlias())) {
         return false;
     }
@@ -76,7 +76,7 @@ static bool isAggregateExpression(
 }
 
 static expression_vector getAggregateExpressions(
-    const std::shared_ptr<Expression>& expression, VariableScope* scope) {
+    const std::shared_ptr<Expression>& expression, BinderScope* scope) {
     expression_vector result;
     if (expression->hasAlias() && scope->contains(expression->getAlias())) {
         return result;
@@ -101,8 +101,8 @@ std::unique_ptr<BoundProjectionBody> Binder::bindProjectionBody(
     expression_vector groupByExpressions;
     expression_vector aggregateExpressions;
     for (auto& expression : projectionExpressions) {
-        if (isAggregateExpression(expression, variableScope.get())) {
-            for (auto& agg : getAggregateExpressions(expression, variableScope.get())) {
+        if (isAggregateExpression(expression, scope.get())) {
+            for (auto& agg : getAggregateExpressions(expression, scope.get())) {
                 aggregateExpressions.push_back(agg);
             }
         } else {
@@ -167,11 +167,11 @@ expression_vector Binder::bindProjectionExpressions(
     for (auto& expression : projectionExpressions) {
         if (expression->getExpressionType() == common::ExpressionType::STAR) {
             // Rewrite star expression as all expression in scope.
-            if (variableScope->empty()) {
+            if (scope->empty()) {
                 throw BinderException(
                     "RETURN or WITH * is not allowed when there are no variables in scope.");
             }
-            for (auto& expr : variableScope->getExpressions()) {
+            for (auto& expr : scope->getExpressions()) {
                 result.push_back(expr);
             }
         } else if (expression->getExpressionType() == common::ExpressionType::PROPERTY) {
@@ -225,7 +225,7 @@ void Binder::addExpressionsToScope(const expression_vector& projectionExpression
     for (auto& expression : projectionExpressions) {
         // In RETURN clause, if expression is not aliased, its input name will serve its alias.
         auto alias = expression->hasAlias() ? expression->getAlias() : expression->toString();
-        variableScope->addExpression(alias, expression);
+        scope->addExpression(alias, expression);
     }
 }
 

diff --git a/src/binder/bind/bind_query.cpp b/src/binder/bind/bind_query.cpp
@@ -10,7 +10,7 @@ std::unique_ptr<BoundRegularQuery> Binder::bindQuery(const RegularQuery& regular
     for (auto i = 0u; i < regularQuery.getNumSingleQueries(); i++) {
         // Don't clear scope within bindSingleQuery() yet because it is also used for subquery
         // binding.
-        variableScope->clear();
+        scope->clear();
         boundSingleQueries.push_back(bindSingleQuery(*regularQuery.getSingleQuery(i)));
     }
     validateUnionColumnsOfTheSameType(boundSingleQueries);

diff --git a/src/binder/bind/bind_updating_clause.cpp b/src/binder/bind/bind_updating_clause.cpp
@@ -32,23 +32,37 @@ std::unique_ptr<BoundUpdatingClause> Binder::bindUpdatingClause(
 std::unique_ptr<BoundUpdatingClause> Binder::bindCreateClause(
     const UpdatingClause& updatingClause) {
     auto& createClause = (CreateClause&)updatingClause;
-    auto prevVariableScope = variableScope->copy();
+    auto prevScope = scope->copy();
+    expression_set nodesScope;
+    expression_set relsScope;
+    for (auto& expression : scope->getExpressions()) {
+        if (ExpressionUtil::isNodeVariable(*expression)) {
+            nodesScope.insert(expression);
+        } else if (ExpressionUtil::isRelVariable(*expression)) {
+            relsScope.insert(expression);
+        }
+    }
+    // bindGraphPattern will update scope.
     auto [queryGraphCollection, propertyCollection] =
         bindGraphPattern(createClause.getPatternElements());
     auto boundCreateClause = std::make_unique<BoundCreateClause>();
     for (auto i = 0u; i < queryGraphCollection->getNumQueryGraphs(); ++i) {
         auto queryGraph = queryGraphCollection->getQueryGraph(i);
         for (auto j = 0u; j < queryGraph->getNumQueryNodes(); ++j) {
             auto node = queryGraph->getQueryNode(j);
-            if (!prevVariableScope->contains(node->toString())) {
-                boundCreateClause->addCreateNode(bindCreateNode(node, *propertyCollection));
+            if (nodesScope.contains(node)) {
+                continue;
             }
+            nodesScope.insert(node);
+            boundCreateClause->addCreateNode(bindCreateNode(node, *propertyCollection));
         }
         for (auto j = 0u; j < queryGraph->getNumQueryRels(); ++j) {
             auto rel = queryGraph->getQueryRel(j);
-            if (!prevVariableScope->contains(rel->toString())) {
-                boundCreateClause->addCreateRel(bindCreateRel(rel, *propertyCollection));
+            if (relsScope.contains(rel)) {
+                continue;
             }
+            relsScope.insert(rel);
+            boundCreateClause->addCreateRel(bindCreateRel(rel, *propertyCollection));
         }
     }
     return boundCreateClause;

diff --git a/src/binder/bind_expression/bind_subquery_expression.cpp b/src/binder/bind_expression/bind_subquery_expression.cpp
@@ -11,7 +11,7 @@ namespace binder {
 std::shared_ptr<Expression> ExpressionBinder::bindExistentialSubqueryExpression(
     const ParsedExpression& parsedExpression) {
     auto& subqueryExpression = (ParsedSubqueryExpression&)parsedExpression;
-    auto prevVariableScope = binder->saveScope();
+    auto prevScope = binder->saveScope();
     auto [queryGraph, _] = binder->bindGraphPattern(subqueryExpression.getPatternElements());
     auto rawName = parsedExpression.getRawName();
     auto uniqueName = binder->getUniqueExpressionName(rawName);
@@ -21,7 +21,7 @@ std::shared_ptr<Expression> ExpressionBinder::bindExistentialSubqueryExpression(
         boundSubqueryExpression->setWhereExpression(
             binder->bindWhereExpression(*subqueryExpression.getWhereClause()));
     }
-    binder->restoreScope(std::move(prevVariableScope));
+    binder->restoreScope(std::move(prevScope));
     return boundSubqueryExpression;
 }
 

diff --git a/src/binder/bind_expression/bind_variable_expression.cpp b/src/binder/bind_expression/bind_variable_expression.cpp
@@ -12,8 +12,8 @@ std::shared_ptr<Expression> ExpressionBinder::bindVariableExpression(
     const ParsedExpression& parsedExpression) {
     auto& variableExpression = (ParsedVariableExpression&)parsedExpression;
     auto variableName = variableExpression.getVariableName();
-    if (binder->variableScope->contains(variableName)) {
-        return binder->variableScope->getExpression(variableName);
+    if (binder->scope->contains(variableName)) {
+        return binder->scope->getExpression(variableName);
     }
     throw common::BinderException(
         "Variable " + parsedExpression.getRawName() + " is not in scope.");

diff --git a/src/binder/binder.cpp b/src/binder/binder.cpp
@@ -72,13 +72,13 @@ table_id_t Binder::bindNodeTableID(const std::string& tableName) const {
 
 std::shared_ptr<Expression> Binder::createVariable(
     const std::string& name, const LogicalType& dataType) {
-    if (variableScope->contains(name)) {
+    if (scope->contains(name)) {
         throw BinderException("Variable " + name + " already exists.");
     }
     auto uniqueName = getUniqueExpressionName(name);
     auto expression = expressionBinder.createVariableExpression(dataType, uniqueName, name);
     expression->setAlias(name);
-    variableScope->addExpression(name, expression);
+    scope->addExpression(name, expression);
     return expression;
 }
 
@@ -197,12 +197,12 @@ std::string Binder::getUniqueExpressionName(const std::string& name) {
     return "_" + std::to_string(lastExpressionId++) + "_" + name;
 }
 
-std::unique_ptr<VariableScope> Binder::saveScope() {
-    return variableScope->copy();
+std::unique_ptr<BinderScope> Binder::saveScope() {
+    return scope->copy();
 }
 
-void Binder::restoreScope(std::unique_ptr<VariableScope> prevVariableScope) {
-    variableScope = std::move(prevVariableScope);
+void Binder::restoreScope(std::unique_ptr<BinderScope> prevVariableScope) {
+    scope = std::move(prevVariableScope);
 }
 
 } // namespace binder

diff --git a/src/include/binder/binder.h b/src/include/binder/binder.h
@@ -19,12 +19,12 @@ class BoundSetNodeProperty;
 class BoundSetRelProperty;
 class BoundDeleteNode;
 
-// VariableScope keeps track of expressions in scope and their aliases. We maintain the order of
+// BinderScope keeps track of expressions in scope and their aliases. We maintain the order of
 // expressions in
-class VariableScope {
+class BinderScope {
 public:
-    VariableScope() = default;
-    VariableScope(expression_vector expressions,
+    BinderScope() = default;
+    BinderScope(expression_vector expressions,
         std::unordered_map<std::string, common::vector_idx_t> varNameToIdx)
         : expressions{std::move(expressions)}, varNameToIdx{std::move(varNameToIdx)} {}
 
@@ -44,8 +44,8 @@ class VariableScope {
         expressions.clear();
         varNameToIdx.clear();
     }
-    inline std::unique_ptr<VariableScope> copy() {
-        return std::make_unique<VariableScope>(expressions, varNameToIdx);
+    inline std::unique_ptr<BinderScope> copy() {
+        return std::make_unique<BinderScope>(expressions, varNameToIdx);
     }
 
 private:
@@ -58,7 +58,7 @@ class Binder {
 
 public:
     explicit Binder(const catalog::Catalog& catalog, main::ClientContext* clientContext)
-        : catalog{catalog}, lastExpressionId{0}, variableScope{std::make_unique<VariableScope>()},
+        : catalog{catalog}, lastExpressionId{0}, scope{std::make_unique<BinderScope>()},
           expressionBinder{this}, clientContext{clientContext} {}
 
     std::unique_ptr<BoundStatement> bind(const parser::Statement& statement);
@@ -236,13 +236,13 @@ class Binder {
     /*** helpers ***/
     std::string getUniqueExpressionName(const std::string& name);
 
-    std::unique_ptr<VariableScope> saveScope();
-    void restoreScope(std::unique_ptr<VariableScope> prevVariableScope);
+    std::unique_ptr<BinderScope> saveScope();
+    void restoreScope(std::unique_ptr<BinderScope> prevVariableScope);
 
 private:
     const catalog::Catalog& catalog;
     uint32_t lastExpressionId;
-    std::unique_ptr<VariableScope> variableScope;
+    std::unique_ptr<BinderScope> scope;
     ExpressionBinder expressionBinder;
     main::ClientContext* clientContext;
 };