Move ProjectionPushdown into datafusion-physical-optimizer crate

[alamb]

Is your feature request related to a problem or challenge?

• Part of [EPIC] Extract remaining physical optimizer out of core #11502
• Related to Building project takes a *long* time (esp compilation time for datafusion core crate) #13814

Historically DataFusion was one (very) large crate datafusion, and as it grew bigger we extracted various functionality into separate crates. This leads to both faster compile times (as the crates can be compiled in parallel) as well easier to navigate code (as the crates force a cleaner dependency separation)

One project we have not yet completed is to extract the physical optimizer passes out #11502

• The original physical optimizers are here https://github.com/apache/datafusion/tree/main/datafusion/core/src/physical_optimizer
• The new crate is here https://github.com/apache/datafusion/tree/main/datafusion/physical-optimizer

Describe the solution you'd like

Extract the ProjectionPushdown from the datafusion core crate to the datafusion-physical-optimizer crate

Describe alternatives you've considered

Move the code

• From https://github.com/apache/datafusion/blob/main/datafusion/core/src/physical_optimizer/projection_pushdown.rs
• To https://github.com/apache/datafusion/tree/main/datafusion/physical-optimizer/src/projection_pushdown.rs

Notes that due to dependencies (e.g. on SessionContext or functions), you may have to move the tests into the core_integration tests here:

• https://github.com/apache/datafusion/tree/main/datafusion/core/tests/physical_optimizer

Additional context

Here are some example PRs

• chore: move SanityChecker into physical-optimizer crate #14083
• Move JoinSelection into datafusion-physical-optimizer crate (#14073) #14085
• Move Pruning into physical-optimizer crate #13485

I think this is a good first issue as it is mostly mechanical, has several good examples, and does not require deep internals knowledge

[logan-keede]

take

[logan-keede]

I think this would require making a separate crate for datasource since ProjectionPushdown has a dependency on datasource in core unless I am missing some major and glaring fact.

datafusion/datafusion/core/src/physical_optimizer/projection_pushdown.rs

Line 27 in 3a65be0

use crate::datasource::physical_plan::CsvExec;

If it is as I believe, I think that deserves a whole issue to itself.
On a side note, would it make sense to open an issue to update imports all over the core module(don't use it if it is not necessary)? I think that should make the whole refactoring a bit less daunting.

- use crate::error::Result;
+ use datafusion_common::error::Result;

[logan-keede]

@alamb Please let me know your thoughts on this and #14190, whenever possible,
Thanks,
Logan

[logan-keede]

cc @berkaysynnada

[berkaysynnada]

There are 2 options:
Plan A:

1. Decrease the functionality, and take back projections pushdown feature over CsvExec's.
2. Then, this rule has no dependency with datasource, and can be easily moved to PhysicalOptimizer crate.
3. Add an API to ExecutionPlan like handles_projection_pushdown, and implement it for all operators, including CsvExec. That will bring projection pushdown over CsvExec back.

Plan B:

1. We first add that handles_projection_pushdown API in ExecutionPlan's, and eliminate the dependency on datasource in projection_pushdown.rs
2. Then complete the migration

[buraksenn]

take

[buraksenn]

I think I understand @berkaysynnada proposal, currently working on moving swapping to ExecutionPlan trait and will open a PR soon to verify approach

[buraksenn]

Opened #14235 for this please review when you find time @berkaysynnada @alamb

[logan-keede]

take

@alamb
I thought this issue was assigned to me, is this an issue with the bot?

[buraksenn]

take

@alamb I thought this issue was assigned to me, is this an issue with the bot?

I did not saw any assignments to this sorry, that's why I took it. I can unassign. My PR actually does not move ProjectionPushdown so I think after that you can move it

EDIT: I don't see unassign me option not sure why

[logan-keede]

I did not saw any assignments to this sorry, that's why I took it. I can unassign. My PR actually does not move ProjectionPushdown so I think after that you can move it

no biggies, I had not really started working on it so that's fine, I might have needed some help either way.

maybe we can work on this together(if that is fine with you), so no need to unassign.

EDIT: I don't see unassign me option not sure why

I am not really sure if this behaviour of unassign/take is intended or not.

[alamb]

I did not saw any assignments to this sorry, that's why I took it. I can unassign. My PR actually does not move ProjectionPushdown so I think after that you can move it

No worries -- we tried to document the norms here: https://datafusion.apache.org/contributor-guide/index.html#open-contribution-and-assigning-tickets

Basically what you both are doing (good communication) in my mind is the key thing -- exactly who is assigned in github is just a sometimes convenience means for that communication, not any sort of requirement

[alamb]

• Done In Complete moving PhysicalOptimizer into datafusion-physical-optimizer #14300