Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation feature list: InQueryCall, Unwind, Merge, YieldField, Multi-Match, With, Aggregate, Expression, Exists, cypher::FieldData of type map and List of run-through tests includes TestMultiMatch,Testparameter, TestWith, TestUnwind, TestAdd, TestDelete, TestMerge, TestCreatYago, TestAggregate, TestTopn, TestLdbcSnb, TestOpt, TestCreateLabel, TestEdgeIdQuery #561

Merged
merged 52 commits into from
Jun 27, 2024

Conversation

jiazhenjiang
Copy link
Contributor

No description provided.

@lipanpan03 lipanpan03 enabled auto-merge (squash) June 24, 2024 07:31
@lipanpan03 lipanpan03 disabled auto-merge June 24, 2024 07:32
@@ -188,6 +188,8 @@ using StrArray = std::array<const char* const, N>;
TYPE(MkRecord, kMkRecord, "MkRecord") \
TYPE(MkSet, kMkSet, "MkSet") \
TYPE(MkTuple, kMkTuple, "MkTuple") \
TYPE(UnwindStatement, kUnwindStatement, "UnwindStatement") \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unwind statement不能复用for statement吗

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gql for包含 cypher unwind,可以复用

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这块的计划是先把其他的都做完,然后再把unwind和for统一起来

@@ -188,6 +188,8 @@ using StrArray = std::array<const char* const, N>;
TYPE(MkRecord, kMkRecord, "MkRecord") \
TYPE(MkSet, kMkSet, "MkSet") \
TYPE(MkTuple, kMkTuple, "MkTuple") \
TYPE(UnwindStatement, kUnwindStatement, "UnwindStatement") \
TYPE(InQueryProcedureCall, kInQueryProcedureCall, "InQueryProcedureCall") \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in query call在gql ast中没有吗?在gql语法中呢

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gql语法中只有带参数的函数调用和不带参数的函数调用。cypher中in query call本质和procedure call在ast层面是同一个东西,但是在执行计划里因为区分了两种算子,所以需要区分是procedure还是inquery call,暂时通过新增一个ast结构做为区分,后续计划是从算子层面统一。

@@ -18,6 +18,8 @@
#ifndef GEAXFRONTEND_AST_EXPR_MKLIST_H_
#define GEAXFRONTEND_AST_EXPR_MKLIST_H_

#include <vector>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些新增的头文件符合cpplint吗

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里用到了std::vector,不添加头文件,在后续的编译中会出问题。

