Minor: Move PlanType, StringifiedPlan and ToStringifiedPlan datafusion_common
#6571
Conversation
| @@ -21,7 +21,7 @@ | |||
|
|
|||
| use std::fmt; | |||
|
|
|||
| use crate::logical_expr::{StringifiedPlan, ToStringifiedPlan}; | |||
| use datafusion_common::display::{StringifiedPlan, ToStringifiedPlan}; | |||
There was a problem hiding this comment.
the whole point of this PR is to remove the logical_expr from the use statements in physical_plan
alamb
changed the title
datafusion_commonPlanType, StringifiedPlan and ToStringifiedPlan datafusion_common
alamb
changed the title
PlanType, StringifiedPlan and ToStringifiedPlan datafusion_common PlanType, StringifiedPlan and ToStringifiedPlan datafusion_common
| // backwards compatibility | ||
| pub use datafusion_common::display::{PlanType, StringifiedPlan, ToStringifiedPlan}; |
There was a problem hiding this comment.
For such things, I'm wondering as no deprecated message, it is possibly we can remove this kind of compatibility later?
There was a problem hiding this comment.
I tried to add a deprecated message (that could point people to use the new location) but it didn't seem to work.
I think we can remove the pub use whenever we want, it will simply mean users of the crate will have to update their pub uses (and will technically be a breaking API change)
I guess I am hoping that over time we can remove some of the old pub use but we don't really have a structured plan for doing so. 🤔
There was a problem hiding this comment.
Yea, for such re-exported things, seems it is impossible to add deprecated message. So the only way is just to remove this kind of old APIs after some time (e.g., few releases?).
Co-authored-by: Liang-Chi Hsieh <viirya@gmail.com>
…tafusion_common` (apache#6571) * Move DisplayablePlan to `datafusion_common` * Update uses * Update datafusion/common/src/display.rs Co-authored-by: Liang-Chi Hsieh <viirya@gmail.com> --------- Co-authored-by: Liang-Chi Hsieh <viirya@gmail.com>
Which issue does this PR close?
Part of #1754 (see #1754 (comment))
Rationale for this change
I am trying to extract
physical_planinto its own crate to improve modularity and compile timesDisplayablePlanand related structures to are used in physical_plan and thus they need to be out of datafusion core to pull out the physical plan:What changes are included in this PR?
PlanType,StringifiedPlanandToStringifiedPlandatafusion_commonAre these changes tested?
Existing tests
Are there any user-facing changes?
No, I left a backwards compatible
pub use