Skip to content
This repository was archived by the owner on Oct 10, 2025. It is now read-only.

Conversation

ted-wq-x
Copy link
Contributor

@ted-wq-x ted-wq-x commented Mar 15, 2025

Description

  • Remove unused scanNodeTable properties,while a filter is pushed into scanNodeTable
  • Remove unnecessary projection upon scanNodeTable

Contributor agreement

#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>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change <> to "" for consistency

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) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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 (scanNodeTable.getProperties().empty()) {
return false;
}
bool hasPropertiesInUse = false;
Copy link
Contributor

Choose a reason for hiding this comment

The 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 clearProperty. Scan operator can scan a subset of its current properties.

}
scanState = std::make_unique<NodeTableScanState>(
resultSet->getValueVector(info.nodeIDPos).get(), outVectors, outVectors[0]->state);
if (outVectors.empty()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ray6080 should review this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants