Skip to content

Commit

Permalink
Improve SanityChecker error message
Browse files Browse the repository at this point in the history
  • Loading branch information
alamb committed Sep 24, 2024
1 parent 380d843 commit 53f2479
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 8 deletions.
7 changes: 4 additions & 3 deletions datafusion/core/src/physical_optimizer/sanity_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use datafusion_physical_expr::intervals::utils::{check_support, is_datatype_supp
use datafusion_physical_plan::joins::SymmetricHashJoinExec;
use datafusion_physical_plan::{get_plan_string, ExecutionPlanProperties};

use datafusion_physical_expr_common::sort_expr::format_physical_sort_requirement_list;
use datafusion_physical_optimizer::PhysicalOptimizerRule;
use itertools::izip;

Expand Down Expand Up @@ -130,9 +131,9 @@ pub fn check_plan_sanity(
if !child_eq_props.ordering_satisfy_requirement(sort_req) {
let plan_str = get_plan_string(&plan);
return plan_err!(
"Plan: {:?} does not satisfy order requirements: {:?}. Child-{} order: {:?}",
"Plan: {:?} does not satisfy order requirements: {}. Child-{} order: {}",
plan_str,
sort_req,
format_physical_sort_requirement_list(sort_req),
idx,
child_eq_props.oeq_class
);
Expand All @@ -145,7 +146,7 @@ pub fn check_plan_sanity(
{
let plan_str = get_plan_string(&plan);
return plan_err!(
"Plan: {:?} does not satisfy distribution requirements: {:?}. Child-{} output partitioning: {:?}",
"Plan: {:?} does not satisfy distribution requirements: {}. Child-{} output partitioning: {}",
plan_str,
dist_req,
idx,
Expand Down
28 changes: 26 additions & 2 deletions datafusion/physical-expr-common/src/sort_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

//! Sort expressions

use std::fmt::Display;
use std::fmt::{Display, Formatter};
use std::hash::{Hash, Hasher};
use std::ops::Deref;
use std::sync::Arc;
Expand Down Expand Up @@ -252,13 +252,37 @@ impl PartialEq for PhysicalSortRequirement {
}
}

impl std::fmt::Display for PhysicalSortRequirement {
impl Display for PhysicalSortRequirement {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
let opts_string = self.options.as_ref().map_or("NA", to_str);
write!(f, "{} {}", self.expr, opts_string)
}
}

/// Writes a list of [`PhysicalSortRequirement`]s to a `std::fmt::Formatter`.
///
/// Example output: `[a + 1, b]`
pub fn format_physical_sort_requirement_list(
exprs: &[PhysicalSortRequirement],
) -> impl Display + '_ {
struct DisplayWrapper<'a>(&'a [PhysicalSortRequirement]);
impl<'a> Display for DisplayWrapper<'a> {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
let mut iter = self.0.iter();
write!(f, "[")?;
if let Some(expr) = iter.next() {
write!(f, "{}", expr)?;
}
for expr in iter {
write!(f, ", {}", expr)?;
}
write!(f, "]")?;
Ok(())
}
}
DisplayWrapper(exprs)
}

impl PhysicalSortRequirement {
/// Creates a new requirement.
///
Expand Down
19 changes: 16 additions & 3 deletions datafusion/physical-expr/src/partitioning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@

//! [`Partitioning`] and [`Distribution`] for `ExecutionPlans`

use std::fmt;
use std::sync::Arc;

use crate::{
equivalence::ProjectionMapping, expressions::UnKnownColumn, physical_exprs_equal,
EquivalenceProperties, PhysicalExpr,
};
use datafusion_physical_expr_common::physical_expr::format_physical_expr_list;
use std::fmt;
use std::fmt::Display;
use std::sync::Arc;

/// Output partitioning supported by [`ExecutionPlan`]s.
///
Expand Down Expand Up @@ -264,6 +265,18 @@ impl Distribution {
}
}

impl Display for Distribution {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Distribution::UnspecifiedDistribution => write!(f, "Unspecified"),
Distribution::SinglePartition => write!(f, "SinglePartition"),
Distribution::HashPartitioned(exprs) => {
write!(f, "HashPartitioned[{}])", format_physical_expr_list(exprs))
}
}
}
}

#[cfg(test)]
mod tests {

Expand Down

0 comments on commit 53f2479

Please sign in to comment.