Skip to content

Commit c65af67

Browse files
committed
fix: fix pivot extra columns on projection
1 parent ff62551 commit c65af67

File tree

2 files changed

+141
-35
lines changed

2 files changed

+141
-35
lines changed

src/query/sql/src/planner/binder/bind_query/bind_select.rs

Lines changed: 105 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,19 @@ use databend_common_ast::ast::Expr::Array;
2323
use databend_common_ast::ast::FunctionCall;
2424
use databend_common_ast::ast::GroupBy;
2525
use databend_common_ast::ast::Identifier;
26+
use databend_common_ast::ast::Indirection;
2627
use databend_common_ast::ast::Join;
2728
use databend_common_ast::ast::JoinCondition;
2829
use databend_common_ast::ast::JoinOperator;
2930
use databend_common_ast::ast::Literal;
3031
use databend_common_ast::ast::OrderByExpr;
3132
use databend_common_ast::ast::Pivot;
3233
use databend_common_ast::ast::PivotValues;
34+
use databend_common_ast::ast::Query;
3335
use databend_common_ast::ast::SelectStmt;
3436
use databend_common_ast::ast::SelectTarget;
37+
use databend_common_ast::ast::SetExpr;
38+
use databend_common_ast::ast::TableAlias;
3539
use databend_common_ast::ast::TableReference;
3640
use databend_common_ast::ast::UnpivotName;
3741
use databend_common_ast::Span;
@@ -50,7 +54,6 @@ use crate::planner::binder::BindContext;
5054
use crate::planner::binder::Binder;
5155
use crate::planner::QueryExecutor;
5256
use crate::AsyncFunctionRewriter;
53-
use crate::ColumnBinding;
5457

