From 491df426df2a2cab690e22cd52145bf38bcf6252 Mon Sep 17 00:00:00 2001
From: Shylock Hg <33566796+Shylock-Hg@users.noreply.github.com>
Date: Mon, 28 Dec 2020 21:37:31 +0800
Subject: [PATCH] Correct the filter not clone issue. (#531)
Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com>
---
src/planner/PlanNode.h | 14 +++
src/planner/Query.cpp | 112 ++++++++++++++----------
src/planner/Query.h | 17 ++++
tests/tck/features/go/GO.IntVid.feature | 10 +++
tests/tck/features/go/GO.feature | 9 ++
5 files changed, 115 insertions(+), 47 deletions(-)
diff --git a/src/planner/PlanNode.h b/src/planner/PlanNode.h
index f9587335..8c7b9cef 100644
--- a/src/planner/PlanNode.h
+++ b/src/planner/PlanNode.h
@@ -228,6 +228,12 @@ public:
protected:
static void addDescription(std::string key, std::string value, PlanNodeDescription* desc);
+ void clone(const PlanNode &node) {
+ // TODO maybe shall copy cost_ and dependencies_ too
+ inputVars_ = node.inputVars_;
+ outputVars_ = node.outputVars_;
+ }
+
QueryContext* qctx_{nullptr};
Kind kind_{Kind::kUnknown};
int64_t id_{-1};
@@ -255,6 +261,10 @@ protected:
dependencies_.emplace_back(dep);
}
+ void clone(const SingleDependencyNode &node) {
+ PlanNode::clone(node);
+ }
+
std::unique_ptr<PlanNodeDescription> explain() const override;
};
@@ -292,6 +302,10 @@ public:
std::unique_ptr<PlanNodeDescription> explain() const override;
protected:
+ void clone(const SingleInputNode &node) {
+ SingleDependencyNode::clone(node);
+ }
+
SingleInputNode(QueryContext* qctx, Kind kind, const PlanNode* dep)
: SingleDependencyNode(qctx, kind, dep) {
if (dep != nullptr) {
diff --git a/src/planner/Query.cpp b/src/planner/Query.cpp
index 6aed8397..a7b75b0d 100644
--- a/src/planner/Query.cpp
+++ b/src/planner/Query.cpp
@@ -29,39 +29,7 @@ std::unique_ptr<PlanNodeDescription> Explore::explain() const {
GetNeighbors* GetNeighbors::clone(QueryContext* qctx) const {
auto newGN = GetNeighbors::make(qctx, nullptr, space_);
- newGN->setSrc(qctx->objPool()->add(src_->clone().release()));
- newGN->setEdgeTypes(edgeTypes_);
- newGN->setEdgeDirection(edgeDirection_);
- newGN->setDedup(dedup_);
- newGN->setRandom(random_);
- newGN->setLimit(limit_);
- newGN->setOrderBy(orderBy_);
- newGN->setInputVar(inputVar());
- newGN->setOutputVar(outputVar());
-
- if (vertexProps_) {
- auto vertexProps = *vertexProps_;
- auto vertexPropsPtr = std::make_unique<decltype(vertexProps)>(vertexProps);
- newGN->setVertexProps(std::move(vertexPropsPtr));
- }
-
- if (edgeProps_) {
- auto edgeProps = *edgeProps_;
- auto edgePropsPtr = std::make_unique<decltype(edgeProps)>(std::move(edgeProps));
- newGN->setEdgeProps(std::move(edgePropsPtr));
- }
-
- if (statProps_) {
- auto statProps = *statProps_;
- auto statPropsPtr = std::make_unique<decltype(statProps)>(std::move(statProps));
- newGN->setStatProps(std::move(statPropsPtr));
- }
-
- if (exprs_) {
- auto exprs = *exprs_;
- auto exprsPtr = std::make_unique<decltype(exprs)>(exprs);
- newGN->setExprs(std::move(exprsPtr));
- }
+ newGN->clone(*this);
return newGN;
}
@@ -83,6 +51,38 @@ std::unique_ptr<PlanNodeDescription> GetNeighbors::explain() const {
return desc;
}
+void GetNeighbors::clone(const GetNeighbors& g) {
+ Explore::clone(g);
+ setSrc(qctx_->objPool()->add(g.src_->clone().release()));
+ setEdgeTypes(g.edgeTypes_);
+ setEdgeDirection(g.edgeDirection_);
+ setRandom(g.random_);
+ if (g.vertexProps_) {
+ auto vertexProps = *g.vertexProps_;
+ auto vertexPropsPtr = std::make_unique<decltype(vertexProps)>(vertexProps);
+ setVertexProps(std::move(vertexPropsPtr));
+ }
+
+ if (g.edgeProps_) {
+ auto edgeProps = *g.edgeProps_;
+ auto edgePropsPtr = std::make_unique<decltype(edgeProps)>(std::move(edgeProps));
+ setEdgeProps(std::move(edgePropsPtr));
+ }
+
+ if (g.statProps_) {
+ auto statProps = *g.statProps_;
+ auto statPropsPtr = std::make_unique<decltype(statProps)>(std::move(statProps));
+ setStatProps(std::move(statPropsPtr));
+ }
+
+ if (g.exprs_) {
+ auto exprs = *g.exprs_;
+ auto exprsPtr = std::make_unique<decltype(exprs)>(exprs);
+ setExprs(std::move(exprsPtr));
+ }
+}
+
+
std::unique_ptr<PlanNodeDescription> GetVertices::explain() const {
auto desc = Explore::explain();
addDescription("src", src_ ? src_->toString() : "", desc.get());
@@ -103,14 +103,25 @@ std::unique_ptr<PlanNodeDescription> GetEdges::explain() const {
}
IndexScan* IndexScan::clone(QueryContext* qctx) const {
- auto ctx = std::make_unique<std::vector<storage::cpp2::IndexQueryContext>>();
- auto returnCols = std::make_unique<std::vector<std::string>>(*returnColumns());
auto* scan = IndexScan::make(
- qctx, nullptr, space(), std::move(ctx), std::move(returnCols), isEdge(), schemaId());
- scan->setOutputVar(this->outputVar());
+ qctx, nullptr, space(), nullptr, nullptr, isEdge(), schemaId());
+ scan->clone(*this);
return scan;
}
+void IndexScan::clone(const IndexScan &g) {
+ Explore::clone(g);
+ if (g.contexts_ != nullptr) {
+ contexts_ = std::make_unique<std::vector<storage::cpp2::IndexQueryContext>>(*g.contexts_);
+ }
+ if (g.returnCols_ != nullptr) {
+ returnCols_ = std::make_unique<std::vector<std::string>>(*g.returnCols_);
+ }
+ isEdge_ = g.isEdge_;
+ schemaId_ = g.schemaId_;
+ isEmptyResultSet_ = g.isEmptyResultSet_;
+}
+
std::unique_ptr<PlanNodeDescription> IndexScan::explain() const {
auto desc = Explore::explain();
addDescription("schemaId", util::toJson(schemaId_), desc.get());
@@ -127,17 +138,19 @@ std::unique_ptr<PlanNodeDescription> Filter::explain() const {
}
Project* Project::clone(QueryContext* qctx) const {
- auto cols = qctx->objPool()->add(new YieldColumns());
- for (auto col : columns()->columns()) {
- cols->addColumn((col->clone()).release());
- }
-
- auto newProj = Project::make(qctx, nullptr, cols);
- newProj->setInputVar(inputVar());
- newProj->setOutputVar(outputVar());
+ auto newProj = Project::make(qctx, nullptr, nullptr);
+ newProj->clone(*this);
return newProj;
}
+void Project::clone(const Project &p) {
+ SingleInputNode::clone(p);
+ cols_ = qctx_->objPool()->makeAndAdd<YieldColumns>();
+ for (const auto &col : p.columns()->columns()) {
+ cols_->addColumn(col->clone().release());
+ }
+}
+
std::unique_ptr<PlanNodeDescription> Project::explain() const {
auto desc = SingleInputNode::explain();
addDescription("columns", cols_ ? cols_->toString() : "", desc.get());
@@ -152,11 +165,16 @@ std::unique_ptr<PlanNodeDescription> Sort::explain() const {
Limit* Limit::clone(QueryContext* qctx) const {
auto newLimit = Limit::make(qctx, nullptr, offset_, count_);
- newLimit->setInputVar(inputVar());
- newLimit->setOutputVar(outputVar());
+ newLimit->clone(*this);
return newLimit;
}
+void Limit::clone(const Limit &l) {
+ SingleInputNode::clone(l);
+ offset_ = l.offset_;
+ count_ = l.count_;
+}
+
std::unique_ptr<PlanNodeDescription> Limit::explain() const {
auto desc = SingleInputNode::explain();
addDescription("offset", folly::to<std::string>(offset_), desc.get());
diff --git a/src/planner/Query.h b/src/planner/Query.h
index 6ac8b068..d6d6ca2a 100644
--- a/src/planner/Query.h
+++ b/src/planner/Query.h
@@ -90,6 +90,15 @@ protected:
: SingleInputNode(qctx, kind, input), space_(space) {}
protected:
+ void clone(const Explore& e) {
+ SingleInputNode::clone(e);
+ space_ = e.space_;
+ dedup_ = e.dedup_;
+ limit_ = e.limit_;
+ filter_ = e.filter_;
+ orderBy_ = e.orderBy_;
+ }
+
GraphSpaceID space_;
bool dedup_{false};
int64_t limit_{std::numeric_limits<int64_t>::max()};
@@ -215,6 +224,8 @@ private:
: Explore(qctx, Kind::kGetNeighbors, input, space) {}
private:
+ void clone(const GetNeighbors& g);
+
Expression* src_{nullptr};
std::vector<EdgeType> edgeTypes_;
storage::cpp2::EdgeDirection edgeDirection_{storage::cpp2::EdgeDirection::OUT_EDGE};
@@ -502,6 +513,8 @@ private:
isEmptyResultSet_ = isEmptyResultSet;
}
+ void clone(const IndexScan &g);
+
private:
IndexQueryCtx contexts_;
IndexReturnCols returnCols_;
@@ -617,6 +630,8 @@ private:
Project(QueryContext* qctx, PlanNode* input, YieldColumns* cols)
: SingleInputNode(qctx, Kind::kProject, input), cols_(cols) { }
+ void clone(const Project &p);
+
private:
YieldColumns* cols_{nullptr};
};
@@ -694,6 +709,8 @@ private:
count_ = count;
}
+ void clone(const Limit &l);
+
private:
int64_t offset_{-1};
int64_t count_{-1};
diff --git a/tests/tck/features/go/GO.IntVid.feature b/tests/tck/features/go/GO.IntVid.feature
index f5462b44..c802d664 100644
--- a/tests/tck/features/go/GO.IntVid.feature
+++ b/tests/tck/features/go/GO.IntVid.feature
@@ -1593,3 +1593,13 @@ Feature: IntegerVid Go Sentence
Then the result should be, in any order, with relax comparison:
| COUNT($-.id) |
| 4 |
+
+ @skip
+ Scenario: Integer Vid Bugfix filter not pushdown
+ When executing query:
+ """
+ GO FROM hash("Tim Duncan") OVER like WHERE like._dst == hash("Tony Parker") | limit 10;
+ """
+ Then the result should be, in any order, with relax comparison:
+ | like._dst |
+ | hash("Tony Parker") |
diff --git a/tests/tck/features/go/GO.feature b/tests/tck/features/go/GO.feature
index 61400991..f0ca0849 100644
--- a/tests/tck/features/go/GO.feature
+++ b/tests/tck/features/go/GO.feature
@@ -1593,3 +1593,12 @@ Feature: Go Sentence
Then the result should be, in any order, with relax comparison:
| COUNT($-.id) |
| 4 |
+
+ Scenario: Bugfix filter not pushdown
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like WHERE like._dst == "Tony Parker" | limit 10;
+ """
+ Then the result should be, in any order, with relax comparison:
+ | like._dst |
+ | "Tony Parker" |
--
GitLab