Skip to content

Commit e8520ab

Browse files
Lordwormsalamb
andauthored
fix bugs explain with non-correlated query (apache#13210)
* fix bugs explain with non-correlated query * Use explicit enum for physical errors * fix comments / fmt * strip_backtrace to passs ci --------- Co-authored-by: Andrew Lamb <[email protected]>
1 parent e43466a commit e8520ab

File tree

14 files changed

+79
-15
lines changed

14 files changed

+79
-15
lines changed

datafusion/common/src/display/mod.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ pub enum PlanType {
6262
FinalPhysicalPlanWithStats,
6363
/// The final with schema, fully optimized physical plan which would be executed
6464
FinalPhysicalPlanWithSchema,
65+
/// An error creating the physical plan
66+
PhysicalPlanError,
6567
}
6668

6769
impl Display for PlanType {
@@ -91,6 +93,7 @@ impl Display for PlanType {
9193
PlanType::FinalPhysicalPlanWithSchema => {
9294
write!(f, "physical_plan_with_schema")
9395
}
96+
PlanType::PhysicalPlanError => write!(f, "physical_plan_error"),
9497
}
9598
}
9699
}
@@ -118,7 +121,9 @@ impl StringifiedPlan {
118121
/// `verbose_mode = true` will display all available plans
119122
pub fn should_display(&self, verbose_mode: bool) -> bool {
120123
match self.plan_type {
121-
PlanType::FinalLogicalPlan | PlanType::FinalPhysicalPlan => true,
124+
PlanType::FinalLogicalPlan
125+
| PlanType::FinalPhysicalPlan
126+
| PlanType::PhysicalPlanError => true,
122127
_ => verbose_mode,
123128
}
124129
}

datafusion/core/src/datasource/listing/table.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -880,18 +880,18 @@ impl TableProvider for ListingTable {
880880
None => {} // no ordering required
881881
};
882882

883-
let filters = conjunction(filters.to_vec())
884-
.map(|expr| -> Result<_> {
885-
// NOTE: Use the table schema (NOT file schema) here because `expr` may contain references to partition columns.
883+
let filters = match conjunction(filters.to_vec()) {
884+
Some(expr) => {
886885
let table_df_schema = self.table_schema.as_ref().clone().to_dfschema()?;
887886
let filters = create_physical_expr(
888887
&expr,
889888
&table_df_schema,
890889
state.execution_props(),
891890
)?;
892-
Ok(Some(filters))
893-
})
894-
.unwrap_or(Ok(None))?;
891+
Some(filters)
892+
}
893+
None => None,
894+
};
895895

896896
let Some(object_store_url) =
897897
self.table_paths.first().map(ListingTableUrl::object_store)

datafusion/core/src/physical_planner.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1797,8 +1797,12 @@ impl DefaultPhysicalPlanner {
17971797
Err(e) => return Err(e),
17981798
}
17991799
}
1800-
Err(e) => stringified_plans
1801-
.push(StringifiedPlan::new(InitialPhysicalPlan, e.to_string())),
1800+
Err(err) => {
1801+
stringified_plans.push(StringifiedPlan::new(
1802+
PhysicalPlanError,
1803+
err.strip_backtrace(),
1804+
));
1805+
}
18021806
}
18031807
}
18041808

datafusion/physical-plan/src/explain.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,6 @@ impl ExecutionPlan for ExplainExec {
132132
if 0 != partition {
133133
return internal_err!("ExplainExec invalid partition {partition}");
134134
}
135-
136135
let mut type_builder =
137136
StringBuilder::with_capacity(self.stringified_plans.len(), 1024);
138137
let mut plan_builder =

datafusion/proto/proto/datafusion.proto

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,7 @@ message PlanType {
655655
datafusion_common.EmptyMessage FinalPhysicalPlan = 6;
656656
datafusion_common.EmptyMessage FinalPhysicalPlanWithStats = 10;
657657
datafusion_common.EmptyMessage FinalPhysicalPlanWithSchema = 12;
658+
datafusion_common.EmptyMessage PhysicalPlanError = 13;
658659
}
659660
}
660661

