From 63234621e75acf29434172928f24f00338eb9282 Mon Sep 17 00:00:00 2001 From: Shylock Hg <33566796+Shylock-Hg@users.noreply.github.com> Date: Wed, 11 May 2022 14:43:06 +0800 Subject: [PATCH 1/3] Eliminate vid predication Filter. --- src/graph/context/ast/CypherAstContext.h | 12 +++---- src/graph/planner/match/LabelIndexSeek.cpp | 4 +-- .../planner/match/MatchClausePlanner.cpp | 3 +- src/graph/planner/match/MatchPathPlanner.cpp | 10 +++--- src/graph/planner/match/MatchPathPlanner.h | 4 +-- src/graph/planner/match/PropIndexSeek.cpp | 8 ++--- src/graph/planner/match/SegmentsConnector.cpp | 3 ++ src/graph/planner/match/VertexIdSeek.cpp | 8 +++-- .../planner/match/WhereClausePlanner.cpp | 4 +-- src/graph/util/ExpressionUtils.cpp | 35 ++++++++++++++++++- src/graph/util/ExpressionUtils.h | 4 +++ tests/tck/features/match/SeekById.feature | 34 ++++++++++++++++++ .../features/match/SeekById.intVid.feature | 34 ++++++++++++++++++ .../optimizer/PrunePropertiesRule.feature | 15 +++----- 14 files changed, 142 insertions(+), 36 deletions(-) diff --git a/src/graph/context/ast/CypherAstContext.h b/src/graph/context/ast/CypherAstContext.h index cf70d4995b5..feacef056cd 100644 --- a/src/graph/context/ast/CypherAstContext.h +++ b/src/graph/context/ast/CypherAstContext.h @@ -204,11 +204,11 @@ struct PatternContext { }; struct NodeContext final : PatternContext { - NodeContext(QueryContext* q, Expression* b, GraphSpaceID g, NodeInfo* i) - : PatternContext(PatternKind::kNode), qctx(q), bindFilter(b), spaceId(g), info(i) {} + NodeContext(QueryContext* q, WhereClauseContext* b, GraphSpaceID g, NodeInfo* i) + : PatternContext(PatternKind::kNode), qctx(q), bindWhereClause(b), spaceId(g), info(i) {} QueryContext* qctx; - Expression* bindFilter; + WhereClauseContext* bindWhereClause; GraphSpaceID spaceId; NodeInfo* info{nullptr}; std::unordered_set* nodeAliasesAvailable; @@ -221,11 +221,11 @@ struct NodeContext final : PatternContext { }; struct EdgeContext final : PatternContext { - EdgeContext(QueryContext* q, Expression* b, GraphSpaceID g, EdgeInfo* i) - : PatternContext(PatternKind::kEdge), qctx(q), bindFilter(b), spaceId(g), info(i) {} + EdgeContext(QueryContext* q, WhereClauseContext* b, GraphSpaceID g, EdgeInfo* i) + : PatternContext(PatternKind::kEdge), qctx(q), bindWhereClause(b), spaceId(g), info(i) {} QueryContext* qctx; - Expression* bindFilter; + WhereClauseContext* bindWhereClause; GraphSpaceID spaceId; EdgeInfo* info{nullptr}; diff --git a/src/graph/planner/match/LabelIndexSeek.cpp b/src/graph/planner/match/LabelIndexSeek.cpp index 412d28d2f73..54b07f43628 100644 --- a/src/graph/planner/match/LabelIndexSeek.cpp +++ b/src/graph/planner/match/LabelIndexSeek.cpp @@ -84,8 +84,8 @@ StatusOr LabelIndexSeek::transformNode(NodeContext* nodeCtx) { // and it should be converted to an `optRule` after the match validator is // refactored auto* pool = nodeCtx->qctx->objPool(); - if (nodeCtx->bindFilter != nullptr) { - auto* filter = nodeCtx->bindFilter; + if (nodeCtx->bindWhereClause != nullptr && nodeCtx->bindWhereClause->filter != nullptr) { + auto* filter = nodeCtx->bindWhereClause->filter; const auto& nodeAlias = nodeCtx->info->alias; const auto& schemaName = nodeCtx->scanInfo.schemaNames.back(); diff --git a/src/graph/planner/match/MatchClausePlanner.cpp b/src/graph/planner/match/MatchClausePlanner.cpp index 6330986e3bc..f1b8b9c5bc4 100644 --- a/src/graph/planner/match/MatchClausePlanner.cpp +++ b/src/graph/planner/match/MatchClausePlanner.cpp @@ -36,10 +36,9 @@ StatusOr MatchClausePlanner::transform(CypherClauseContextBase* clauseC for (auto iter = pathInfos.begin(); iter < pathInfos.end(); ++iter) { auto& nodeInfos = iter->nodeInfos; MatchPathPlanner matchPathPlanner; - auto bindFilter = matchClauseCtx->where ? matchClauseCtx->where->filter : nullptr; auto result = matchPathPlanner.transform(matchClauseCtx->qctx, matchClauseCtx->space.id, - bindFilter, + matchClauseCtx->where.get(), matchClauseCtx->aliasesAvailable, nodeAliasesSeen, *iter); diff --git a/src/graph/planner/match/MatchPathPlanner.cpp b/src/graph/planner/match/MatchPathPlanner.cpp index 404596fb136..ecca87f9448 100644 --- a/src/graph/planner/match/MatchPathPlanner.cpp +++ b/src/graph/planner/match/MatchPathPlanner.cpp @@ -117,7 +117,7 @@ static Expression* nodeId(ObjectPool* pool, const NodeInfo& node) { StatusOr MatchPathPlanner::transform( QueryContext* qctx, GraphSpaceID spaceId, - Expression* bindFilter, + WhereClauseContext* bindWhere, const std::unordered_map& aliasesAvailable, std::unordered_set nodeAliasesSeen, Path& path) { @@ -135,7 +135,7 @@ StatusOr MatchPathPlanner::transform( edgeInfos, qctx, spaceId, - bindFilter, + bindWhere, aliasesAvailable, nodeAliasesSeen, startFromEdge, @@ -158,7 +158,7 @@ Status MatchPathPlanner::findStarts( std::vector& edgeInfos, QueryContext* qctx, GraphSpaceID spaceId, - Expression* bindFilter, + WhereClauseContext* bindWhereClause, const std::unordered_map& aliasesAvailable, std::unordered_set nodeAliasesSeen, bool& startFromEdge, @@ -178,7 +178,7 @@ Status MatchPathPlanner::findStarts( // Find the start plan node for (auto& finder : startVidFinders) { for (size_t i = 0; i < nodeInfos.size() && !foundStart; ++i) { - auto nodeCtx = NodeContext(qctx, bindFilter, spaceId, &nodeInfos[i]); + auto nodeCtx = NodeContext(qctx, bindWhereClause, spaceId, &nodeInfos[i]); nodeCtx.nodeAliasesAvailable = &allNodeAliasesAvailable; auto nodeFinder = finder(); if (nodeFinder->match(&nodeCtx)) { @@ -197,7 +197,7 @@ Status MatchPathPlanner::findStarts( } if (i != nodeInfos.size() - 1) { - auto edgeCtx = EdgeContext(qctx, bindFilter, spaceId, &edgeInfos[i]); + auto edgeCtx = EdgeContext(qctx, bindWhereClause, spaceId, &edgeInfos[i]); auto edgeFinder = finder(); if (edgeFinder->match(&edgeCtx)) { auto plan = edgeFinder->transform(&edgeCtx); diff --git a/src/graph/planner/match/MatchPathPlanner.h b/src/graph/planner/match/MatchPathPlanner.h index 235deaf1178..51bcf9efe2c 100644 --- a/src/graph/planner/match/MatchPathPlanner.h +++ b/src/graph/planner/match/MatchPathPlanner.h @@ -15,7 +15,7 @@ class MatchPathPlanner final { StatusOr transform(QueryContext* qctx, GraphSpaceID spaceId, - Expression* bindFilter, + WhereClauseContext* bindWhereClause, const std::unordered_map& aliasesAvailable, std::unordered_set nodeAliasesSeen, Path& path); @@ -25,7 +25,7 @@ class MatchPathPlanner final { std::vector& edgeInfos, QueryContext* qctx, GraphSpaceID spaceId, - Expression* bindFilter, + WhereClauseContext* bindWhereClause, const std::unordered_map& aliasesAvailable, std::unordered_set nodeAliases, bool& startFromEdge, diff --git a/src/graph/planner/match/PropIndexSeek.cpp b/src/graph/planner/match/PropIndexSeek.cpp index f93f65dac84..f3b75f0a54e 100644 --- a/src/graph/planner/match/PropIndexSeek.cpp +++ b/src/graph/planner/match/PropIndexSeek.cpp @@ -25,9 +25,9 @@ bool PropIndexSeek::matchEdge(EdgeContext* edgeCtx) { } Expression* filter = nullptr; - if (edgeCtx->bindFilter != nullptr) { + if (edgeCtx->bindWhereClause != nullptr && edgeCtx->bindWhereClause->filter != nullptr) { filter = MatchSolver::makeIndexFilter( - edge.types.back(), edge.alias, edgeCtx->bindFilter, edgeCtx->qctx, true); + edge.types.back(), edge.alias, edgeCtx->bindWhereClause->filter, edgeCtx->qctx, true); } if (filter == nullptr) { if (edge.props != nullptr && !edge.props->items().empty()) { @@ -124,9 +124,9 @@ bool PropIndexSeek::matchNode(NodeContext* nodeCtx) { Expression* filterInWhere = nullptr; Expression* filterInPattern = nullptr; - if (nodeCtx->bindFilter != nullptr) { + if (nodeCtx->bindWhereClause != nullptr && nodeCtx->bindWhereClause->filter != nullptr) { filterInWhere = MatchSolver::makeIndexFilter( - node.labels.back(), node.alias, nodeCtx->bindFilter, nodeCtx->qctx); + node.labels.back(), node.alias, nodeCtx->bindWhereClause->filter, nodeCtx->qctx); } if (!node.labelProps.empty()) { auto props = node.labelProps.back(); diff --git a/src/graph/planner/match/SegmentsConnector.cpp b/src/graph/planner/match/SegmentsConnector.cpp index 50befddf20a..4fe22a89a37 100644 --- a/src/graph/planner/match/SegmentsConnector.cpp +++ b/src/graph/planner/match/SegmentsConnector.cpp @@ -91,6 +91,9 @@ SubPlan SegmentsConnector::cartesianProduct(QueryContext* qctx, } SubPlan SegmentsConnector::addInput(const SubPlan& left, const SubPlan& right, bool copyColNames) { + if (left.root == nullptr) { + return right; + } SubPlan newPlan = left; DCHECK(left.root->isSingleInput()); if (left.tail->isSingleInput()) { diff --git a/src/graph/planner/match/VertexIdSeek.cpp b/src/graph/planner/match/VertexIdSeek.cpp index a232336fc8d..2531bb9743d 100644 --- a/src/graph/planner/match/VertexIdSeek.cpp +++ b/src/graph/planner/match/VertexIdSeek.cpp @@ -27,7 +27,7 @@ StatusOr VertexIdSeek::transformEdge(EdgeContext *edgeCtx) { bool VertexIdSeek::matchNode(NodeContext *nodeCtx) { auto &node = *nodeCtx->info; // auto *matchClauseCtx = nodeCtx->matchClauseCtx; - if (nodeCtx->bindFilter == nullptr) { + if (nodeCtx->bindWhereClause == nullptr || nodeCtx->bindWhereClause->filter == nullptr) { return false; } @@ -37,7 +37,7 @@ bool VertexIdSeek::matchNode(NodeContext *nodeCtx) { } VidExtractVisitor vidExtractVisitor(nodeCtx->qctx); - nodeCtx->bindFilter->accept(&vidExtractVisitor); + nodeCtx->bindWhereClause->filter->accept(&vidExtractVisitor); auto vidResult = vidExtractVisitor.moveVidPattern(); if (vidResult.spec != VidExtractVisitor::VidPattern::Special::kInUsed) { return false; @@ -47,6 +47,10 @@ bool VertexIdSeek::matchNode(NodeContext *nodeCtx) { if (nodeVid.second.kind == VidExtractVisitor::VidPattern::Vids::Kind::kIn) { if (nodeVid.first == node.alias) { nodeCtx->ids = std::move(nodeVid.second.vids); + // Eliminate Filter when it's complete predication about Vertex Id + if (ExpressionUtils::isVidPredication(nodeCtx->bindWhereClause->filter)) { + nodeCtx->bindWhereClause->filter = nullptr; + } return true; } } diff --git a/src/graph/planner/match/WhereClausePlanner.cpp b/src/graph/planner/match/WhereClausePlanner.cpp index bbcb19f0595..19d4cb0f61a 100644 --- a/src/graph/planner/match/WhereClausePlanner.cpp +++ b/src/graph/planner/match/WhereClausePlanner.cpp @@ -19,8 +19,8 @@ StatusOr WhereClausePlanner::transform(CypherClauseContextBase* ctx) { } auto* wctx = static_cast(ctx); + SubPlan wherePlan; if (wctx->filter) { - SubPlan wherePlan; auto* newFilter = MatchSolver::doRewrite(wctx->qctx, wctx->aliasesAvailable, wctx->filter); wherePlan.root = Filter::make(wctx->qctx, nullptr, newFilter, needStableFilter_); wherePlan.tail = wherePlan.root; @@ -46,7 +46,7 @@ StatusOr WhereClausePlanner::transform(CypherClauseContextBase* ctx) { return wherePlan; } - return Status::OK(); + return wherePlan; } } // namespace graph } // namespace nebula diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp index c040b198a4b..0aadfc9105a 100644 --- a/src/graph/util/ExpressionUtils.cpp +++ b/src/graph/util/ExpressionUtils.cpp @@ -1205,7 +1205,40 @@ bool ExpressionUtils::checkExprDepth(const Expression *expr) { } } return true; -} // namespace graph +} + +/*static*/ bool ExpressionUtils::isVidPredication(const Expression *expr) { + if (DCHECK_NOTNULL(expr)->kind() != Expression::Kind::kRelIn && + expr->kind() != Expression::Kind::kRelEQ) { + DLOG(ERROR) << "DEBUG POINT:0"; + return false; + } + const auto *relExpr = static_cast(expr); + if (relExpr->left()->kind() != Expression::Kind::kFunctionCall) { + DLOG(ERROR) << "DEBUG POINT:1"; + return false; + } + const auto *fCallExpr = static_cast(relExpr->left()); + if (fCallExpr->name() != "id" || fCallExpr->args()->numArgs() != 1 || + fCallExpr->args()->args().front()->kind() != Expression::Kind::kLabel) { + DLOG(ERROR) << "DEBUG POINT:2"; + return false; + } + if (expr->kind() == Expression::Kind::kRelIn) { + // id(V) IN [List] + if (relExpr->right()->kind() != Expression::Kind::kList) { + DLOG(ERROR) << "DEBUG POINT:3"; + return false; + } + } else if (expr->kind() == Expression::Kind::kRelEQ) { + // id(V) = Value + if (relExpr->right()->kind() != Expression::Kind::kConstant) { + DLOG(ERROR) << "DEBUG POINT:4"; + return false; + } + } + return true; +} } // namespace graph } // namespace nebula diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h index 761db42fd33..f13074976eb 100644 --- a/src/graph/util/ExpressionUtils.h +++ b/src/graph/util/ExpressionUtils.h @@ -207,6 +207,10 @@ class ExpressionUtils { // Checks if the depth of the expression exceeds the maximum // This is used in the parser to prevent OOM due to highly nested expression static bool checkExprDepth(const Expression* expr); + + // Whether the whole expression is vertex id predication + // e.g. id(v) == 1, id(v) IN [...] + static bool isVidPredication(const Expression* expr); }; } // namespace graph diff --git a/tests/tck/features/match/SeekById.feature b/tests/tck/features/match/SeekById.feature index 5ac4839f7ac..14a29d24358 100644 --- a/tests/tck/features/match/SeekById.feature +++ b/tests/tck/features/match/SeekById.feature @@ -365,3 +365,37 @@ Feature: Match seek by id | 'Aron Baynes' | | 'Blake Griffin' | | 'Grant Hill' | + + Scenario: check plan + When profiling query: + """ + MATCH (v) + WHERE id(v) == 'Paul Gasol' + RETURN v.player.name AS Name, v.player.age AS Age + """ + Then the result should be, in any order: + | Name | Age | + | 'Paul Gasol' | 38 | + And the execution plan should be: + | id | name | dependencies | operator info | + | 12 | Project | 2 | | + | 2 | AppendVertices | 16 | | + | 16 | Dedup | 7 | | + | 7 | PassThrough | 0 | | + | 0 | Start | | | + When profiling query: + """ + MATCH (v) + WHERE id(v) IN ['Paul Gasol'] + RETURN v.player.name AS Name, v.player.age AS Age + """ + Then the result should be, in any order: + | Name | Age | + | 'Paul Gasol' | 38 | + And the execution plan should be: + | id | name | dependencies | operator info | + | 12 | Project | 2 | | + | 2 | AppendVertices | 16 | | + | 16 | Dedup | 7 | | + | 7 | PassThrough | 0 | | + | 0 | Start | | | diff --git a/tests/tck/features/match/SeekById.intVid.feature b/tests/tck/features/match/SeekById.intVid.feature index e693b3b5dfe..6917c43ac70 100644 --- a/tests/tck/features/match/SeekById.intVid.feature +++ b/tests/tck/features/match/SeekById.intVid.feature @@ -405,3 +405,37 @@ Feature: Match seek by id Then the result should be, in any order: | v | | (-100 :player{age: 32, name: "Tim"}) | + + Scenario: check plan + When profiling query: + """ + MATCH (v) + WHERE id(v) == hash('Paul Gasol') + RETURN v.player.name AS Name, v.player.age AS Age + """ + Then the result should be, in any order: + | Name | Age | + | 'Paul Gasol' | 38 | + And the execution plan should be: + | id | name | dependencies | operator info | + | 12 | Project | 2 | | + | 2 | AppendVertices | 16 | | + | 16 | Dedup | 7 | | + | 7 | PassThrough | 0 | | + | 0 | Start | | | + When profiling query: + """ + MATCH (v) + WHERE id(v) IN [hash('Paul Gasol')] + RETURN v.player.name AS Name, v.player.age AS Age + """ + Then the result should be, in any order: + | Name | Age | + | 'Paul Gasol' | 38 | + And the execution plan should be: + | id | name | dependencies | operator info | + | 12 | Project | 2 | | + | 2 | AppendVertices | 16 | | + | 16 | Dedup | 7 | | + | 7 | PassThrough | 0 | | + | 0 | Start | | | diff --git a/tests/tck/features/optimizer/PrunePropertiesRule.feature b/tests/tck/features/optimizer/PrunePropertiesRule.feature index a9e7facd662..98828d3d076 100644 --- a/tests/tck/features/optimizer/PrunePropertiesRule.feature +++ b/tests/tck/features/optimizer/PrunePropertiesRule.feature @@ -173,8 +173,7 @@ Feature: Prune Properties rule | id | name | dependencies | operator info | | 15 | DataCollect | 16 | | | 16 | TopN | 12 | | - | 12 | Project | 17 | | - | 17 | Filter | 9 | | + | 12 | Project | 9 | | | 9 | BiInnerJoin | 22, 23 | | | 22 | Project | 5 | | | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":9}]" } | @@ -208,8 +207,7 @@ Feature: Prune Properties rule | id | name | dependencies | operator info | | 19 | DataCollect | 20 | | | 20 | TopN | 23 | | - | 23 | Project | 21 | | - | 21 | Filter | 13 | | + | 23 | Project | 13 | | | 13 | BiInnerJoin | 9, 30 | | | 9 | BiInnerJoin | 28, 29 | | | 28 | Project | 5 | | @@ -257,8 +255,7 @@ Feature: Prune Properties rule | 17 | TopN | 13 | | | 13 | Project | 12 | | | 12 | BiInnerJoin | 19, 11 | | - | 19 | Project | 18 | | - | 18 | Filter | 5 | | + | 19 | Project | 5 | | | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":9}]" } | | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":9}]" } | | 2 | Dedup | 1 | | @@ -294,8 +291,7 @@ Feature: Prune Properties rule | 21 | TopN | 17 | | | 17 | Project | 16 | | | 16 | BiInnerJoin | 23, 14 | | - | 23 | Project | 22 | | - | 22 | Filter | 5 | | + | 23 | Project | 5 | | | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":9}]" } | | 4 | Traverse | 2 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":9}]" } | | 2 | Dedup | 1 | | @@ -363,8 +359,7 @@ Feature: Prune Properties rule | 17 | TopN | 13 | | | 13 | Project | 12 | | | 12 | BiLeftJoin | 19, 11 | | - | 19 | Project | 18 | | - | 18 | Filter | 5 | | + | 19 | Project | 5 | | | 5 | AppendVertices | 4 | { "props": "[{\"props\":[\"name\"],\"tagId\":9}]" } | | 4 | Traverse | 2 | | | 2 | Dedup | 1 | | From 806b45ab6774e560e77ef2beb6ed721a4f015bd3 Mon Sep 17 00:00:00 2001 From: Shylock Hg <33566796+Shylock-Hg@users.noreply.github.com> Date: Wed, 11 May 2022 15:11:07 +0800 Subject: [PATCH 2/3] Fix cases. --- src/graph/validator/test/QueryValidatorTest.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/graph/validator/test/QueryValidatorTest.cpp b/src/graph/validator/test/QueryValidatorTest.cpp index 8875eb8024c..c1a1e62275e 100644 --- a/src/graph/validator/test/QueryValidatorTest.cpp +++ b/src/graph/validator/test/QueryValidatorTest.cpp @@ -1228,7 +1228,6 @@ TEST_F(QueryValidatorTest, TestMatch) { "RETURN type(r) AS Type, v2.person.name AS Name"; std::vector expected = { PK::kProject, - PK::kFilter, PK::kProject, PK::kAppendVertices, PK::kTraverse, @@ -1245,7 +1244,6 @@ TEST_F(QueryValidatorTest, TestMatch) { "RETURN v1, v2"; std::vector expected = { PK::kProject, - PK::kFilter, PK::kProject, PK::kAppendVertices, PK::kTraverse, From a0bb8d09e3307e6d061f91c090aadecde64abc92 Mon Sep 17 00:00:00 2001 From: Shylock Hg <33566796+Shylock-Hg@users.noreply.github.com> Date: Wed, 11 May 2022 17:37:21 +0800 Subject: [PATCH 3/3] Remove unused code. --- src/graph/util/ExpressionUtils.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp index 0aadfc9105a..945e8eebac2 100644 --- a/src/graph/util/ExpressionUtils.cpp +++ b/src/graph/util/ExpressionUtils.cpp @@ -1210,30 +1210,25 @@ bool ExpressionUtils::checkExprDepth(const Expression *expr) { /*static*/ bool ExpressionUtils::isVidPredication(const Expression *expr) { if (DCHECK_NOTNULL(expr)->kind() != Expression::Kind::kRelIn && expr->kind() != Expression::Kind::kRelEQ) { - DLOG(ERROR) << "DEBUG POINT:0"; return false; } const auto *relExpr = static_cast(expr); if (relExpr->left()->kind() != Expression::Kind::kFunctionCall) { - DLOG(ERROR) << "DEBUG POINT:1"; return false; } const auto *fCallExpr = static_cast(relExpr->left()); if (fCallExpr->name() != "id" || fCallExpr->args()->numArgs() != 1 || fCallExpr->args()->args().front()->kind() != Expression::Kind::kLabel) { - DLOG(ERROR) << "DEBUG POINT:2"; return false; } if (expr->kind() == Expression::Kind::kRelIn) { // id(V) IN [List] if (relExpr->right()->kind() != Expression::Kind::kList) { - DLOG(ERROR) << "DEBUG POINT:3"; return false; } } else if (expr->kind() == Expression::Kind::kRelEQ) { // id(V) = Value if (relExpr->right()->kind() != Expression::Kind::kConstant) { - DLOG(ERROR) << "DEBUG POINT:4"; return false; } }