Skip to content

Conversation

andygrove
Copy link
Member

@andygrove andygrove commented Sep 7, 2019

This PR implements the physical execution plan for the selection operator (the WHERE clause in a SQL query).

In order to have working tests, I also had to implement some subset of expressions (column reference, literal value, comparison expressions, and CAST). However, the goal of this PR is not to add complete support for all expressions but to implement the Selection operator. I will create separate JIRA/PRs for adding support for other expressions and data types in the physical query plan.

@andygrove andygrove self-assigned this Sep 7, 2019
@andygrove andygrove changed the title [Rust] [DataFusion] ARROW-6089: Implement physical plan for "selection" operator ARROW-6089: [Rust] [DataFusion] Implement physical plan for "selection" operator Sep 8, 2019
@andygrove andygrove changed the title ARROW-6089: [Rust] [DataFusion] Implement physical plan for "selection" operator ARROW-6089: [Rust] [DataFusion] Implement physical plan for "selection" operator [WIP] Sep 8, 2019
@andygrove andygrove force-pushed the ARROW-6089 branch 2 times, most recently from 48875e4 to 831e695 Compare September 12, 2019 14:24
@andygrove andygrove marked this pull request as ready for review September 12, 2019 14:25
@andygrove andygrove changed the title ARROW-6089: [Rust] [DataFusion] Implement physical plan for "selection" operator [WIP] ARROW-6089: [Rust] [DataFusion] Implement physical plan for "selection" operator Sep 13, 2019
@andygrove
Copy link
Member Author

@paddyhoran @sunchao @nevi-me This PR is now ready for review. PTAL when you can.

My stretch goal is to implement the aggregate operator as well in time for the 0.15 release if I can finish it this weekend. This would mean that people can start experimenting with parallel queries and then for the next release (1.0?) we can drop the code that directly executes the logical plan and change it to create the physical plan and execute that instead.

@andygrove
Copy link
Member Author

@paddyhoran @sunchao @nevi-me This is my final PR that I'm hoping can make it into 0.15. I could break this out into smaller PRs by having separate PRs for the new expression types (literal, cast, binary) if that helps.

@andygrove
Copy link
Member Author

It's probably too late to try and get this reviewed for 0.15.0 so I have started breaking this down into smaller PRs that are easier to review.

@paddyhoran
Copy link
Contributor

@andygrove we might get it in. I had started reviewing but am struggling to find time now. Breaking it into smaller PR's definitely helps.

@andygrove
Copy link
Member Author

OK thanks @paddyhoran

I created the following PRs for the expressions:

#5474
#5477
#5478

These PRs have more extensive tests too than this current PR so it was good to pull them out.

I'll need to rebase of course once those are merged.

@paddyhoran
Copy link
Contributor

@andygrove can you rebase now please?

@andygrove
Copy link
Member Author

@paddyhoran done!

Copy link
Contributor

@paddyhoran paddyhoran left a comment

Choose a reason for hiding this comment

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

LGTM pending CI

@andygrove
Copy link
Member Author

@paddyhoran Thanks so much for reviewing this late flood of PRs. I really appreciate it!

@paddyhoran
Copy link
Contributor

Great job pushing the project forward. The progress you are making is really exciting!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants