Skip to content

Commit

Permalink
clean
Browse files Browse the repository at this point in the history
  • Loading branch information
andyfengHKU committed Nov 28, 2022
1 parent 80a8e4f commit 2255244
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 35 deletions.
4 changes: 2 additions & 2 deletions src/binder/expression/existential_subquery_expression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ unordered_set<string> ExistentialSubqueryExpression::getDependentVariableNames()
// expressions from predicates and return clause. Plus nodeID expressions from query graph.
expression_vector ExistentialSubqueryExpression::getChildren() const {
expression_vector result;
for (auto& nodeIDExpression : queryGraphCollection->getNodeIDExpressions()) {
result.push_back(nodeIDExpression);
for (auto& node : queryGraphCollection->getQueryNodes()) {
result.push_back(node->getInternalIDProperty());
}
if (hasWhereExpression()) {
result.push_back(whereExpression);
Expand Down
5 changes: 5 additions & 0 deletions src/binder/query/bound_delete_clause.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ expression_vector BoundDeleteClause::getPropertiesToRead() const {
for (auto& deleteNode : deleteNodes) {
result.push_back(deleteNode->getPrimaryKeyExpression());
}
for (auto& deleteRel : deleteRels) {
if (deleteRel->hasInternalIDProperty()) {
result.push_back(deleteRel->getInternalIDProperty());
}
}
return result;
}

Expand Down
16 changes: 4 additions & 12 deletions src/binder/query/query_graph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,14 +187,6 @@ bool QueryGraph::isConnected(const QueryGraph& other) {
return false;
}

vector<shared_ptr<Expression>> QueryGraph::getNodeIDExpressions() const {
vector<shared_ptr<Expression>> result;
for (auto& queryNode : queryNodes) {
result.push_back(queryNode->getInternalIDProperty());
}
return result;
}

void QueryGraphCollection::addAndMergeQueryGraphIfConnected(
unique_ptr<QueryGraph> queryGraphToAdd) {
bool isMerged = false;
Expand All @@ -209,11 +201,11 @@ void QueryGraphCollection::addAndMergeQueryGraphIfConnected(
}
}

expression_vector QueryGraphCollection::getNodeIDExpressions() const {
expression_vector result;
vector<shared_ptr<NodeExpression>> QueryGraphCollection::getQueryNodes() const {
vector<shared_ptr<NodeExpression>> result;
for (auto& queryGraph : queryGraphs) {
for (auto& nodeID : queryGraph->getNodeIDExpressions()) {
result.push_back(nodeID);
for (auto& node : queryGraph->getQueryNodes()) {
result.push_back(node);
}
}
return result;
Expand Down
6 changes: 2 additions & 4 deletions src/include/binder/query/reading_clause/query_graph.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ class QueryGraph {
inline bool containsQueryNode(const string& queryNodeName) const {
return queryNodeNameToPosMap.contains(queryNodeName);
}
inline vector<shared_ptr<NodeExpression>> getQueryNodes() const { return queryNodes; }
inline shared_ptr<NodeExpression> getQueryNode(const string& queryNodeName) const {
return queryNodes[getQueryNodePos(queryNodeName)];
}
Expand Down Expand Up @@ -130,8 +131,6 @@ class QueryGraph {

void merge(const QueryGraph& other);

vector<shared_ptr<Expression>> getNodeIDExpressions() const;

inline unique_ptr<QueryGraph> copy() const { return make_unique<QueryGraph>(*this); }

private:
Expand All @@ -151,8 +150,7 @@ class QueryGraphCollection {
inline uint32_t getNumQueryGraphs() const { return queryGraphs.size(); }
inline QueryGraph* getQueryGraph(uint32_t idx) const { return queryGraphs[idx].get(); }

// TODO: rename to get query node
expression_vector getNodeIDExpressions() const;
vector<shared_ptr<NodeExpression>> getQueryNodes() const;
vector<shared_ptr<RelExpression>> getQueryRels() const;

unique_ptr<QueryGraphCollection> copy() const;
Expand Down
11 changes: 7 additions & 4 deletions src/include/processor/operator/update/delete.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,16 @@ struct DeleteRelInfo {
table_id_t srcNodeTableID;
DataPos dstNodePos;
table_id_t dstNodeTableID;
DataPos relIDPos;

DeleteRelInfo(RelTable* table, const DataPos& srcNodePos, table_id_t srcNodeTableID,
const DataPos& dstNodePos, table_id_t dstNodeTableID)
const DataPos& dstNodePos, table_id_t dstNodeTableID, const DataPos& relIDPos)
: table{table}, srcNodePos{srcNodePos}, srcNodeTableID{srcNodeTableID},
dstNodePos{dstNodePos}, dstNodeTableID{dstNodeTableID} {}
dstNodePos{dstNodePos}, dstNodeTableID{dstNodeTableID}, relIDPos{relIDPos} {}

inline unique_ptr<DeleteRelInfo> clone() {
return make_unique<DeleteRelInfo>(
table, srcNodePos, srcNodeTableID, dstNodePos, dstNodeTableID);
table, srcNodePos, srcNodeTableID, dstNodePos, dstNodeTableID, relIDPos);
}
};

Expand Down Expand Up @@ -95,7 +96,9 @@ class DeleteRel : public PhysicalOperator {
private:
RelsStatistics& relsStatistics;
vector<unique_ptr<DeleteRelInfo>> deleteRelInfos;
vector<pair<ValueVector*, ValueVector*>> srcDstNodeIDVectorPairs;
vector<ValueVector*> srcNodeVectors;
vector<ValueVector*> dstNodeVectors;
vector<ValueVector*> relIDVectors;
};

} // namespace processor
Expand Down
6 changes: 3 additions & 3 deletions src/planner/query_planner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,9 @@ static expression_vector getCorrelatedExpressions(
result.push_back(expression);
}
}
for (auto& nodeIDExpression : collection.getNodeIDExpressions()) {
if (outerSchema->isExpressionInScope(*nodeIDExpression)) {
result.push_back(nodeIDExpression);
for (auto& node : collection.getQueryNodes()) {
if (outerSchema->isExpressionInScope(*node->getInternalIDProperty())) {
result.push_back(node->getInternalIDProperty());
}
}
return result;
Expand Down
3 changes: 2 additions & 1 deletion src/processor/mapper/map_delete.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ unique_ptr<PhysicalOperator> PlanMapper::mapLogicalDeleteRelToPhysical(
auto srcNodeTableID = rel->getSrcNode()->getTableID();
auto dstNodePos = mapperContext.getDataPos(rel->getDstNode()->getIDProperty());
auto dstNodeTableID = rel->getDstNode()->getTableID();
auto relIDPos = mapperContext.getDataPos(rel->getInternalIDProperty()->getUniqueName());
createRelInfos.push_back(make_unique<DeleteRelInfo>(
table, srcNodePos, srcNodeTableID, dstNodePos, dstNodeTableID));
table, srcNodePos, srcNodeTableID, dstNodePos, dstNodeTableID, relIDPos));
}
return make_unique<DeleteRel>(relStore.getRelsStatistics(), std::move(createRelInfos),
std::move(prevOperator), getOperatorID(), logicalOperator->getExpressionsForPrinting());
Expand Down
13 changes: 10 additions & 3 deletions src/processor/operator/update/delete.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,15 @@ shared_ptr<ResultSet> DeleteRel::init(ExecutionContext* context) {
auto srcNodePos = deleteRelInfo->srcNodePos;
auto srcNodeIDVector =
resultSet->dataChunks[srcNodePos.dataChunkPos]->valueVectors[srcNodePos.valueVectorPos];
srcNodeVectors.push_back(srcNodeIDVector.get());
auto dstNodePos = deleteRelInfo->dstNodePos;
auto dstNodeIDVector =
resultSet->dataChunks[dstNodePos.dataChunkPos]->valueVectors[dstNodePos.valueVectorPos];
srcDstNodeIDVectorPairs.emplace_back(srcNodeIDVector.get(), dstNodeIDVector.get());
dstNodeVectors.push_back(dstNodeIDVector.get());
auto relIDPos = deleteRelInfo->relIDPos;
auto relIDVector =
resultSet->dataChunks[relIDPos.dataChunkPos]->valueVectors[relIDPos.valueVectorPos];
relIDVectors.push_back(relIDVector.get());
}
return resultSet;
}
Expand All @@ -49,11 +54,13 @@ bool DeleteRel::getNextTuplesInternal() {
}
for (auto i = 0u; i < deleteRelInfos.size(); ++i) {
auto createRelInfo = deleteRelInfos[i].get();
auto [srcNodeIDVector, dstNodeIDVector] = srcDstNodeIDVectorPairs[i];
auto srcNodeVector = srcNodeVectors[i];
auto dstNodeVector = dstNodeVectors[i];
auto relIDVector = relIDVectors[i];
// TODO(Ziyi): you need to update rel statistics here. Though I don't think this is best
// design, statistics update should be hidden inside deleteRel(). See how node create/delete
// works. You can discuss this with Semih and if you decide to change, change createRel too.
createRelInfo->table->deleteRel(srcNodeIDVector, dstNodeIDVector);
createRelInfo->table->deleteRel(srcNodeVector, dstNodeVector);
}
return true;
}
Expand Down
18 changes: 12 additions & 6 deletions test/include/mock_catalog/mock_catalog.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ class TinySnbCatalogContent : public MockCatalogContent {
.WillByDefault(Return(true));
ON_CALL(*this, containRelProperty(KNOWS_TABLE_ID, KNOWSDATE_PROPERTY_KEY_STR))
.WillByDefault(Return(true));
ON_CALL(*this, containRelProperty(KNOWS_TABLE_ID, KNOWSDATE_PROPERTY_KEY_STR))
.WillByDefault(Return(true));
ON_CALL(*this, containRelProperty(1, _)).WillByDefault(Return(false));
}

Expand Down Expand Up @@ -219,12 +221,16 @@ class TinySnbCatalogContent : public MockCatalogContent {
}

void setRelTableSchemas() {
knowsTableSchema =
make_unique<RelTableSchema>("knows", KNOWS_TABLE_ID, MANY_MANY, vector<Property>{},
vector<pair<table_id_t, table_id_t>>{{PERSON_TABLE_ID, PERSON_TABLE_ID}});
workAtTableSchema =
make_unique<RelTableSchema>("workAt", WORKAT_TABLE_ID, MANY_ONE, vector<Property>{},
vector<pair<table_id_t, table_id_t>>{{PERSON_TABLE_ID, ORGANISATION_TABLE_ID}});
auto knowsID = Property::constructRelProperty(
PropertyNameDataType(INTERNAL_ID_SUFFIX, INT64), 0, KNOWS_TABLE_ID);
knowsTableSchema = make_unique<RelTableSchema>("knows", KNOWS_TABLE_ID, MANY_MANY,
vector<Property>{knowsID},
vector<pair<table_id_t, table_id_t>>{{PERSON_TABLE_ID, PERSON_TABLE_ID}});
auto workAtID = Property::constructRelProperty(
PropertyNameDataType(INTERNAL_ID_SUFFIX, INT64), 0, WORKAT_TABLE_ID);
workAtTableSchema = make_unique<RelTableSchema>("workAt", WORKAT_TABLE_ID, MANY_ONE,
vector<Property>{workAtID},
vector<pair<table_id_t, table_id_t>>{{PERSON_TABLE_ID, ORGANISATION_TABLE_ID}});
}

vector<unordered_set<table_id_t>> srcNodeIDToRelIDs, dstNodeIDToRelIDs;
Expand Down

0 comments on commit 2255244

Please sign in to comment.