@@ -44,7 +44,8 @@ class AstAggExprDetector : public cypher::AstNodeVisitorImpl {

private:
std::any visit(geax::frontend::AggFunc* node) override;
std::any visit(geax::frontend::Ref* node) override;
std::any visit(geax::frontend::BAggFunc* node) override;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里删掉了visit Ref,举个例子在处理什么情况

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里是删除了Ref新增了GetField表达式,为了区分 MATCH (n:City) RETURN collect(n.name) + n.name;
MATCH (n:Person) RETURN avg(n.age) + 100;

@@ -44,7 +44,8 @@ class AstAggExprDetector : public cypher::AstNodeVisitorImpl {

private:
std::any visit(geax::frontend::AggFunc* node) override;
std::any visit(geax::frontend::Ref* node) override;
std::any visit(geax::frontend::BAggFunc* node) override;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里删掉了visit Ref,举个例子在处理什么情况

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里是删除了Ref新增了GetField表达式,为了区分 MATCH (n:City) RETURN collect(n.name) + n.name;
MATCH (n:Person) RETURN avg(n.age) + 100;

delete array;
break;
case MAP:
delete array;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

错误

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已处理

type = rhs.type;
if (rhs.type == ARRAY) {

switch (type) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

两个switch合并一起做

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里要先根据this.type回收资源,然后再根据rhs.type分配资源,合并起来代码可读性很差,不如这样清晰

scalar = std::move(rhs.scalar);
array = rhs.array;
// rhs.scalar becomes FieldType::NUL after move
switch (type) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

合并

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里要先根据this.type回收资源,然后再根据rhs.type分配资源,合并起来代码可读性很差,不如这样清晰

scalar = std::move(rhs.scalar);
break;
}
// TODO(lingsu) : rhs should return to the default state
rhs.type = SCALAR;
return *this;
}

bool operator==(const FieldData& rhs) const {
if (type != rhs.type)
throw std::runtime_error("Unable to compare between SCALAR and ARRAY.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

msg要改,比如 between different field data types

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已处理

@@ -118,16 +212,30 @@ struct FieldData {
bool operator>(const FieldData& rhs) const {
if (type != rhs.type)
throw std::runtime_error("Unable to compare between SCALAR and ARRAY.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同上

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已处理

@jiazhenjiang jiazhenjiang changed the title Implement cypher query statements in addition to profile, union, remove and list_comprehension Implement Jun 24, 2024
@jiazhenjiang jiazhenjiang changed the title Implement Implementation feature list: InQueryCall, Unwind, Merge, YieldField, Multi-Match, With, Aggregate, Expression, Exists and cypher::FieldData of type map Jun 24, 2024
@jiazhenjiang jiazhenjiang changed the title Implementation feature list: InQueryCall, Unwind, Merge, YieldField, Multi-Match, With, Aggregate, Expression, Exists and cypher::FieldData of type map Implementation feature list: InQueryCall, Unwind, Merge, YieldField, Multi-Match, With, Aggregate, Expression, Exists cypher::FieldData of type map and List of run-through tests includes TestMultiMatch,Testparameter, TestWith, TestUnwind, TestAdd, TestDelete, TestMerge, TestCreatYago, TestAggregate, TestTopn, TestLdbcSnb, TestOpt, TestCreateLabel, TestEdgeIdQuery Jun 24, 2024
@jiazhenjiang jiazhenjiang changed the title Implementation feature list: InQueryCall, Unwind, Merge, YieldField, Multi-Match, With, Aggregate, Expression, Exists cypher::FieldData of type map and List of run-through tests includes TestMultiMatch,Testparameter, TestWith, TestUnwind, TestAdd, TestDelete, TestMerge, TestCreatYago, TestAggregate, TestTopn, TestLdbcSnb, TestOpt, TestCreateLabel, TestEdgeIdQuery Implementation feature list: InQueryCall, Unwind, Merge, YieldField, Multi-Match, With, Aggregate, Expression, Exists, cypher::FieldData of type map and List of run-through tests includes TestMultiMatch,Testparameter, TestWith, TestUnwind, TestAdd, TestDelete, TestMerge, TestCreatYago, TestAggregate, TestTopn, TestLdbcSnb, TestOpt, TestCreateLabel, TestEdgeIdQuery Jun 24, 2024
if (agg_it != registered_agg_funcs.end()) {
// Evalute Mode
if (visit_mode_ == VisitMode::EVALUATE) {
if (agg_pos_ >= agg_ctxs_.size()) {
NOT_SUPPORT_AND_THROW();
return Entry(cypher::FieldData(lgraph_api::FieldData(0)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

举个例子,什么情况会走到这个分支

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我理解是某个aggfunction在统计时,结果为空,代码会执行到这个分支,之前的case里是返回了0,因此这里也返回0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

讨论

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

之前的test case里返回的是0,这里是兼容之前的case

geax::frontend::GEAXErrorCode ret = geax::frontend::GEAXErrorCode::GEAX_SUCCEED;
// build pattern graph
PatternGraphMaker pattern_graph_maker(pattern_graphs_);
ret = pattern_graph_maker.Build(astNode);
ret = pattern_graph_maker.Build(astNode, ctx);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

传入ctx什么作用

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

为了在PatternGraphMaker中处理YieldField时判断是否是正确的返回值

public:
explicit MultiPathPatternRewriter(geax::common::ObjectArenaAllocator& allocator)
: allocator_(allocator) {}
std::any visit(geax::frontend::GraphPattern* node) override {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个是把什么重写成什么了?举个例子

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MATCH (x)<-[:ACTED_IN]-(p)-[:MARRIED]->(y), (p)-[:HAS_CHILD]->(z) RETURN p,x,y,z; 把多段pattern中相同的点挑出来,通过filter确定相同的点,执行的时候对同一个点的查找只执行一次。例子中就是将p挑出来了

@@ -202,7 +205,7 @@ class TestCypherV2 : public TuGraphTest {
} catch (std::exception& e) {
UT_LOG() << e.what();
result = e.what();
return false;
return true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

为什么改动

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

之前的test case里有一些就是要抛异常,这里如果不改,抛异常就被视为失败了

if (agg_it != registered_agg_funcs.end()) {
// Evalute Mode
if (visit_mode_ == VisitMode::EVALUATE) {
if (agg_pos_ >= agg_ctxs_.size()) {
NOT_SUPPORT_AND_THROW();
return Entry(cypher::FieldData(lgraph_api::FieldData(0)));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

讨论

@@ -318,6 +332,36 @@ std::any cypher::AstExprEvaluator::visit(geax::frontend::AggFunc* node) {
}

std::any cypher::AstExprEvaluator::visit(geax::frontend::BAggFunc* node) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这部分的作用是?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gql中aggfunction分为单参数和双参数的,cypher没区分这些,因此只用agg不完整,所以加了这个

@@ -5,7 +5,7 @@ match (v1:Film) return v1.title order by v1.title limit 3;
match (v1:Film)<-[:ACTED_IN|DIRECTED]-(v2:Person) return v1.title,v2.name as cnt order by cnt desc limit 3;
[{"cnt":"Vanessa Redgrave","v1.title":"Camelot"},{"cnt":"Richard Harris","v1.title":"Camelot"},{"cnt":"Richard Harris","v1.title":"Harry Potter and the Sorcerer's Stone"}]
match (:Person {name:'Vanessa Redgrave'})<-[:HAS_CHILD]-(p)-[:ACTED_IN*0..]->(m) return p.name,m order by p.name limit 3;
[{"m":{"identity":16,"label":"Film","properties":{"title":"Goodbye, Mr. Chips"}},"p.name":"Michael Redgrave"},{"m":{"identity":1,"label":"Person","properties":{"birthyear":1908,"name":"Michael Redgrave"}},"p.name":"Michael Redgrave"},{"m":{"identity":0,"label":"Person","properties":{"birthyear":1910,"name":"Rachel Kempson"}},"p.name":"Rachel Kempson"}]
[{"m":{"identity":1,"label":"Person","properties":{"birthyear":1908,"name":"Michael Redgrave"}},"p.name":"Michael Redgrave"},{"m":{"identity":16,"label":"Film","properties":{"title":"Goodbye, Mr. Chips"}},"p.name":"Michael Redgrave"},{"m":{"identity":0,"label":"Person","properties":{"birthyear":1910,"name":"Rachel Kempson"}},"p.name":"Rachel Kempson"}]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

结果变化?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

理论上查询中是order by p.name,因此结果中p.name相同的其他部分,因为未指明排序规则,按照neo4j语法,是无序的

scalar = rhs.scalar;
break;
}
return *this;
}

FieldData& operator=(::cypher::FieldData&& rhs) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cypher_types多补一些单测,这里有内存泄露
注意一下map和array,可能原来就有

测例比较简单:
std::unordered_map<std::string, lgraph::FieldData> fields;
fields["a"] = lgraph::FieldData();
cypher::FieldData f1(fields);
cypher::FieldData f2;
f2 = std::move(f1);

@spasserby spasserby merged commit 6e5585d into TuGraph-family:master Jun 27, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants