Skip to content

Commit

Permalink
Adddress @dangleptr's comment.
Browse files Browse the repository at this point in the history
  • Loading branch information
CPWstatic committed Feb 20, 2020
1 parent 641d50c commit 8ab536c
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 45 deletions.
9 changes: 7 additions & 2 deletions src/dataman/RowReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,12 @@ class RowReader {
static StatusOr<VariantType> getDefaultProp(const meta::SchemaProviderIf* schema,
const std::string& prop) {
auto& vType = schema->getFieldType(prop);
return getDefaultProp(vType.type);
auto defaultVal = getDefaultProp(vType.type);
if (!defaultVal.ok()) {
LOG(ERROR) << "Get default value for `" << prop << "' failed: " << defaultVal.status();
}

return defaultVal;
}

static StatusOr<VariantType> getDefaultProp(const nebula::cpp2::SupportedType& type) {
Expand All @@ -114,7 +119,7 @@ class RowReader {
}
default:
auto msg = folly::sformat("Unknown type: {}", static_cast<int32_t>(type));
LOG(ERROR) << "Unknown type: " << msg;
LOG(ERROR) << msg;
return Status::Error(msg);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/graph/FindPathExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ Status FindPathExecutor::doFilter(
DCHECK(vdata.__isset.edge_data);
for (auto &edata : vdata.edge_data) {
auto edgeType = edata.type;
auto it = edgeSchema.find(std::abs(edgeType));
auto it = edgeSchema.find(edgeType);
DCHECK(it != edgeSchema.end());
Neighbors neighbors;
for (auto& edge : edata.get_edges()) {
Expand Down
74 changes: 64 additions & 10 deletions src/graph/GoExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ Status GoExecutor::addToEdgeTypes(EdgeType type) {
edgeTypes_.push_back(type);
break;
}
case OverClause::Direction::kBiDirect: {
case OverClause::Direction::kBidirect: {
edgeTypes_.push_back(type);
type = -type;
edgeTypes_.push_back(type);
Expand Down Expand Up @@ -717,7 +717,7 @@ StatusOr<std::vector<storage::cpp2::PropDef>> GoExecutor::getStepOutProps() {
storage::cpp2::PropDef pd;
pd.owner = storage::cpp2::PropOwner::EDGE;
pd.name = _DST;
pd.id.set_edge_type(std::abs(e));
pd.id.set_edge_type(e);
props.emplace_back(std::move(pd));
}
return props;
Expand All @@ -726,7 +726,7 @@ StatusOr<std::vector<storage::cpp2::PropDef>> GoExecutor::getStepOutProps() {
storage::cpp2::PropDef pd;
pd.owner = storage::cpp2::PropOwner::EDGE;
pd.name = _DST;
pd.id.set_edge_type(std::abs(e));
pd.id.set_edge_type(e);
props.emplace_back(std::move(pd));
VLOG(3) << "Need edge props: " << e << ", _dst";
}
Expand Down Expand Up @@ -758,7 +758,28 @@ StatusOr<std::vector<storage::cpp2::PropDef>> GoExecutor::getStepOutProps() {
return Status::Error("the edge was not found '%s'", prop.first.c_str());
}
pd.id.set_edge_type(edgeType);
props.emplace_back(std::move(pd));
switch (direction_) {
case OverClause::Direction::kForward: {
props.emplace_back(std::move(pd));
break;
}
case OverClause::Direction::kBackward: {
edgeType = -edgeType;
pd.id.set_edge_type(edgeType);
props.emplace_back(std::move(pd));
break;
}
case OverClause::Direction::kBidirect: {
props.emplace_back(pd);
edgeType = -edgeType;
pd.id.set_edge_type(edgeType);
props.emplace_back(std::move(pd));
break;
}
default:
return Status::Error(
"Unknown direction: %ld", static_cast<int64_t>(direction_));
}
VLOG(3) << "Need edge props: " << prop.first << ", " << prop.second;
}
return props;
Expand Down Expand Up @@ -979,7 +1000,7 @@ bool GoExecutor::processFinalResult(RpcResponse &rpcResp, Callback cb) const {
VLOG(1) << "Total edata.edges size " << edata.edges.size()
<< ", for edge " << edgeType;
std::shared_ptr<ResultSchemaProvider> currEdgeSchema;
auto it = edgeSchema.find(std::abs(edgeType));
auto it = edgeSchema.find(edgeType);
if (it != edgeSchema.end()) {
currEdgeSchema = it->second;
}
Expand Down Expand Up @@ -1082,12 +1103,45 @@ bool GoExecutor::processFinalResult(RpcResponse &rpcResp, Callback cb) const {
"Get edge type for `%s' failed in getters.", edgeName.c_str());
}
if (std::abs(edgeType) != type) {
auto sit = edgeSchema.find(type);
if (sit == edgeSchema.end()) {
LOG(ERROR) << "Can't find schema for " << edgeName;
return Status::Error("get schema failed");
switch (direction_) {
case OverClause::Direction::kForward: {
auto sit = edgeSchema.find(type);
if (sit != edgeSchema.end()) {
return RowReader::getDefaultProp(sit->second.get(), prop);
}
break;
}
case OverClause::Direction::kBackward: {
type = -type;
auto sit = edgeSchema.find(type);
if (sit != edgeSchema.end()) {
return RowReader::getDefaultProp(sit->second.get(), prop);
}
break;
}
case OverClause::Direction::kBidirect: {
auto sit = edgeSchema.find(type);
if (sit != edgeSchema.end()) {
return RowReader::getDefaultProp(sit->second.get(), prop);
}

type = -type;
sit = edgeSchema.find(type);
if (sit != edgeSchema.end()) {
return RowReader::getDefaultProp(sit->second.get(), prop);
}
break;
}
default:
return Status::Error(
"Unknown direction: %ld",
static_cast<int64_t>(direction_));
}
return RowReader::getDefaultProp(sit->second.get(), prop);

std::string errMsg = folly::stringPrintf(
"Can't find shcema for %s when get default.", edgeName.c_str());
LOG(ERROR) << errMsg;
return Status::Error(errMsg);
}
if (prop == _SRC) {
return srcId;
Expand Down
2 changes: 1 addition & 1 deletion src/parser/Clauses.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ std::string OverClause::toString() const {

if (direction_ == OverClause::Direction::kBackward) {
buf += " REVERSELY";
} else if (direction_ == OverClause::Direction::kBiDirect) {
} else if (direction_ == OverClause::Direction::kBidirect) {
buf += " BIDIRECT";
}

Expand Down
2 changes: 1 addition & 1 deletion src/parser/Clauses.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ class OverClause final : public Clause {
enum class Direction : uint8_t {
kForward,
kBackward,
kBiDirect
kBidirect
};

OverClause(OverEdges *edges,
Expand Down
4 changes: 2 additions & 2 deletions src/parser/parser.yy
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,7 @@ over_clause
auto s = new std::string("*");
auto edge = new OverEdge(s, nullptr);
edges->addEdge(edge);
$$ = new OverClause(edges, OverClause::Direction::kBiDirect);
$$ = new OverClause(edges, OverClause::Direction::kBidirect);
}
| KW_OVER over_edges {
$$ = new OverClause($2);
Expand All @@ -693,7 +693,7 @@ over_clause
$$ = new OverClause($2, OverClause::Direction::kBackward);
}
| KW_OVER over_edges KW_BIDIRECT {
$$ = new OverClause($2, OverClause::Direction::kBiDirect);
$$ = new OverClause($2, OverClause::Direction::kBidirect);
}
;

Expand Down
37 changes: 10 additions & 27 deletions src/storage/query/QueryBaseProcessor.inl
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ cpp2::ErrorCode QueryBaseProcessor<REQ, RESP>::checkAndBuildContexts(const REQ&
int32_t index = std::accumulate(edgeContexts_.cbegin(), edgeContexts_.cend(), 0,
[](int ac, auto& ec) { return ac + ec.second.size(); });
std::unordered_map<TagID, int32_t> tagIndex;
std::unordered_map<EdgeType, std::vector<PropContext>> edgeProps;
for (auto& col : req.get_return_columns()) {
PropContext prop;
switch (col.owner) {
Expand Down Expand Up @@ -104,13 +103,13 @@ cpp2::ErrorCode QueryBaseProcessor<REQ, RESP>::checkAndBuildContexts(const REQ&
break;
}
case cpp2::PropOwner::EDGE: {
EdgeType edgeType = std::abs(col.id.get_edge_type());
auto edgeName = this->schemaMan_->toEdgeName(spaceId_, edgeType);
EdgeType edgeType = col.id.get_edge_type();
auto edgeName = this->schemaMan_->toEdgeName(spaceId_, std::abs(edgeType));
if (!edgeName.ok()) {
VLOG(3) << "Can't find spaceId " << spaceId_ << ", edgeType " << edgeType;
return cpp2::ErrorCode::E_EDGE_NOT_FOUND;
}
this->edgeMap_.emplace(edgeName.value(), edgeType);
this->edgeMap_.emplace(edgeName.value(), std::abs(edgeType));

auto it = kPropsInKey_.find(col.name);
if (it != kPropsInKey_.end()) {
Expand All @@ -123,7 +122,7 @@ cpp2::ErrorCode QueryBaseProcessor<REQ, RESP>::checkAndBuildContexts(const REQ&
}
} else {
auto schema = this->schemaMan_->getEdgeSchema(spaceId_,
edgeType);
std::abs(edgeType));
if (!schema) {
return cpp2::ErrorCode::E_EDGE_PROP_NOT_FOUND;
}
Expand All @@ -139,29 +138,17 @@ cpp2::ErrorCode QueryBaseProcessor<REQ, RESP>::checkAndBuildContexts(const REQ&
prop.retIndex_ = index++;
prop.prop_ = std::move(col);
prop.returned_ = true;
auto it2 = edgeProps.find(edgeType);
if (it2 == edgeProps.end()) {
auto it2 = edgeContexts_.find(edgeType);
if (it2 == edgeContexts_.end()) {
std::vector<PropContext> v{std::move(prop)};
edgeProps.emplace(edgeType, std::move(v));
edgeContexts_.emplace(edgeType, std::move(v));
break;
}
it2->second.emplace_back(std::move(prop));
break;
}
}
}
for (auto &eCtx : edgeContexts_) {
auto edgeType = std::abs(eCtx.first);
auto it = edgeProps.find(edgeType);
if (it == edgeProps.end()) {
LOG(ERROR) << "Edge type " << edgeType << "was given, but no props request.";
return cpp2::ErrorCode::E_EDGE_NOT_FOUND;
}
// There might exist default props in context,
// so insert the props.
eCtx.second.insert(eCtx.second.end(), it->second.begin(), it->second.end());
}

const auto& filterStr = req.get_filter();
if (!filterStr.empty()) {
StatusOr<std::unique_ptr<Expression>> expRet = Expression::decode(filterStr);
Expand Down Expand Up @@ -738,16 +725,12 @@ void QueryBaseProcessor<REQ, RESP>::buildTTLInfoAndRespSchema() {
VLOG(1) << "EdgeType " << ec.first << ", onlyStructure " << respEdge.columns.empty();
onlyStructures_.emplace(ec.first, respEdge.columns.empty());
if (!respEdge.columns.empty()) {
auto it = edgeSchema_.find(ec.first);
if (it == edgeSchema_.end()) {
auto it = edgeSchemaResp_.find(ec.first);
if (it == edgeSchemaResp_.end()) {
auto schemaProvider = std::make_shared<ResultSchemaProvider>(respEdge);
edgeSchema_.emplace(ec.first, std::move(schemaProvider));
VLOG(1) << "Fulfill edge for edge " << ec.first;
}
auto itResp = edgeSchemaResp_.find(std::abs(ec.first));
if (itResp == edgeSchemaResp_.end()) {
VLOG(1) << "Fulfill edge response for edge " << ec.first;
edgeSchemaResp_.emplace(std::abs(ec.first), std::move(respEdge));
edgeSchemaResp_.emplace(ec.first, std::move(respEdge));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/storage/test/QueryBoundTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ void checkResponse(cpp2::QueryResponse& resp,
folly::stringPrintf("tag_string_col_4"));

for (auto& ep : vp.edge_data) {
auto it2 = schema.find(std::abs(ep.type));
auto it2 = schema.find(ep.type);
DCHECK(it2 != schema.end()) << ep.type;
auto provider = it2->second;
int32_t rowNum = 0;
Expand Down

0 comments on commit 8ab536c

Please sign in to comment.