Fix the issue that "orderby" variable is not in "return" clause (#585)

* fix orderby problem * fix orderby problem * fix orderby bug, change throwcode in arthmetic_expression.cpp * fix orderby bug, change throwcode in arthmetic_expression.cpp * add orderby testcase * add test case MATCH (p:Person) RETURN p ORDER BY p.birthyear DESC * add base.result * add orderby testcase add test case MATCH (p:Person) RETURN p ORDER BY p.birthyear DESC add base.result dealing with the last line problem in base.result * add last line in base.result
TuGraph-family · Jul 7, 2024 · d587f2e · d587f2e
1 parent 3a0600c
commit d587f2e
Show file tree

Hide file tree

Showing 8 changed files with 29 additions and 8 deletions.
diff --git a/ci/start_dev.sh b/ci/start_dev.sh
@@ -77,6 +77,7 @@ if [ -z "$CONTAINER_ID" ]; then
         ${DOCKER_IMAGE} \
         bash -c "chown ${USER_NAME}:${GROUP_NAME} /home/${USER_NAME} &&
           echo 'export JAVA_HOME=/usr/lib/jvm/java-11-openjdk' > /home/${USER_NAME}/.bashrc &&
+          echo 'export LD_LIBRARY_PATH=/usr/lib64:/usr/local/lib64:/usr/local/lib:/usr/lib/jvm/java-11-openjdk/lib/server' > /home/${USER_NAME}/.bashrc &&
           echo 'export JAVA_HOME=/usr/lib/jvm/java-11-openjdk' > /home/${USER_NAME}/.bash_profile &&
           /usr/sbin/sshd -D"
 fi
diff --git a/src/cypher/arithmetic/arithmetic_expression.cpp b/src/cypher/arithmetic/arithmetic_expression.cpp
@@ -1396,7 +1396,11 @@ void ArithOperandNode::SetEntity(const std::string &alias, const SymbolTable &sy
     type = AR_OPERAND_VARIADIC;
     variadic.alias = alias;
     auto it = sym_tab.symbols.find(alias);
-    CYPHER_THROW_ASSERT(it != sym_tab.symbols.end());
+    if (it == sym_tab.symbols.end()) {
+        THROW_CODE(InputError,
+            "Variable `{}` not defined", alias);
+    }
+    // CYPHER_THROW_ASSERT(it != sym_tab.symbols.end());
     variadic.alias_idx = it->second.id;
 }
 

diff --git a/src/cypher/execution_plan/execution_plan.cpp b/src/cypher/execution_plan/execution_plan.cpp
@@ -152,6 +152,10 @@ static void BuildResultSetInfo(const QueryPart &stmt, ResultInfo &result_info) {
         for (auto &item : ret_items) {
             auto &e = std::get<0>(item);
             auto &alias = std::get<1>(item);
+            bool isHidden = std::get<2>(item);
+            if (isHidden) {
+                continue;
+            }
             ArithExprNode ae(e, stmt.symbol_table);
             bool aggregate = ae.ContainsAggregation();
             if (aggregate) result_info.aggregated = true;

diff --git a/src/cypher/parser/cypher_base_visitor.h b/src/cypher/parser/cypher_base_visitor.h
@@ -269,7 +269,7 @@ class CypherBaseVisitor : public LcypherVisitor {
         AddSymbol(variable, cypher::SymbolNode::CONSTANT, var_scope);
         Clause clause;
         clause.type = Clause::UNWIND;
-        clause.data = std::make_shared<Clause::TYPE_UNWIND>(e, variable);
+        clause.data = std::make_shared<Clause::TYPE_UNWIND>(e, variable, false);
         _query[_curr_query].parts[_curr_part].AddClause(clause);
         _LeaveClauseUNWIND();
         return 0;
@@ -654,8 +654,14 @@ class CypherBaseVisitor : public LcypherVisitor {
                 }
                 // sort_item alias is not in return_item
                 if (!sort_idx_found) {
-                    THROW_CODE(InputError,
-                        "Variable `{}` not defined", sort_item.first.ToString());
+                    // THROW_CODE(InputError,
+                    //     "Variable `{}` not defined", sort_item.first.ToString());
+                    auto expr = sort_item.first;
+                    auto alias = sort_item.first.ToString();
+                    bool isHidden = true;
+                    return_items.emplace_back(std::make_tuple(expr, alias, isHidden));
+                    int cur_sort_item_idx = return_items.size() - 1;
+                    sort_items_idx.emplace_back(cur_sort_item_idx, sort_item.second);
                 }
             }
             CYPHER_THROW_ASSERT(sort_items_idx.size() == sort_items.size());
@@ -712,7 +718,7 @@ class CypherBaseVisitor : public LcypherVisitor {
             AddSymbol(as_variable.empty() ? variable : as_variable, type,
                       cypher::SymbolNode::LOCAL);
         }
-        return std::make_tuple(expr, as_variable);
+        return std::make_tuple(expr, as_variable, false);
     }
 
     std::any visitOC_Order(LcypherParser::OC_OrderContext *ctx) override {

diff --git a/src/cypher/parser/data_typedef.h b/src/cypher/parser/data_typedef.h
@@ -71,7 +71,7 @@ typedef std::tuple<std::string, TUP_PATTERN_ELEMENT> TUP_PATTERN_PART;
 // pattern_parts
 typedef std::vector<TUP_PATTERN_PART> VEC_PATTERN;
 // expression, as_variable
-typedef std::tuple<Expression, std::string> TUP_RETURN_ITEM;
+typedef std::tuple<Expression, std::string, bool> TUP_RETURN_ITEM;
 // return_items, sort_items(return_item_idx, asc), skip, limit
 typedef std::tuple<std::vector<TUP_RETURN_ITEM>, std::vector<std::pair<int, bool>>, Expression,
                    Expression>

diff --git a/test/resource/cases/suite/cypher/base.result b/test/resource/cases/suite/cypher/base.result
@@ -21,3 +21,7 @@ MATCH (n:Person)-[r:ACTED_IN {charactername:'Henri Ducard'}]->(m) RETURN r;
 [{"r":{"dst":17,"forward":false,"identity":0,"label":"ACTED_IN","label_id":5,"properties":{"charactername":"Henri Ducard"},"src":4,"temporal_id":0}}]
 MATCH (n:Person)<-[r:ACTED_IN {charactername:'Henri Ducard'}]-(m) RETURN r;
 []
+MATCH (p:Person) RETURN p.name ORDER BY p.birthyear DESC LIMIT 5;
+[{"p.name":"Lindsay Lohan"},{"p.name":"Christopher Nolan"},{"p.name":"Jemma Redgrave"},{"p.name":"Natasha Richardson"},{"p.name":"Dennis Quaid"}]
+MATCH (p:Person) RETURN p ORDER BY p.birthyear DESC LIMIT 5;
+[{"p":{"identity":8,"label":"Person","properties":{"birthyear":1986,"name":"Lindsay Lohan"}}},{"p":{"identity":12,"label":"Person","properties":{"birthyear":1970,"name":"Christopher Nolan"}}},{"p":{"identity":9,"label":"Person","properties":{"birthyear":1965,"name":"Jemma Redgrave"}}},{"p":{"identity":5,"label":"Person","properties":{"birthyear":1963,"name":"Natasha Richardson"}}},{"p":{"identity":7,"label":"Person","properties":{"birthyear":1954,"name":"Dennis Quaid"}}}]
diff --git a/test/resource/cases/suite/cypher/base.test b/test/resource/cases/suite/cypher/base.test
@@ -10,4 +10,6 @@ MATCH (n:Person)<-[r:BORN_IN {reg_time:datetime('2023-05-01 14:00:00')}]-(m) RET
 MATCH (n:Person)-[r:BORN_IN {reg_time:datetime('2023-05-01 14:00:00')}]->(m) where r.weight > 20 RETURN r;
 MATCH (n:Person)-[r:BORN_IN {reg_time:datetime('2023-05-01 14:00:00')}]->(m) where r.weight < 20 RETURN r;
 MATCH (n:Person)-[r:ACTED_IN {charactername:'Henri Ducard'}]->(m) RETURN r;
-MATCH (n:Person)<-[r:ACTED_IN {charactername:'Henri Ducard'}]-(m) RETURN r;
+MATCH (n:Person)<-[r:ACTED_IN {charactername:'Henri Ducard'}]-(m) RETURN r;
+MATCH (p:Person) RETURN p.name ORDER BY p.birthyear DESC LIMIT 5;
+MATCH (p:Person) RETURN p ORDER BY p.birthyear DESC LIMIT 5;
diff --git a/test/test_query.cpp b/test/test_query.cpp
@@ -178,7 +178,7 @@ class TestQuery : public TuGraphTest {
             execution_plan.Build(visitor.GetQuery(), visitor.CommandType(), ctx_.get());
             execution_plan.Validate(ctx_.get());
             LOG_INFO() << execution_plan.DumpGraph();
-            execution_plan.DumpPlan(0, false);
+            LOG_INFO() << execution_plan.DumpPlan(0, false);
             execution_plan.Execute(ctx_.get());
             result = ctx_->result_->Dump(false);
             UT_LOG() << "-----result-----";