Skip to content

Commit

Permalink
minor: remove redundant prefix. (#3973)
Browse files Browse the repository at this point in the history
* minor: remove redundant prefix.

* minor: fix the pull_request_template.md

* fix fmt
  • Loading branch information
jackwener authored Oct 26, 2022
1 parent 648ed3f commit 00d23a2
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 34 deletions.
5 changes: 4 additions & 1 deletion .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,21 @@ We generally require a GitHub issue to be filed for all bug fixes and enhancemen

Closes #.

# Rationale for this change
# Rationale for this change

<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->

# What changes are included in this PR?

<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->

# Are there any user-facing changes?

<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
Expand Down
14 changes: 4 additions & 10 deletions datafusion/expr/src/logical_plan/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl<'a, 'b> IndentVisitor<'a, 'b> {
impl<'a, 'b> PlanVisitor for IndentVisitor<'a, 'b> {
type Error = fmt::Error;

fn pre_visit(&mut self, plan: &LogicalPlan) -> std::result::Result<bool, fmt::Error> {
fn pre_visit(&mut self, plan: &LogicalPlan) -> Result<bool, fmt::Error> {
if self.indent > 0 {
writeln!(self.f)?;
}
Expand All @@ -66,10 +66,7 @@ impl<'a, 'b> PlanVisitor for IndentVisitor<'a, 'b> {
Ok(true)
}

fn post_visit(
&mut self,
_plan: &LogicalPlan,
) -> std::result::Result<bool, fmt::Error> {
fn post_visit(&mut self, _plan: &LogicalPlan) -> Result<bool, fmt::Error> {
self.indent -= 1;
Ok(true)
}
Expand Down Expand Up @@ -190,7 +187,7 @@ impl<'a, 'b> GraphvizVisitor<'a, 'b> {
impl<'a, 'b> PlanVisitor for GraphvizVisitor<'a, 'b> {
type Error = fmt::Error;

fn pre_visit(&mut self, plan: &LogicalPlan) -> std::result::Result<bool, fmt::Error> {
fn pre_visit(&mut self, plan: &LogicalPlan) -> Result<bool, fmt::Error> {
let id = self.graphviz_builder.next_id();

// Create a new graph node for `plan` such as
Expand Down Expand Up @@ -226,10 +223,7 @@ impl<'a, 'b> PlanVisitor for GraphvizVisitor<'a, 'b> {
Ok(true)
}

fn post_visit(
&mut self,
_plan: &LogicalPlan,
) -> std::result::Result<bool, fmt::Error> {
fn post_visit(&mut self, _plan: &LogicalPlan) -> Result<bool, fmt::Error> {
// always be non-empty as pre_visit always pushes
// So it should always be Ok(true)
let res = self.parent_ids.pop();
Expand Down
42 changes: 19 additions & 23 deletions datafusion/expr/src/logical_plan/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,17 +378,13 @@ pub trait PlanVisitor {
/// visited. If Ok(true) is returned, the recursion continues. If
/// Err(..) or Ok(false) are returned, the recursion stops
/// immediately and the error, if any, is returned to `accept`
fn pre_visit(&mut self, plan: &LogicalPlan)
-> std::result::Result<bool, Self::Error>;
fn pre_visit(&mut self, plan: &LogicalPlan) -> Result<bool, Self::Error>;

/// Invoked on a logical plan after all of its child inputs have
/// been visited. The return value is handled the same as the
/// return value of `pre_visit`. The provided default implementation
/// returns `Ok(true)`.
fn post_visit(
&mut self,
_plan: &LogicalPlan,
) -> std::result::Result<bool, Self::Error> {
fn post_visit(&mut self, _plan: &LogicalPlan) -> Result<bool, Self::Error> {
Ok(true)
}
}
Expand All @@ -398,7 +394,7 @@ impl LogicalPlan {
/// all nodes were visited, and Ok(false) if any call to
/// `pre_visit` or `post_visit` returned Ok(false) and may have
/// cut short the recursion
pub fn accept<V>(&self, visitor: &mut V) -> std::result::Result<bool, V::Error>
pub fn accept<V>(&self, visitor: &mut V) -> Result<bool, V::Error>
where
V: PlanVisitor,
{
Expand Down Expand Up @@ -545,12 +541,12 @@ impl LogicalPlan {
/// assert_eq!("Filter: t1.id = Int32(5)\n TableScan: t1",
/// display_string);
/// ```
pub fn display_indent(&self) -> impl fmt::Display + '_ {
pub fn display_indent(&self) -> impl Display + '_ {
// Boilerplate structure to wrap LogicalPlan with something
// that that can be formatted
struct Wrapper<'a>(&'a LogicalPlan);
impl<'a> fmt::Display for Wrapper<'a> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
impl<'a> Display for Wrapper<'a> {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
let with_schema = false;
let mut visitor = IndentVisitor::new(f, with_schema);
match self.0.accept(&mut visitor) {
Expand Down Expand Up @@ -588,12 +584,12 @@ impl LogicalPlan {
/// \n TableScan: t1 [id:Int32]",
/// display_string);
/// ```
pub fn display_indent_schema(&self) -> impl fmt::Display + '_ {
pub fn display_indent_schema(&self) -> impl Display + '_ {
// Boilerplate structure to wrap LogicalPlan with something
// that that can be formatted
struct Wrapper<'a>(&'a LogicalPlan);
impl<'a> fmt::Display for Wrapper<'a> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
impl<'a> Display for Wrapper<'a> {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
let with_schema = true;
let mut visitor = IndentVisitor::new(f, with_schema);
match self.0.accept(&mut visitor) {
Expand Down Expand Up @@ -634,12 +630,12 @@ impl LogicalPlan {
/// dot -Tpdf < /tmp/example.dot > /tmp/example.pdf
/// ```
///
pub fn display_graphviz(&self) -> impl fmt::Display + '_ {
pub fn display_graphviz(&self) -> impl Display + '_ {
// Boilerplate structure to wrap LogicalPlan with something
// that that can be formatted
struct Wrapper<'a>(&'a LogicalPlan);
impl<'a> fmt::Display for Wrapper<'a> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
impl<'a> Display for Wrapper<'a> {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
writeln!(
f,
"// Begin DataFusion GraphViz Plan (see https://graphviz.org)"
Expand Down Expand Up @@ -686,12 +682,12 @@ impl LogicalPlan {
///
/// assert_eq!("TableScan: t1", display_string);
/// ```
pub fn display(&self) -> impl fmt::Display + '_ {
pub fn display(&self) -> impl Display + '_ {
// Boilerplate structure to wrap LogicalPlan with something
// that that can be formatted
struct Wrapper<'a>(&'a LogicalPlan);
impl<'a> fmt::Display for Wrapper<'a> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
impl<'a> Display for Wrapper<'a> {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
match self.0 {
LogicalPlan::EmptyRelation(_) => write!(f, "EmptyRelation"),
LogicalPlan::Values(Values { ref values, .. }) => {
Expand Down Expand Up @@ -957,8 +953,8 @@ impl LogicalPlan {
}
}

impl fmt::Debug for LogicalPlan {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
impl Debug for LogicalPlan {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
self.display_indent().fmt(f)
}
}
Expand Down Expand Up @@ -1554,8 +1550,8 @@ pub enum PlanType {
FinalPhysicalPlan,
}

impl fmt::Display for PlanType {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
impl Display for PlanType {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
match self {
PlanType::InitialLogicalPlan => write!(f, "initial_logical_plan"),
PlanType::OptimizedLogicalPlan { optimizer_name } => {
Expand Down

0 comments on commit 00d23a2

Please sign in to comment.