datafusion/proto/src/generated/pbjson.rs

Lines changed: 13 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

datafusion/proto/src/generated/prost.rs

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

datafusion/proto/src/logical_plan/from_proto.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ use crate::protobuf::{
4444
AnalyzedLogicalPlan, FinalAnalyzedLogicalPlan, FinalLogicalPlan,
4545
FinalPhysicalPlan, FinalPhysicalPlanWithStats, InitialLogicalPlan,
4646
InitialPhysicalPlan, InitialPhysicalPlanWithStats, OptimizedLogicalPlan,
47-
OptimizedPhysicalPlan,
47+
OptimizedPhysicalPlan, PhysicalPlanError,
4848
},
4949
AnalyzedLogicalPlanType, CubeNode, GroupingSetNode, OptimizedLogicalPlanType,
5050
OptimizedPhysicalPlanType, PlaceholderNode, RollupNode,
@@ -141,6 +141,7 @@ impl From<&protobuf::StringifiedPlan> for StringifiedPlan {
141141
FinalPhysicalPlan(_) => PlanType::FinalPhysicalPlan,
142142
FinalPhysicalPlanWithStats(_) => PlanType::FinalPhysicalPlanWithStats,
143143
FinalPhysicalPlanWithSchema(_) => PlanType::FinalPhysicalPlanWithSchema,
144+
PhysicalPlanError(_) => PlanType::PhysicalPlanError,
144145
},
145146
plan: Arc::new(stringified_plan.plan.clone()),
146147
}

datafusion/proto/src/logical_plan/to_proto.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ use crate::protobuf::{
3838
FinalPhysicalPlan, FinalPhysicalPlanWithSchema, FinalPhysicalPlanWithStats,
3939
InitialLogicalPlan, InitialPhysicalPlan, InitialPhysicalPlanWithSchema,
4040
InitialPhysicalPlanWithStats, OptimizedLogicalPlan, OptimizedPhysicalPlan,
41+
PhysicalPlanError,
4142
},
4243
AnalyzedLogicalPlanType, CubeNode, EmptyMessage, GroupingSetNode, LogicalExprList,
4344
OptimizedLogicalPlanType, OptimizedPhysicalPlanType, PlaceholderNode, RollupNode,
@@ -115,6 +116,9 @@ impl From<&StringifiedPlan> for protobuf::StringifiedPlan {
115116
PlanType::FinalPhysicalPlanWithSchema => Some(protobuf::PlanType {
116117
plan_type_enum: Some(FinalPhysicalPlanWithSchema(EmptyMessage {})),
117118
}),
119+
PlanType::PhysicalPlanError => Some(protobuf::PlanType {
120+
plan_type_enum: Some(PhysicalPlanError(EmptyMessage {})),
121+
}),
118122
},
119123
plan: stringified_plan.plan.to_string(),
120124
}

datafusion/sqllogictest/test_files/explain.slt

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,3 +411,28 @@ logical_plan
411411
physical_plan
412412
01)ProjectionExec: expr=[{c0:1,c1:2.3,c2:abc} as struct(Int64(1),Float64(2.3),Utf8("abc"))]
413413
02)--PlaceholderRowExec
414+
415+
416+
statement ok
417+
create table t1(a int);
418+
419+
statement ok
420+
create table t2(b int);
421+
422+
query TT
423+
explain select a from t1 where exists (select count(*) from t2);
424+
----
425+
logical_plan
426+
01)Filter: EXISTS (<subquery>)
427+
02)--Subquery:
428+
03)----Projection: count(*)
429+
04)------Aggregate: groupBy=[[]], aggr=[[count(Int64(1)) AS count(*)]]
430+
05)--------TableScan: t2
431+
06)--TableScan: t1 projection=[a]
432+
physical_plan_error This feature is not implemented: Physical plan does not support logical expression Exists(Exists { subquery: <subquery>, negated: false })
433+
434+
statement ok
435+
drop table t1;
436+
437+
statement ok
438+
drop table t2;

0 commit comments

Comments
 (0)