-
Notifications
You must be signed in to change notification settings - Fork 294
Remove unused LogicalScanNodeTable's properties #5055
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |
| #include "planner/operator/persistent/logical_insert.h" | ||
| #include "planner/operator/persistent/logical_merge.h" | ||
| #include "planner/operator/persistent/logical_set.h" | ||
| #include <planner/operator/scan/logical_scan_node_table.h> | ||
|
|
||
| using namespace kuzu::common; | ||
| using namespace kuzu::planner; | ||
|
|
@@ -266,6 +267,28 @@ void ProjectionPushDownOptimizer::visitTableFunctionCall(LogicalOperator* op) { | |
| tableFunctionCall.setColumnSkips(std::move(columnSkips)); | ||
| } | ||
|
|
||
| void ProjectionPushDownOptimizer::visitScanNodeTable(LogicalOperator* op) { | ||
| clearScanNodeTableProperties(op); | ||
| } | ||
|
|
||
| bool ProjectionPushDownOptimizer::clearScanNodeTableProperties(planner::LogicalOperator* op) const { | ||
| auto& scanNodeTable = op->cast<LogicalScanNodeTable>(); | ||
| if (scanNodeTable.getProperties().empty()) { | ||
| return false; | ||
| } | ||
| bool hasPropertiesInUse = false; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. First I wanna understand on which query this optimization will take effect so that we can add test & benchmark. Second, I think we can do better than |
||
| for (auto expression : scanNodeTable.getProperties()) { | ||
| if (propertiesInUse.contains(expression)) { | ||
| hasPropertiesInUse = true; | ||
| break; | ||
| } | ||
| } | ||
| if (!hasPropertiesInUse) { | ||
| scanNodeTable.clearProperty(); | ||
| } | ||
| return !hasPropertiesInUse; | ||
| } | ||
|
|
||
| void ProjectionPushDownOptimizer::visitSetInfo(const binder::BoundSetPropertyInfo& info) { | ||
| switch (info.tableType) { | ||
| case TableType::NODE: { | ||
|
|
@@ -361,8 +384,14 @@ void ProjectionPushDownOptimizer::preAppendProjection(LogicalOperator* op, idx_t | |
| // We don't have a way to handle | ||
| return; | ||
| } | ||
| auto projection = | ||
| std::make_shared<LogicalProjection>(std::move(expressions), op->getChild(childIdx)); | ||
| auto child = op->getChild(childIdx); | ||
| // reset scan node table properties | ||
| if (child->getOperatorType() == LogicalOperatorType::SCAN_NODE_TABLE) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would add a new optimizer that remove any unnecessary projection instead of coupling the logic inside projection push down optimizer. |
||
| if (clearScanNodeTableProperties(child.get())) { | ||
| return; | ||
| } | ||
| } | ||
| auto projection = std::make_shared<LogicalProjection>(std::move(expressions), child); | ||
| projection->computeFlatSchema(); | ||
| op->setChild(childIdx, std::move(projection)); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,9 +16,10 @@ std::string PrimaryKeyScanPrintInfo::toString() const { | |
| result += ",Alias: "; | ||
| result += alias; | ||
| } | ||
| result += ", Expressions: "; | ||
| result += binder::ExpressionUtil::toString(expressions); | ||
|
|
||
| if (!expressions.empty()) { | ||
| result += ", Expressions: "; | ||
| result += binder::ExpressionUtil::toString(expressions); | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
|
|
@@ -36,8 +37,18 @@ void PrimaryKeyScanNodeTable::initLocalStateInternal(ResultSet* resultSet, | |
| for (auto& pos : info.outVectorsPos) { | ||
| outVectors.push_back(resultSet->getValueVector(pos).get()); | ||
| } | ||
| scanState = std::make_unique<NodeTableScanState>( | ||
| resultSet->getValueVector(info.nodeIDPos).get(), outVectors, outVectors[0]->state); | ||
| if (outVectors.empty()) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ray6080 should review this. |
||
|
|
||
| scanState = | ||
| std::make_unique<NodeTableScanState>(resultSet->getValueVector(info.nodeIDPos).get(), | ||
| outVectors, DataChunkState::getSingleValueDataChunkState()); | ||
|
|
||
| ; | ||
| } else { | ||
|
|
||
| scanState = std::make_unique<NodeTableScanState>( | ||
| resultSet->getValueVector(info.nodeIDPos).get(), outVectors, outVectors[0]->state); | ||
| } | ||
| indexEvaluator->init(*resultSet, context->clientContext); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change
<>to""for consistency