From a6dc673b2ccb9155410f564571d4552fe18d7a10 Mon Sep 17 00:00:00 2001 From: Sophie <84560950+Sophie-Xie@users.noreply.github.com> Date: Mon, 6 Mar 2023 18:33:05 +0800 Subject: [PATCH] =?UTF-8?q?Cherry=20pick=20v3.4.1=20=EF=BC=88from=20releas?= =?UTF-8?q?e=203.4.0=20to=2003.05)=20(#5387)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix the crash when lookup parameter expression eval in storage (#5336) * Small fix for parameter tck (#5371) * Fix crash of list related functions (#5383) * fix coflicts * fix coflicts * fix tck --------- Co-authored-by: kyle.cao --- src/common/function/FunctionManager.cpp | 13 +++++++++---- src/graph/validator/LookupValidator.cpp | 3 +++ src/storage/ExprVisitorBase.cpp | 4 +--- tests/tck/features/yield/parameter.feature | 20 ++++++++++++++++++-- tests/tck/features/yield/return.feature | 12 ++++++++++++ 5 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/common/function/FunctionManager.cpp b/src/common/function/FunctionManager.cpp index 11d41741cc8..0d9d4874375 100644 --- a/src/common/function/FunctionManager.cpp +++ b/src/common/function/FunctionManager.cpp @@ -2040,8 +2040,11 @@ FunctionManager::FunctionManager() { return v.vid; } case Value::Type::LIST: { - const auto &listVal = args[0].get().getList(); - auto &lastVal = listVal.values.back(); + const auto &listVal = args[0].get().getList().values; + if (listVal.empty()) { + return Value::kNullBadType; + } + auto &lastVal = listVal.back(); if (lastVal.isEdge()) { return lastVal.getEdge().dst; } else if (lastVal.isVertex()) { @@ -2134,7 +2137,8 @@ FunctionManager::FunctionManager() { return Value::kNullValue; } case Value::Type::LIST: { - return args[0].get().getList().values.front(); + const auto &items = args[0].get().getList().values; + return items.empty() ? Value::kNullValue : items.front(); } default: { return Value::kNullBadType; @@ -2153,7 +2157,8 @@ FunctionManager::FunctionManager() { return Value::kNullValue; } case Value::Type::LIST: { - return args[0].get().getList().values.back(); + const auto &items = args[0].get().getList().values; + return items.empty() ? Value::kNullValue : items.back(); } default: { return Value::kNullBadType; diff --git a/src/graph/validator/LookupValidator.cpp b/src/graph/validator/LookupValidator.cpp index 4f185327280..31bbe16f4cc 100644 --- a/src/graph/validator/LookupValidator.cpp +++ b/src/graph/validator/LookupValidator.cpp @@ -191,6 +191,9 @@ Status LookupValidator::validateWhere() { } auto* filter = whereClause->filter(); + if (filter != nullptr) { + filter = graph::ExpressionUtils::rewriteParameter(filter, qctx_); + } if (FTIndexUtils::needTextSearch(filter)) { lookupCtx_->isFulltextIndex = true; lookupCtx_->fulltextExpr = filter; diff --git a/src/storage/ExprVisitorBase.cpp b/src/storage/ExprVisitorBase.cpp index 8d8b21ec603..d2f286b1fa2 100644 --- a/src/storage/ExprVisitorBase.cpp +++ b/src/storage/ExprVisitorBase.cpp @@ -33,9 +33,7 @@ void ExprVisitorBase::visit(SubscriptExpression *expr) { expr->left()->accept(this); expr->right()->accept(this); } -void ExprVisitorBase::visit(AttributeExpression *expr) { - fatal(expr); -} +void ExprVisitorBase::visit(AttributeExpression *) {} void ExprVisitorBase::visit(LogicalExpression *expr) { for (auto operand : expr->operands()) { operand->accept(this); diff --git a/tests/tck/features/yield/parameter.feature b/tests/tck/features/yield/parameter.feature index 83f44a85fa8..d4a77ae2494 100644 --- a/tests/tck/features/yield/parameter.feature +++ b/tests/tck/features/yield/parameter.feature @@ -4,7 +4,8 @@ Feature: Parameter Background: - Given a graph with space named "nba" + Given an empty graph + And load "nba" csv data to a new space Given parameters: {"p1":1,"p2":true,"p3":"Tim Duncan","p4":3.3,"p5":[1,true,3],"p6":{"a":3,"b":false,"c":"Tim Duncan"},"p7":{"a":{"b":{"c":"Tim Duncan","d":[1,2,3,true,"Tim Duncan"]}}},"p8":"Manu Ginobili", "p9":["Tim Duncan","Tony Parker"]} Scenario: [param-test-001] without define param @@ -253,7 +254,7 @@ Feature: Parameter """ LOOKUP ON player WHERE player.age>$p2+43 """ - Then a SemanticError should be raised at runtime: Column type error : age + Then a SemanticError should be raised at runtime: Type error `(true+43)' When executing query: """ MATCH (v:player) RETURN v LIMIT $p6 @@ -330,3 +331,18 @@ Feature: Parameter | v | | BAD_TYPE | | BAD_TYPE | + When executing query: + """ + $var=lookup on player where player.name==$p6.c and player.age in [43,35,42,45] yield id(vertex) AS VertexID;RETURN count($var.VertexID) AS record + """ + Then the execution should be successful + When executing query: + """ + $var=lookup on player where player.name==$p3 and player.age in [43,35,42,45] yield id(vertex) AS VertexID;RETURN count($var.VertexID) AS record + """ + Then the execution should be successful + When executing query: + """ + $var=lookup on player where player.name==$p7.a.b.d[4] and player.age in [43,35,42,45] yield id(vertex) AS VertexID;RETURN count($var.VertexID) AS record + """ + Then the execution should be successful diff --git a/tests/tck/features/yield/return.feature b/tests/tck/features/yield/return.feature index 468cbe93000..542eb175480 100644 --- a/tests/tck/features/yield/return.feature +++ b/tests/tck/features/yield/return.feature @@ -14,6 +14,18 @@ Feature: Return. A standalone return sentence is actually a yield sentence Then the result should be, in any order: | sum | | 2 | + When executing query: + """ + RETURN last(LIST[]) AS a, head(LIST[]) AS b + """ + Then the result should be, in any order: + | a | b | + | NULL | NULL | + When executing query: + """ + MATCH (v:player) RETURN none_direct_dst(LIST[]) AS a + """ + Then a SemanticError should be raised at runtime:`none_direct_dst([])' is not a valid expression : Function `none_direct_dst' not defined When executing query: """ RETURN DISTINCT 1+1, '1+1', (int)3.14, (string)(1+1), (string)true