5558
// A normalized IR for `SELECT` clause.
5659
#[derive(Debug, Default)]
@@ -84,6 +87,12 @@ impl Binder {
8487
.check_enterprise_enabled(self.ctx.get_license_key(), Feature::VirtualColumn)
8588
.is_ok();
8689

90+
let mut rewriter =
91+
SelectRewriter::new(self.name_resolution_ctx.unquoted_ident_case_sensitive)
92+
.with_subquery_executor(self.subquery_executor.clone());
93+
let new_stmt = rewriter.rewrite(stmt)?;
94+
let stmt = new_stmt.as_ref().unwrap_or(stmt);
95+
8796
let (mut s_expr, mut from_context) = if stmt.from.is_empty() {
8897
let select_list = &stmt.select_list;
8998
self.bind_dummy_table(bind_context, select_list)?
@@ -111,14 +120,6 @@ impl Binder {
111120
self.bind_table_reference(bind_context, &cross_joins)?
112121
};
113122

114-
let mut rewriter = SelectRewriter::new(
115-
from_context.all_column_bindings(),
116-
self.name_resolution_ctx.unquoted_ident_case_sensitive,
117-
)
118-
.with_subquery_executor(self.subquery_executor.clone());
119-
let new_stmt = rewriter.rewrite(stmt)?;
120-
let stmt = new_stmt.as_ref().unwrap_or(stmt);
121-
122123
// Try put window definitions into bind context.
123124
// This operation should be before `normalize_select_list` because window functions can be used in select list.
124125
self.analyze_window_definition(&mut from_context, &stmt.window_list)?;
@@ -275,19 +276,16 @@ impl Binder {
275276

276277
/// It is useful when implementing some SQL syntax sugar,
277278
///
278-
/// [`column_binding`] contains the column binding information of the SelectStmt.
279-
///
280279
/// to rewrite the SelectStmt, just add a new rewrite_* function and call it in the `rewrite` function.
281280
#[allow(dead_code)]
282-
struct SelectRewriter<'a> {
283-
column_binding: &'a [ColumnBinding],
281+
struct SelectRewriter {
284282
new_stmt: Option<SelectStmt>,
285283
is_unquoted_ident_case_sensitive: bool,
286284
subquery_executor: Option<Arc<dyn QueryExecutor>>,
287285
}
288286

289287
// helper functions to SelectRewriter
290-
impl SelectRewriter<'_> {
288+
impl SelectRewriter {
291289
fn parse_aggregate_function(expr: &Expr) -> Result<(&Identifier, &[Expr])> {
292290
match expr {
293291
Expr::FunctionCall {
@@ -372,10 +370,9 @@ impl SelectRewriter<'_> {
372370
}
373371
}
374372

375-
impl<'a> SelectRewriter<'a> {
376-
fn new(column_binding: &'a [ColumnBinding], is_unquoted_ident_case_sensitive: bool) -> Self {
373+
impl SelectRewriter {
374+
fn new(is_unquoted_ident_case_sensitive: bool) -> Self {
377375
SelectRewriter {
378-
column_binding,
379376
new_stmt: None,
380377
is_unquoted_ident_case_sensitive,
381378
subquery_executor: None,
@@ -432,12 +429,14 @@ impl<'a> SelectRewriter<'a> {
432429
.collect::<Result<Vec<_>>>()?;
433430
let new_group_by = stmt.group_by.clone().unwrap_or_else(|| GroupBy::All);
434431

435-
let mut new_select_list = stmt.select_list.clone();
436-
if let Some(star) = new_select_list.iter_mut().find(|target| target.is_star()) {
437-
let mut exclude_columns = aggregate_args_names;
438-
exclude_columns.push(pivot.value_column.clone());
439-
star.exclude(exclude_columns);
432+
let mut exclude_columns = aggregate_args_names.clone();
433+
exclude_columns.push(pivot.value_column.clone());
434+
let mut star_target = SelectTarget::StarColumns {
435+
qualified: vec![Indirection::Star(None)],
436+
column_filter: None,
440437
};
438+
star_target.exclude(exclude_columns);
439+
let mut inner_select_list = vec![star_target];
441440
let new_aggregate_name = Identifier {
442441
name: format!("{}_if", aggregate_name.name),
443442
..aggregate_name.clone()
@@ -465,7 +464,7 @@ impl<'a> SelectRewriter<'a> {
465464
&values,
466465
&new_aggregate_name,
467466
aggregate_args,
468-
&mut new_select_list,
467+
&mut inner_select_list,
469468
stmt,
470469
)?;
471470
}
@@ -485,7 +484,7 @@ impl<'a> SelectRewriter<'a> {
485484
&values,
486485
&new_aggregate_name,
487486
aggregate_args,
488-
&mut new_select_list,
487+
&mut inner_select_list,
489488
stmt,
490489
)?;
491490
} else {
@@ -526,7 +525,7 @@ impl<'a> SelectRewriter<'a> {
526525
&values,
527526
&new_aggregate_name,
528527
aggregate_args,
529-
&mut new_select_list,
528+
&mut inner_select_list,
530529
stmt,
531530
)?;
532531
} else {
@@ -537,16 +536,46 @@ impl<'a> SelectRewriter<'a> {
537536
}
538537
}
539538

540-
if let Some(ref mut new_stmt) = self.new_stmt {
541-
new_stmt.select_list = new_select_list;
542-
new_stmt.group_by = Some(new_group_by);
543-
} else {
544-
self.new_stmt = Some(SelectStmt {
545-
select_list: new_select_list,
546-
group_by: Some(new_group_by),
547-
..stmt.clone()
548-
});
549-
}
539+
let mut inner_from = stmt.from[0].clone();
540+
Self::strip_pivot(&mut inner_from);
541+
542+
let inner_stmt = SelectStmt {
543+
span: stmt.span,
544+
hints: None,
545+
distinct: false,
546+
top_n: None,
547+
select_list: inner_select_list,
548+
from: vec![inner_from],
549+
selection: None,
550+
group_by: Some(new_group_by),
551+
having: None,
552+
window_list: None,
553+
qualify: None,
554+
};
555+
556+
let inner_query = Query {
557+
span: stmt.span,
558+
with: None,
559+
body: SetExpr::Select(Box::new(inner_stmt)),
560+
order_by: vec![],
561+
limit: vec![],
562+
offset: None,
563+
ignore_result: false,
564+
};
565+
566+
let subquery_ref = TableReference::Subquery {
567+
span: Self::table_ref_span(&stmt.from[0]),
568+
lateral: false,
569+
subquery: Box::new(inner_query),
570+
alias: Some(Self::table_ref_alias(&stmt.from[0])),
571+
pivot: None,
572+
unpivot: None,
573+
};
574+
575+
let mut outer_stmt = stmt.clone();
576+
outer_stmt.from = vec![subquery_ref];
577+
578+
self.new_stmt = Some(outer_stmt);
550579
Ok(())
551580
}
552581

@@ -625,6 +654,47 @@ impl<'a> SelectRewriter<'a> {
625654
Ok(values)
626655
}
627656

657+
fn strip_pivot(table_ref: &mut TableReference) {
658+
match table_ref {
659+
TableReference::Table { pivot, .. } => {
660+
*pivot = None;
661+
}
662+
TableReference::Subquery { pivot, .. } => {
663+
*pivot = None;
664+
}
665+
_ => {}
666+
}
667+
}
668+
669+
fn table_ref_span(table_ref: &TableReference) -> Span {
670+
match table_ref {
671+
TableReference::Table { span, .. } => *span,
672+
TableReference::TableFunction { span, .. } => *span,
673+
TableReference::Subquery { span, .. } => *span,
674+
TableReference::Join { span, .. } => *span,
675+
TableReference::Location { span, .. } => *span,
676+
}
677+
}
678+
679+
fn table_ref_alias(table_ref: &TableReference) -> TableAlias {
680+
match table_ref {
681+
TableReference::Table { table, alias, .. } => {
682+
alias.clone().unwrap_or_else(|| TableAlias {
683+
name: table.clone(),
684+
columns: vec![],
685+
})
686+
}
687+
TableReference::Subquery { alias, .. } => alias.clone().unwrap_or_else(|| TableAlias {
688+
name: Identifier::from_name(Self::table_ref_span(table_ref), "__pivot_subquery"),
689+
columns: vec![],
690+
}),
691+
_ => TableAlias {
692+
name: Identifier::from_name(Self::table_ref_span(table_ref), "__pivot_subquery"),
693+
columns: vec![],
694+
},
695+
}
696+
}
697+
628698
fn build_pivot_source_query(&self, stmt: &SelectStmt) -> Result<String> {
629699
// Build the source query for the pivot table without the pivot clause
630700
// This is used to get distinct values for ANY pivot

tests/sqllogictests/suites/query/pivot.test

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,24 @@ SELECT *
6868
1 18000 8000 10400 11000
6969
2 5300 90700 39500 12000
7070

71+
query I
72+
SELECT empid
73+
FROM monthly_sales
74+
PIVOT(SUM(amount) FOR MONTH IN ('JAN', 'FEB', 'MAR', 'APR'))
75+
ORDER BY EMPID
76+
----
77+
1
78+
2
79+
80+
query I
81+
SELECT empid
82+
FROM monthly_sales
83+
PIVOT(SUM(amount) FOR MONTH IN (SELECT DISTINCT month FROM monthly_sales))
84+
ORDER BY EMPID
85+
----
86+
1
87+
2
88+
7189
query IIIII
7290
SELECT *
7391
FROM (SELECT * FROM monthly_sales)
@@ -77,6 +95,24 @@ SELECT *
7795
1 18000 8000 10400 11000
7896
2 5300 90700 39500 12000
7997

98+
query IIIII
99+
SELECT empid, JAN, FEB, MAR, APR
100+
FROM monthly_sales
101+
PIVOT(SUM(amount) FOR MONTH IN ('JAN', 'FEB', 'MAR', 'APR'))
102+
ORDER BY EMPID
103+
----
104+
1 10400 8000 11000 18000
105+
2 39500 90700 12000 5300
106+
107+
query IIII
108+
SELECT empid, APR, FEB, JAN
109+
FROM monthly_sales
110+
PIVOT(SUM(amount) FOR MONTH IN (SELECT DISTINCT month FROM monthly_sales))
111+
ORDER BY EMPID
112+
----
113+
1 18000 8000 10400
114+
2 5300 90700 39500
115+
80116
## The subquery of `pivot in` must return one column
81117
statement error 1065
82118
SELECT empid,jan,feb,mar,apr FROM (

0 commit comments

Comments
 (0)