Enhance property pruner for aggregate (#5301)

* fix property pruner for aggregate * update * add tck --------- Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com>
vesoft-inc · Feb 1, 2023 · 970e06c · 970e06c
1 parent afd6a4d
commit 970e06c
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 20 deletions.
diff --git a/src/graph/visitor/PropertyTrackerVisitor.cpp b/src/graph/visitor/PropertyTrackerVisitor.cpp
@@ -10,6 +10,36 @@
 
 namespace nebula {
 namespace graph {
+
+std::string PropertyTracker::toString() const {
+ std::string str;
+ str += "vertexPropsMap: ";
+ for (const auto &v : vertexPropsMap) {
+ str += v.first + ": ";
+ for (const auto &t : v.second) {
+ str += std::to_string(t.first) + ": ";
+ for (const auto &p : t.second) {
+ str += p + ", ";
+ }
+ }
+ }
+ str += "edgePropsMap: ";
+ for (const auto &v : edgePropsMap) {
+ str += v.first + ": ";
+ for (const auto &t : v.second) {
+ str += std::to_string(t.first) + ": ";
+ for (const auto &p : t.second) {
+ str += p + ", ";
+ }
+ }
+ }
+ str += "colsSet: ";
+ for (const auto &c : colsSet) {
+ str += c + ", ";
+ }
+ return str;
+}
+
 void PropertyTracker::insertVertexProp(const std::string &name,
  TagID tagId,
  const std::string &propName) {

diff --git a/src/graph/visitor/PropertyTrackerVisitor.h b/src/graph/visitor/PropertyTrackerVisitor.h
@@ -30,6 +30,8 @@ struct PropertyTracker {
  void insertVertexProp(const std::string& name, TagID tagId, const std::string& propName);
  void insertEdgeProp(const std::string& name, EdgeType type, const std::string& propName);
  void insertCols(const std::string& name);
+
+ std::string toString() const;
 };
 
 class PropertyTrackerVisitor : public ExprVisitorImpl {

diff --git a/src/graph/visitor/PrunePropertiesVisitor.cpp b/src/graph/visitor/PrunePropertiesVisitor.cpp
@@ -107,12 +107,27 @@ void PrunePropertiesVisitor::visitCurrent(Project *node) {
 }
 
 void PrunePropertiesVisitor::visit(Aggregate *node) {
- rootNode_ = false;
  visitCurrent(node);
  status_ = depsPruneProperties(node->dependencies());
 }
 
 void PrunePropertiesVisitor::visitCurrent(Aggregate *node) {
+ if (rootNode_) {
+ for (auto *groupKey : node->groupKeys()) {
+ status_ = extractPropsFromExpr(groupKey);
+ if (!status_.ok()) {
+ return;
+ }
+ }
+ for (auto *groupItem : node->groupItems()) {
+ status_ = extractPropsFromExpr(groupItem);
+ if (!status_.ok()) {
+ return;
+ }
+ }
+ rootNode_ = false;
+ return;
+ }
  for (auto *groupKey : node->groupKeys()) {
  if (groupKey->kind() == Expression::Kind::kVarProperty ||
  groupKey->kind() == Expression::Kind::kInputProperty ||
@@ -125,6 +140,11 @@ void PrunePropertiesVisitor::visitCurrent(Aggregate *node) {
  }
  }
  for (auto *groupItem : node->groupItems()) {
+ if (groupItem->kind() == Expression::Kind::kVarProperty ||
+ groupItem->kind() == Expression::Kind::kInputProperty ||
+ groupItem->kind() == Expression::Kind::kConstant) {
+ continue;
+ }
  status_ = extractPropsFromExpr(groupItem);
  if (!status_.ok()) {
  return;

diff --git a/tests/tck/features/optimizer/PrunePropertiesRule.feature b/tests/tck/features/optimizer/PrunePropertiesRule.feature
@@ -328,6 +328,31 @@ Feature: Prune Properties rule
  | 6 | Argument | 0 | |
  | 0 | Start | | |
 
+ Scenario: Project after aggregate
+ Given a graph with space named "nba"
+ When profiling query:
+ """
+ MATCH (v1:player)-[e:like]->(v2:player)
+ WHERE id(v1) == "Tony Parker"
+ WITH v1, v2, count(e.likeness) AS cnt
+ RETURN v1.player.age, v2.player.age, cnt
+ """
+ Then the result should be, in any order:
+ | v1.player.age | v2.player.age | cnt |
+ | 36 | 42 | 1 |
+ | 36 | 41 | 1 |
+ | 36 | 33 | 1 |
+ And the execution plan should be:
+ | id | name | dependencies | profiling data | operator info |
+ | 8 | Project | 7 | | |
+ | 7 | Aggregate | 6 | | |
+ | 6 | Project | 10 | | |
+ | 10 | AppendVertices | 9 | | { "props": "[{ \"props\":[\"age\", \"_tag\"]}]" } |
+ | 9 | Traverse | 2 | | { "vertexProps": "[{ \"props\":[\"age\"]}]", "edgeProps": "[{\"props\": [\"_src\", \"_dst\", \"_rank\", \"_type\", \"likeness\"]}]" } |
+ | 2 | Dedup | 1 | | |
+ | 1 | PassThrough | 3 | | |
+ | 3 | Start | | | |
+
  # The schema id is not fixed in standalone cluster, so we skip it
  @distonly
  Scenario: Optional Match
@@ -547,25 +572,25 @@ Feature: Prune Properties rule
  | "Spurs" | 11 |
  | "Hornets" | 3 |
  And the execution plan should be:
- | id | name | dependencies | operator info  |
- | 19 | Aggregate | 18 |  |
- | 18 | Aggregate | 27 |  |
- | 27 | HashLeftJoin | 10,26 |  |
- | 10 | Aggregate | 21 |  |
- | 21 | Project | 20 |  |
- | 20 | Filter | 25 |  |
- | 25 | AppendVertices | 24 | { "props": "[{ \"props\":[\"name\",\"age\",\"_tag\"]},{\"props\":[\"name\",\"speciality\",\"_tag\"] },{ \"props\":[\"name\",\"_tag\"]}]" }  |
- | 24 | Traverse | 23 | {"vertexProps": "[{ \"props\":[\"age\"]}]", "edgeProps": "[{ \"props\": [\"_type\", \"_rank\", \"_dst\"]}]" }  |
- | 23 | Traverse | 22 | {"vertexProps": "", "edgeProps": "[{ \"props\": [\"_type\", \"_rank\", \"_dst\"]}]" }  |
- | 22 | Traverse | 2 | {"vertexProps": "[{ \"props\":[\"name\",\"age\",\"_tag\"]},{\"props\":[\"name\",\"speciality\",\"_tag\"] },{ \"props\":[\"name\",\"_tag\"]}]", "edgeProps": "[{ \"props\": [\"_type\", \"_rank\", \"_dst\"]}, { \"props\": [\"_type\", \"_rank\", \"_dst\"]}]" } |
- | 2 | Dedup | 1 |  |
- | 1 | PassThrough | 3 |  |
- | 3 | Start | |  |
- | 26 | Project | 14 |  |
- | 14 | Traverse | 13 | {"vertexProps": "", "edgeProps": "[{ \"props\": [\"_src\", \"_type\", \"_rank\", \"_dst\", \"start_year\", \"end_year\"]}]" }  |
- | 13 | Traverse | 12 | {"vertexProps": "", "edgeProps": "[{ \"props\": [\"_type\", \"_rank\", \"_dst\"]}]" }  |
- | 12 | Traverse | 11 | {"vertexProps": "[{ \"props\":[\"name\",\"age\",\"_tag\"]},{\"props\":[\"name\",\"speciality\",\"_tag\"] },{ \"props\":[\"name\",\"_tag\"]}]", "edgeProps": "[{ \"props\": [\"_type\", \"_rank\", \"_dst\"]}, { \"props\": [\"_type\", \"_rank\", \"_dst\"]}]" } |
- | 11 | Argument | |  |
+ | id | name | dependencies | operator info |
+ | 19 | Aggregate | 18 | |
+ | 18 | Aggregate | 27 | |
+ | 27 | HashLeftJoin | 10,26 | |
+ | 10 | Aggregate | 21 | |
+ | 21 | Project | 20 | |
+ | 20 | Filter | 25 | |
+ | 25 | AppendVertices | 24 | { "props": "[{ \"props\":[\"name\",\"_tag\"]}]" } |
+ | 24 | Traverse | 23 | {"vertexProps": "[{ \"props\":[\"age\"]}]", "edgeProps": "[{ \"props\": [\"_type\", \"_rank\", \"_dst\"]}]" } |
+ | 23 | Traverse | 22 | {"vertexProps": "", "edgeProps": "[{ \"props\": [\"_type\", \"_rank\", \"_dst\"]}]" } |
+ | 22 | Traverse | 2 | {"vertexProps": "", "edgeProps": "[{ \"props\": [\"_type\", \"_rank\", \"_dst\"]}, { \"props\": [\"_type\", \"_rank\", \"_dst\"]}]" } |
+ | 2 | Dedup | 1 | |
+ | 1 | PassThrough | 3 | |
+ | 3 | Start | | |
+ | 26 | Project | 14 | |
+ | 14 | Traverse | 13 | {"vertexProps": "", "edgeProps": "[{ \"props\": [\"_src\", \"_type\", \"_rank\", \"_dst\", \"start_year\", \"end_year\"]}]" } |
+ | 13 | Traverse | 12 | {"vertexProps": "", "edgeProps": "[{ \"props\": [\"_type\", \"_rank\", \"_dst\"]}]" } |
+ | 12 | Traverse | 11 | {"vertexProps": "", "edgeProps": "[{ \"props\": [\"_type\", \"_rank\", \"_dst\"]}, { \"props\": [\"_type\", \"_rank\", \"_dst\"]}]" } |
+ | 11 | Argument | | |
 
  @distonly
  Scenario: test properties: