Extract CreateMemoryTable, DropTable, CreateExternalTable in LogicalPlan as independent struct#1311
Conversation
liukun4515
force-pushed
the
refactor_1306
branch
from
656d373 to
0919bb3
Compare
liukun4515
force-pushed
the
refactor_1306
branch
from
0919bb3 to
daf2899
Compare
liukun4515
changed the title
|
@xudong963 PTAL |
|
Thanks @liukun4515, I'll take a look tomorrow! |
datafusion/src/logical_plan/mod.rs
Outdated
| mod extension; | ||
| mod operators; | ||
| pub mod plan; | ||
| mod plan; |
There was a problem hiding this comment.
I don't think this is backward compatible
There was a problem hiding this comment.
I agree with @jimexist 's implicit suggestion that we should leave the code as pub mod plan;
There was a problem hiding this comment.
@jimexist @alamb
I have doubts about this backward compatible.
Before this commit #1290, this is mod plan, and this commit is not in the release 6.0.0 datafusion.
The reason i did this is to keep code style with original usage:
pub use plan::{
JoinConstraint, JoinType, LogicalPlan, Partitioning, PlanType, PlanVisitor,
};
and make some struct public we wanted.
|
This PR now also has several logical conflicts with master :( |
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thanks @liukun4515 -- can you please address @jimexist 's comment and resolve the merge conflicts and I think this one will be ready to merge
datafusion/src/logical_plan/mod.rs
Outdated
| mod extension; | ||
| mod operators; | ||
| pub mod plan; | ||
| mod plan; |
There was a problem hiding this comment.
I agree with @jimexist 's implicit suggestion that we should leave the code as pub mod plan;
|
LGTM! |
|
@alamb please merge this pr |
|
Thanks @liukun4515 ! |
4 participants
Which issue does this PR close?
Closes #1306
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?