Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(optimizer): column pruning framework #420

Merged
merged 3 commits into from
Feb 7, 2022

Conversation

st1page
Copy link
Collaborator

@st1page st1page commented Feb 7, 2022

add prune_col method on PlanNode, every logical node should implement fn prune_col(&self, required_cols: BitSet) -> PlanRef, which will optimize itself and output only the required columns. Now there is a default implementation which is insert a LogialProjection on the top of the Node.

ref #388

Signed-off-by: st1page <1245835950@qq.com>
Signed-off-by: st1page <1245835950@qq.com>
@st1page st1page requested a review from skyzh February 7, 2022 08:05
Copy link
Member

@skyzh skyzh left a comment

Choose a reason for hiding this comment

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

Rest LGTM

@@ -42,6 +44,8 @@ impl Optimizer {
}
let hep_optimizer = HeuristicOptimizer { rules };
plan = hep_optimizer.optimize(plan);
let out_types_num = plan.out_types().len();
plan = plan.prune_col(BitSet::from_iter(0..out_types_num));
Copy link
Member

Choose a reason for hiding this comment

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

I think we already have bitvec, and there's no need to import another bit-vec-like crate...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here we might need a collection which is more like a "Set", Maybe we will change it to fixedbitset

})
})
.collect();
LogicalProjection::new(exprs, self.clone_as_plan_ref()).into_plan_ref()
Copy link
Member

Choose a reason for hiding this comment

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

Simply doing projection won't save I/Os. Will we have plan rules to merge LogicalProjection with TableScan, and merge LogicalProjection with LogicalProjection?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here we simply doing projection to maintain the correct behavior of the fn prune_col. in future, every PlanNode should implement its own prune_col method, which can call its inputs' prune_col recursively. in the end, when call the prune_col of the TableScan Node, it will remove the columns not required.

@st1page st1page enabled auto-merge (squash) February 7, 2022 08:56
@st1page st1page disabled auto-merge February 7, 2022 08:56
@st1page st1page enabled auto-merge (squash) February 7, 2022 09:00
@st1page st1page merged commit 9daff3d into main Feb 7, 2022
@st1page st1page deleted the sts/column_pruning_framework branch February 7, 2022 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants