Skip to content

Conversation

@jackwener
Copy link
Member

Which issue does this PR close?

Closes #.

What changes are included in this PR?

Add the Optimizer struct.

Are there any user-facing changes?

None

Does this PR break compatibility with Ballista?

No

@jackwener jackwener force-pushed the refactor_optimzer branch 2 times, most recently from 7afb925 to 77049a2 Compare May 25, 2022 15:44
@jackwener jackwener force-pushed the refactor_optimzer branch from 77049a2 to 76f9703 Compare May 25, 2022 15:47
pub fn optimize<F>(
&self,
plan: &LogicalPlan,
execution_props: &mut ExecutionProps,
Copy link
Member

Choose a reason for hiding this comment

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

Not related to your changes but I am trying to get rid of the use of ExecutionProps here - #2614

Copy link
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

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

Thanks @jackwener. This looks like a good change to me and helps towards #2599 as well

@andygrove andygrove changed the title refactor: add optimzer struct MINOR: add optimizer struct May 25, 2022
pub session_id: String,
/// Responsible for optimizing a logical plan
pub optimizers: Vec<Arc<dyn OptimizerRule + Send + Sync>>,
pub optimizer: Optimizer,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a nice improvement 👍

@andygrove andygrove merged commit 894be67 into apache:master May 25, 2022
@jackwener jackwener deleted the refactor_optimzer branch May 26, 2022 02:00
gandronchik pushed a commit to cube-js/arrow-datafusion that referenced this pull request Aug 30, 2022
gandronchik pushed a commit to cube-js/arrow-datafusion that referenced this pull request Aug 31, 2022
gandronchik pushed a commit to cube-js/arrow-datafusion that referenced this pull request Sep 2, 2022
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.

3 participants