Skip to content

Commit 2b986c8

Browse files
Fix index panic in unparser with mismatched stacked projections (#21094)
## Rationale for this change This PR adds a length guard in `subquery_alias_inner_query_and_columns` to prevent a panic when outer and inner projections have different expression counts Without this fix, optimizer passes like `CommonSubexprEliminate` can produce stacked projections where the inner projections has more exprs than the outer. The function iterates over `inner_projection.expr` by index and uses the _same_ index to access `outer_projections.expr`, causing an index oob panic This PR bails out early when the lengths don't match. This is consistent with all the other early returns in the function that reject non-matching plan shapes
1 parent bec6714 commit 2b986c8

File tree

1 file changed

+54
-0
lines changed

1 file changed

+54
-0
lines changed

datafusion/sql/src/unparser/rewrite.rs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,10 @@ pub(super) fn subquery_alias_inner_query_and_columns(
318318
// Check if the inner projection and outer projection have a matching pattern like
319319
// Projection: j1.j1_id AS id
320320
// Projection: j1.j1_id
321+
if outer_projections.expr.len() != inner_projection.expr.len() {
322+
return (plan, vec![]);
323+
}
324+
321325
for (i, inner_expr) in inner_projection.expr.iter().enumerate() {
322326
let Expr::Alias(outer_alias) = &outer_projections.expr[i] else {
323327
return (plan, vec![]);
@@ -490,3 +494,53 @@ impl TreeNodeRewriter for TableAliasRewriter<'_> {
490494
}
491495
}
492496
}
497+
498+
#[cfg(test)]
499+
mod tests {
500+
use super::*;
501+
use arrow::datatypes::{DataType, Field};
502+
use datafusion_expr::{LogicalPlanBuilder, col, table_scan};
503+
504+
// this is a regression test: when the outer projection has fewer expressions than
505+
// the inner projection, `subquery_alias_inner_query_and_columns` must not panic
506+
// with an index oob error
507+
// note: this happens when optimizer passes (e.g. CommonSubexprEliminate)
508+
// insert an inner projection with extra columns that a subsequent projection narrows
509+
// back down
510+
#[test]
511+
fn test_stacked_projections_mismatched_lengths_no_panic() {
512+
let schema = Schema::new(vec![
513+
Field::new("id", DataType::Int32, false),
514+
Field::new("name", DataType::Utf8, false),
515+
]);
516+
517+
// Inner projection has 2 expressions, outer has 0 (empty).
518+
let inner_plan = LogicalPlanBuilder::from(
519+
table_scan(Some("t"), &schema, Some(vec![0, 1]))
520+
.unwrap()
521+
.build()
522+
.unwrap(),
523+
)
524+
.project(vec![col("t.id"), col("t.name")])
525+
.unwrap()
526+
.build()
527+
.unwrap();
528+
529+
// Build an empty outer projection over the inner.
530+
let outer_plan = LogicalPlanBuilder::from(inner_plan)
531+
.project(Vec::<Expr>::new())
532+
.unwrap()
533+
.alias("sub")
534+
.unwrap()
535+
.build()
536+
.unwrap();
537+
538+
let LogicalPlan::SubqueryAlias(subquery_alias) = &outer_plan else {
539+
panic!("expected SubqueryAlias");
540+
};
541+
542+
// should return early without panicking
543+
let (_plan, columns) = subquery_alias_inner_query_and_columns(subquery_alias);
544+
assert!(columns.is_empty());
545+
}
546+
}

0 commit comments

Comments
 (0)