Skip to content

Commit be98f04

Browse files
jussisaurioalpaylan
authored andcommitted
core/optimizer: do expression rewriting on all expressions
1 parent 13ec82b commit be98f04

File tree

3 files changed

+81
-25
lines changed

3 files changed

+81
-25
lines changed

core/translate/optimizer.rs

Lines changed: 56 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ pub fn optimize_plan(plan: &mut Plan) -> Result<()> {
2424
*/
2525
fn optimize_select_plan(plan: &mut SelectPlan) -> Result<()> {
2626
optimize_subqueries(&mut plan.source)?;
27-
rewrite_exprs(&mut plan.source, &mut plan.where_clause)?;
27+
rewrite_exprs_select(plan)?;
2828
if let ConstantConditionEliminationResult::ImpossibleCondition =
2929
eliminate_constants(&mut plan.source, &mut plan.where_clause)?
3030
{
@@ -55,7 +55,7 @@ fn optimize_select_plan(plan: &mut SelectPlan) -> Result<()> {
5555
}
5656

5757
fn optimize_delete_plan(plan: &mut DeletePlan) -> Result<()> {
58-
rewrite_exprs(&mut plan.source, &mut plan.where_clause)?;
58+
rewrite_exprs_delete(plan)?;
5959
if let ConstantConditionEliminationResult::ImpossibleCondition =
6060
eliminate_constants(&mut plan.source, &mut plan.where_clause)?
6161
{
@@ -603,52 +603,83 @@ fn push_scan_direction(operator: &mut SourceOperator, direction: &Direction) {
603603
}
604604
}
605605

606-
fn rewrite_exprs(
607-
operator: &mut SourceOperator,
608-
where_clauses: &mut Option<Vec<ast::Expr>>,
609-
) -> Result<()> {
610-
if let Some(predicates) = where_clauses {
611-
for expr in predicates.iter_mut() {
606+
fn rewrite_exprs_select(plan: &mut SelectPlan) -> Result<()> {
607+
rewrite_source_operator_exprs(&mut plan.source)?;
608+
for rc in plan.result_columns.iter_mut() {
609+
rewrite_expr(&mut rc.expr)?;
610+
}
611+
for agg in plan.aggregates.iter_mut() {
612+
rewrite_expr(&mut agg.original_expr)?;
613+
}
614+
if let Some(predicates) = &mut plan.where_clause {
615+
for expr in predicates {
616+
rewrite_expr(expr)?;
617+
}
618+
}
619+
if let Some(group_by) = &mut plan.group_by {
620+
for expr in group_by.exprs.iter_mut() {
621+
rewrite_expr(expr)?;
622+
}
623+
}
624+
if let Some(order_by) = &mut plan.order_by {
625+
for (expr, _) in order_by.iter_mut() {
626+
rewrite_expr(expr)?;
627+
}
628+
}
629+
630+
Ok(())
631+
}
632+
633+
fn rewrite_exprs_delete(plan: &mut DeletePlan) -> Result<()> {
634+
rewrite_source_operator_exprs(&mut plan.source)?;
635+
if let Some(predicates) = &mut plan.where_clause {
636+
for expr in predicates {
612637
rewrite_expr(expr)?;
613638
}
614639
}
615640

641+
Ok(())
642+
}
643+
644+
fn rewrite_source_operator_exprs(operator: &mut SourceOperator) -> Result<()> {
616645
match operator {
617646
SourceOperator::Join {
618647
left,
619648
right,
620649
predicates,
621650
..
622651
} => {
623-
rewrite_exprs(left, where_clauses)?;
624-
rewrite_exprs(right, where_clauses)?;
652+
rewrite_source_operator_exprs(left)?;
653+
rewrite_source_operator_exprs(right)?;
625654

626655
if let Some(predicates) = predicates {
627656
for expr in predicates.iter_mut() {
628657
rewrite_expr(expr)?;
629658
}
630659
}
660+
661+
Ok(())
631662
}
632-
SourceOperator::Scan {
633-
predicates: Some(preds),
634-
..
635-
} => {
636-
for expr in preds.iter_mut() {
637-
rewrite_expr(expr)?;
663+
SourceOperator::Scan { predicates, .. } | SourceOperator::Search { predicates, .. } => {
664+
if let Some(predicates) = predicates {
665+
for expr in predicates.iter_mut() {
666+
rewrite_expr(expr)?;
667+
}
638668
}
669+
670+
Ok(())
639671
}
640-
SourceOperator::Search {
641-
predicates: Some(preds),
642-
..
643-
} => {
644-
for expr in preds.iter_mut() {
645-
rewrite_expr(expr)?;
672+
SourceOperator::Subquery { predicates, .. } => {
673+
if let Some(predicates) = predicates {
674+
for expr in predicates.iter_mut() {
675+
rewrite_expr(expr)?;
676+
}
646677
}
678+
679+
Ok(())
647680
}
648-
_ => (),
681+
SourceOperator::Nothing { .. } => Ok(()),
649682
}
650-
651-
Ok(())
652683
}
653684

654685
#[derive(Debug, Clone, Copy, PartialEq, Eq)]

core/translate/planner.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,14 @@ fn parse_join(
544544
pub fn parse_limit(limit: Limit) -> Option<usize> {
545545
if let Expr::Literal(ast::Literal::Numeric(n)) = limit.expr {
546546
n.parse().ok()
547+
} else if let Expr::Id(id) = limit.expr {
548+
if id.0.eq_ignore_ascii_case("true") {
549+
Some(1)
550+
} else if id.0.eq_ignore_ascii_case("false") {
551+
Some(0)
552+
} else {
553+
None
554+
}
547555
} else {
548556
None
549557
}

testing/select.test

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,14 @@ do_execsql_test select-const-2 {
1111
SELECT 2
1212
} {2}
1313

14+
do_execsql_test select-true {
15+
SELECT true
16+
} {1}
17+
18+
do_execsql_test select-false {
19+
SELECT false
20+
} {0}
21+
1422
do_execsql_test select-text-escape-1 {
1523
SELECT '''a'
1624
} {'a}
@@ -31,6 +39,15 @@ do_execsql_test select-limit-0 {
3139
SELECT id FROM users LIMIT 0;
3240
} {}
3341

42+
# ORDER BY id here because sqlite uses age_idx here and we (yet) don't so force it to evaluate in ID order
43+
do_execsql_test select-limit-true {
44+
SELECT id FROM users ORDER BY id LIMIT true;
45+
} {1}
46+
47+
do_execsql_test select-limit-false {
48+
SELECT id FROM users ORDER BY id LIMIT false;
49+
} {}
50+
3451
do_execsql_test realify {
3552
select price from products limit 1;
3653
} {79.0}

0 commit comments

Comments
 (0)