Skip to content

Commit b0e29f0

Browse files
authored
Fix Explain Analyze (datafusion-contrib#168)
1 parent da7285e commit b0e29f0

2 files changed

Lines changed: 66 additions & 7 deletions

File tree

datafusion-federation/src/optimizer/mod.rs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -191,14 +191,22 @@ impl FederationOptimizerRule {
191191
return Ok((None, ScanResult::Distinct(provider)));
192192
}
193193

194-
let Some(optimizer) = provider.optimizer() else {
195-
// No optimizer provided
196-
return Ok((None, ScanResult::None));
197-
};
194+
// Analyze plans (EXPLAIN ANALYZE) cannot be converted to SQL by
195+
// the Unparser, so they must not be federated as a whole. Only the
196+
// inner query should be federated; DataFusion's AnalyzeExec will
197+
// handle executing it and collecting metrics.
198+
if matches!(plan, LogicalPlan::Analyze(_)) {
199+
// Fall through to federate children instead.
200+
} else {
201+
let Some(optimizer) = provider.optimizer() else {
202+
// No optimizer provided
203+
return Ok((None, ScanResult::None));
204+
};
198205

199-
// If this is the root plan node; federate the entire plan
200-
let optimized = optimizer.optimize(plan.clone(), _config, |_, _| {})?;
201-
return Ok((Some(optimized), ScanResult::None));
206+
// If this is the root plan node; federate the entire plan
207+
let optimized = optimizer.optimize(plan.clone(), _config, |_, _| {})?;
208+
return Ok((Some(optimized), ScanResult::None));
209+
}
202210
}
203211

204212
// The plan is ambiguous; any input that is not yet optimized and has a

datafusion-federation/src/sql/mod.rs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -766,6 +766,57 @@ mod tests {
766766
Ok(())
767767
}
768768

769+
/// EXPLAIN ANALYZE must not federate the Analyze wrapper — only the inner
770+
/// query should be federated. Otherwise the SQL Unparser fails because it
771+
/// cannot convert Analyze to SQL.
772+
#[tokio::test]
773+
async fn explain_analyze_not_federated() -> Result<(), DataFusionError> {
774+
let executor = TestExecutor {
775+
compute_context: "a".into(),
776+
};
777+
778+
let table_ref = "test_table".to_string();
779+
let table = get_test_table_provider(table_ref.clone(), executor);
780+
781+
let state = crate::default_session_state();
782+
let ctx = SessionContext::new_with_state(state);
783+
ctx.register_table(table_ref, table).unwrap();
784+
785+
let plan = ctx
786+
.sql("EXPLAIN ANALYZE SELECT * FROM test_table")
787+
.await?
788+
.into_optimized_plan()?;
789+
790+
// The top-level node must be Analyze, not Federated.
791+
assert!(
792+
matches!(plan, LogicalPlan::Analyze(_)),
793+
"Expected Analyze at root, got: {}",
794+
plan.display_indent()
795+
);
796+
797+
// The inner plan should contain a Federated extension node.
798+
let mut found_federated = false;
799+
plan.apply(|node| {
800+
if let LogicalPlan::Extension(ext) = node {
801+
if ext.node.name() == "Federated" {
802+
found_federated = true;
803+
return Ok(TreeNodeRecursion::Stop);
804+
}
805+
}
806+
Ok(TreeNodeRecursion::Continue)
807+
})?;
808+
assert!(
809+
found_federated,
810+
"Expected a Federated node inside the Analyze plan"
811+
);
812+
813+
// Physical planning should succeed (this is where it used to fail).
814+
let physical_plan = ctx.state().create_physical_plan(&plan).await?;
815+
assert_eq!(physical_plan.name(), "AnalyzeExec");
816+
817+
Ok(())
818+
}
819+
769820
#[tokio::test]
770821
async fn sql_query_rewriter_hook_invoked_and_rewrites_sql() -> Result<(), DataFusionError> {
771822
let executor = TestExecutor {

0 commit comments

Comments
 (0)