From fa0e7d572925f1be6705f4cae7d94682958e9d99 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Wed, 8 Jan 2025 09:47:16 -0500 Subject: [PATCH 1/3] Support nested parenthesized conditional exprs in translator --- core/translate/expr.rs | 83 +++++++++++++++++++++++++++---------- core/translate/group_by.rs | 1 + core/translate/main_loop.rs | 4 ++ 3 files changed, 66 insertions(+), 22 deletions(-) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 4c4ee72b2..6efbe8d89 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -18,6 +18,7 @@ pub struct ConditionMetadata { pub jump_if_condition_is_true: bool, pub jump_target_when_true: BranchOffset, pub jump_target_when_false: BranchOffset, + pub parent_op: Option, } pub fn translate_condition_expr( @@ -30,48 +31,87 @@ pub fn translate_condition_expr( match expr { ast::Expr::Between { .. } => todo!(), ast::Expr::Binary(lhs, ast::Operator::And, rhs) => { - // In a binary AND, never jump to the 'jump_target_when_true' label on the first condition, because - // the second condition must also be true. + // We’re in an AND, so short-circuit on false: let _ = translate_condition_expr( program, referenced_tables, lhs, ConditionMetadata { jump_if_condition_is_true: false, + // Mark that the parent op for sub-expressions is AND + parent_op: Some(ast::Operator::And), ..condition_metadata }, resolver, ); + // Then evaluate RHS with the parent's metadata (still AND) let _ = translate_condition_expr( program, referenced_tables, rhs, - condition_metadata, + ConditionMetadata { + parent_op: Some(ast::Operator::And), + ..condition_metadata + }, resolver, ); } ast::Expr::Binary(lhs, ast::Operator::Or, rhs) => { - let jump_target_when_false = program.allocate_label(); - let _ = translate_condition_expr( - program, - referenced_tables, - lhs, - ConditionMetadata { - // If the first condition is true, we don't need to evaluate the second condition. + if matches!(condition_metadata.parent_op, Some(ast::Operator::And)) { + // we are inside a bigger AND expression, so we do NOT jump to parent's 'true' if LHS or RHS is true. + // we only short-circuit the parent's false label if LHS and RHS are both false. + let local_true_label = program.allocate_label(); + let local_false_label = program.allocate_label(); + + // evaluate LHS in normal OR fashion, short-circuit if true + let lhs_metadata = ConditionMetadata { + jump_if_condition_is_true: true, + jump_target_when_true: local_true_label, + jump_target_when_false: local_false_label, + parent_op: Some(ast::Operator::Or), + }; + translate_condition_expr(program, referenced_tables, lhs, lhs_metadata, resolver)?; + + // if lhs was false, we land here: + program.resolve_label(local_false_label, program.offset()); + + // evaluate rhs with normal OR: short-circuit if true, go to local_true + let rhs_metadata = ConditionMetadata { + jump_if_condition_is_true: true, + jump_target_when_true: local_true_label, + jump_target_when_false: condition_metadata.jump_target_when_false, + // if rhs is also false => parent's false + parent_op: Some(ast::Operator::Or), + }; + translate_condition_expr(program, referenced_tables, rhs, rhs_metadata, resolver)?; + + // if we get here, both lhs+rhs are false: explicit jump to parent's false + program.emit_insn(Insn::Goto { + target_pc: condition_metadata.jump_target_when_false, + }); + // local_true means (C OR D) is true => we do *not* jump to parent's "true" label, + // because the parent is an AND, so we want to keep evaluating the rest: + program.resolve_label(local_true_label, program.offset()); + } else { + let jump_target_when_false = program.allocate_label(); + + let lhs_metadata = ConditionMetadata { jump_if_condition_is_true: true, jump_target_when_false, + parent_op: Some(ast::Operator::Or), ..condition_metadata - }, - resolver, - ); - program.resolve_label(jump_target_when_false, program.offset()); - let _ = translate_condition_expr( - program, - referenced_tables, - rhs, - condition_metadata, - resolver, - ); + }; + + translate_condition_expr(program, referenced_tables, lhs, lhs_metadata, resolver)?; + + // if LHS was false, we land here: + program.resolve_label(jump_target_when_false, program.offset()); + let rhs_metadata = ConditionMetadata { + parent_op: Some(ast::Operator::Or), + ..condition_metadata + }; + translate_condition_expr(program, referenced_tables, rhs, rhs_metadata, resolver)?; + } } ast::Expr::Binary(lhs, op, rhs) => { let lhs_reg = program.alloc_register(); @@ -1917,7 +1957,6 @@ fn translate_variable_sized_function_parameter_list( for arg in args.iter() { translate_expr(program, referenced_tables, arg, current_reg, resolver)?; - current_reg += 1; } diff --git a/core/translate/group_by.rs b/core/translate/group_by.rs index 0990bee0d..c20466f27 100644 --- a/core/translate/group_by.rs +++ b/core/translate/group_by.rs @@ -382,6 +382,7 @@ pub fn emit_group_by<'a>( jump_if_condition_is_true: false, jump_target_when_false: group_by_end_without_emitting_row_label, jump_target_when_true: BranchOffset::Placeholder, // not used. FIXME: this is a bug. HAVING can have e.g. HAVING a OR b. + parent_op: None, }, &t_ctx.resolver, )?; diff --git a/core/translate/main_loop.rs b/core/translate/main_loop.rs index 05daf01e5..e95b3dc22 100644 --- a/core/translate/main_loop.rs +++ b/core/translate/main_loop.rs @@ -225,6 +225,7 @@ pub fn open_loop( jump_if_condition_is_true: false, jump_target_when_true, jump_target_when_false: next, + parent_op: None, }; translate_condition_expr( program, @@ -273,6 +274,7 @@ pub fn open_loop( jump_if_condition_is_true: false, jump_target_when_true, jump_target_when_false, + parent_op: None, }; for predicate in predicates.iter() { translate_condition_expr( @@ -345,6 +347,7 @@ pub fn open_loop( jump_if_condition_is_true: false, jump_target_when_true, jump_target_when_false: next, + parent_op: None, }; translate_condition_expr( program, @@ -529,6 +532,7 @@ pub fn open_loop( jump_if_condition_is_true: false, jump_target_when_true, jump_target_when_false: next, + parent_op: None, }; translate_condition_expr( program, From 183797898b9f91cffaf062e3e650212142e3281e Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Wed, 8 Jan 2025 12:04:17 -0500 Subject: [PATCH 2/3] Add tests for nested conditional expressions --- core/translate/expr.rs | 4 ++-- testing/where.test | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 6efbe8d89..ec2205e7d 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -31,7 +31,8 @@ pub fn translate_condition_expr( match expr { ast::Expr::Between { .. } => todo!(), ast::Expr::Binary(lhs, ast::Operator::And, rhs) => { - // We’re in an AND, so short-circuit on false: + // In a binary AND, never jump to the 'jump_target_when_true' label on the first condition, because + // the second condition must also be true. let _ = translate_condition_expr( program, referenced_tables, @@ -44,7 +45,6 @@ pub fn translate_condition_expr( }, resolver, ); - // Then evaluate RHS with the parent's metadata (still AND) let _ = translate_condition_expr( program, referenced_tables, diff --git a/testing/where.test b/testing/where.test index feb4e812a..c613f784b 100755 --- a/testing/where.test +++ b/testing/where.test @@ -343,3 +343,25 @@ do_execsql_test where-between-true-and-2 { select id from users where id between true and 2; } {1 2} + +do_execsql_test nested-parens-conditionals-or-and-or { + SELECT count(*) FROM users WHERE ((age > 25 OR age < 18) AND (city = 'Boston' OR state = 'MA')); +} {146} + +do_execsql_test nested-parens-conditionals-and-or-and { + SELECT * FROM users WHERE (((age > 18 AND city = 'New Mario') OR age = 92) AND city = 'Lake Paul'); +} {{9989|Timothy|Harrison|woodsmichael@example.net|+1-447-830-5123|782 Wright Harbors|Lake Paul|ID|52330|92}} + + +do_execsql_test nested-parens-conditionals-and-double-or { + SELECT * FROM users WHERE ((age > 30 OR age < 20) AND (state = 'NY' OR state = 'CA')) AND first_name glob 'An*' order by id; +} {{1738|Angelica|Pena|jacksonjonathan@example.net|(867)536-1578x039|663 Jacqueline Estate Apt. 652|Clairehaven|NY|64172|74 +1811|Andrew|Mckee|jchen@example.net|359.939.9548|19809 Blair Junction Apt. 438|New Lawrencefort|NY|26240|42 +3773|Andrew|Peterson|cscott@example.com|(405)410-4972x90408|90513 Munoz Radial Apt. 786|Travisfurt|CA|52951|43 +3875|Anthony|Cordova|ocross@example.org|+1-356-999-4070x557|77081 Aguilar Turnpike|Michaelfurt|CA|73353|37 +4909|Andrew|Carson|michelle31@example.net|823.423.1516|78514 Luke Springs|Lake Crystal|CA|49481|74 +5498|Anna|Hall|elizabethheath@example.org|9778473725|5803 Taylor Tunnel|New Nicholaston|NY|21825|14 +6340|Angela|Freeman|juankelly@example.net|501.372.4720|3912 Ricardo Mission|West Nancyville|NY|60823|34 +8171|Andrea|Lee|dgarrison@example.com|001-594-430-0646|452 Anthony Stravenue|Sandraville|CA|28572|12 +9110|Anthony|Barrett|steven05@example.net|(562)928-9177x8454|86166 Foster Inlet Apt. 284|North Jeffreyburgh|CA|80147|97 +9279|Annette|Lynn|joanne37@example.com|(272)700-7181|2676 Laura Points Apt. 683|Tristanville|NY|48646|91}} From 97d0fc68b2908f2064643b9b5326d6a1b2548597 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Wed, 8 Jan 2025 16:54:41 -0500 Subject: [PATCH 3/3] Add macro and helper to clean up expression translations --- core/translate/expr.rs | 324 ++++++++++++++--------------------------- 1 file changed, 107 insertions(+), 217 deletions(-) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index ec2205e7d..aef2fe30d 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -21,6 +21,46 @@ pub struct ConditionMetadata { pub parent_op: Option, } +fn emit_cond_jump(program: &mut ProgramBuilder, cond_meta: ConditionMetadata, reg: usize) { + if cond_meta.jump_if_condition_is_true { + program.emit_insn(Insn::If { + reg, + target_pc: cond_meta.jump_target_when_true, + null_reg: reg, + }); + } else { + program.emit_insn(Insn::IfNot { + reg, + target_pc: cond_meta.jump_target_when_false, + null_reg: reg, + }); + } +} +macro_rules! emit_cmp_insn { + ( + $program:expr, + $cond:expr, + $op_true:ident, + $op_false:ident, + $lhs:expr, + $rhs:expr + ) => {{ + if $cond.jump_if_condition_is_true { + $program.emit_insn(Insn::$op_true { + lhs: $lhs, + rhs: $rhs, + target_pc: $cond.jump_target_when_true, + }); + } else { + $program.emit_insn(Insn::$op_false { + lhs: $lhs, + rhs: $rhs, + target_pc: $cond.jump_target_when_false, + }); + } + }}; +} + pub fn translate_condition_expr( program: &mut ProgramBuilder, referenced_tables: &[TableReference], @@ -63,7 +103,7 @@ pub fn translate_condition_expr( let local_true_label = program.allocate_label(); let local_false_label = program.allocate_label(); - // evaluate LHS in normal OR fashion, short-circuit if true + // evaluate LHS in normal OR fashion, short-circuit local if true let lhs_metadata = ConditionMetadata { jump_if_condition_is_true: true, jump_target_when_true: local_true_label, @@ -89,8 +129,8 @@ pub fn translate_condition_expr( program.emit_insn(Insn::Goto { target_pc: condition_metadata.jump_target_when_false, }); - // local_true means (C OR D) is true => we do *not* jump to parent's "true" label, - // because the parent is an AND, so we want to keep evaluating the rest: + // local_true: we do not jump to parent's "true" label because the parent is AND, + // so we want to keep evaluating the rest program.resolve_label(local_true_label, program.offset()); } else { let jump_target_when_false = program.allocate_label(); @@ -114,106 +154,26 @@ pub fn translate_condition_expr( } } ast::Expr::Binary(lhs, op, rhs) => { - let lhs_reg = program.alloc_register(); - let _ = translate_expr(program, Some(referenced_tables), lhs, lhs_reg, resolver); - if let ast::Expr::Literal(_) = lhs.as_ref() { - program.mark_last_insn_constant() - } - let rhs_reg = program.alloc_register(); - let _ = translate_expr(program, Some(referenced_tables), rhs, rhs_reg, resolver); - if let ast::Expr::Literal(_) = rhs.as_ref() { - program.mark_last_insn_constant() - } + let lhs_reg = translate_and_mark(program, Some(referenced_tables), lhs, resolver)?; + let rhs_reg = translate_and_mark(program, Some(referenced_tables), rhs, resolver)?; match op { ast::Operator::Greater => { - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::Gt { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }) - } else { - program.emit_insn(Insn::Le { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }) - } + emit_cmp_insn!(program, condition_metadata, Gt, Le, lhs_reg, rhs_reg) } ast::Operator::GreaterEquals => { - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::Ge { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }) - } else { - program.emit_insn(Insn::Lt { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }) - } + emit_cmp_insn!(program, condition_metadata, Ge, Lt, lhs_reg, rhs_reg) } ast::Operator::Less => { - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::Lt { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }) - } else { - program.emit_insn(Insn::Ge { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }) - } + emit_cmp_insn!(program, condition_metadata, Lt, Ge, lhs_reg, rhs_reg) } ast::Operator::LessEquals => { - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::Le { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }) - } else { - program.emit_insn(Insn::Gt { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }) - } + emit_cmp_insn!(program, condition_metadata, Le, Gt, lhs_reg, rhs_reg) } ast::Operator::Equals => { - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::Eq { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }) - } else { - program.emit_insn(Insn::Ne { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }) - } + emit_cmp_insn!(program, condition_metadata, Eq, Ne, lhs_reg, rhs_reg) } ast::Operator::NotEquals => { - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::Ne { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }) - } else { - program.emit_insn(Insn::Eq { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }) - } + emit_cmp_insn!(program, condition_metadata, Ne, Eq, lhs_reg, rhs_reg) } ast::Operator::Is => todo!(), ast::Operator::IsNot => todo!(), @@ -231,19 +191,7 @@ pub fn translate_condition_expr( value: int_value, dest: reg, }); - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::If { - reg, - target_pc: condition_metadata.jump_target_when_true, - null_reg: reg, - }) - } else { - program.emit_insn(Insn::IfNot { - reg, - target_pc: condition_metadata.jump_target_when_false, - null_reg: reg, - }) - } + emit_cond_jump(program, condition_metadata, reg); } else { crate::bail_parse_error!("unsupported literal type in condition"); } @@ -254,19 +202,7 @@ pub fn translate_condition_expr( value: string.clone(), dest: reg, }); - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::If { - reg, - target_pc: condition_metadata.jump_target_when_true, - null_reg: reg, - }) - } else { - program.emit_insn(Insn::IfNot { - reg, - target_pc: condition_metadata.jump_target_when_false, - null_reg: reg, - }) - } + emit_cond_jump(program, condition_metadata, reg); } unimpl => todo!("literal {:?} not implemented", unimpl), }, @@ -392,18 +328,8 @@ pub fn translate_condition_expr( match op { ast::LikeOperator::Like | ast::LikeOperator::Glob => { let pattern_reg = program.alloc_register(); - let column_reg = program.alloc_register(); let mut constant_mask = 0; - let _ = translate_expr( - program, - Some(referenced_tables), - lhs, - column_reg, - resolver, - )?; - if let ast::Expr::Literal(_) = lhs.as_ref() { - program.mark_last_insn_constant(); - } + let _ = translate_and_mark(program, Some(referenced_tables), lhs, resolver); let _ = translate_expr( program, Some(referenced_tables), @@ -411,7 +337,7 @@ pub fn translate_condition_expr( pattern_reg, resolver, )?; - if let ast::Expr::Literal(_) = rhs.as_ref() { + if matches!(rhs.as_ref(), ast::Expr::Literal(_)) { program.mark_last_insn_constant(); constant_mask = 1; } @@ -434,19 +360,7 @@ pub fn translate_condition_expr( ast::LikeOperator::Regexp => todo!(), } if !*not { - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::If { - reg: cur_reg, - target_pc: condition_metadata.jump_target_when_true, - null_reg: cur_reg, - }); - } else { - program.emit_insn(Insn::IfNot { - reg: cur_reg, - target_pc: condition_metadata.jump_target_when_false, - null_reg: cur_reg, - }); - } + emit_cond_jump(program, condition_metadata, cur_reg); } else if condition_metadata.jump_if_condition_is_true { program.emit_insn(Insn::IfNot { reg: cur_reg, @@ -462,16 +376,18 @@ pub fn translate_condition_expr( } } ast::Expr::Parenthesized(exprs) => { - // TODO: this is probably not correct; multiple expressions in a parenthesized expression - // are reserved for special cases like `(a, b) IN ((1, 2), (3, 4))`. - for expr in exprs { + if exprs.len() == 1 { let _ = translate_condition_expr( program, referenced_tables, - expr, + &exprs[0], condition_metadata, resolver, ); + } else { + crate::bail_parse_error!( + "parenthesized condtional should have exactly one expression" + ); } } _ => todo!("op {:?} not implemented", expr), @@ -875,7 +791,7 @@ pub fn translate_expr( unreachable!("this is always ast::Expr::Cast") } ScalarFunc::Changes => { - if let Some(_) = args { + if args.is_some() { crate::bail_parse_error!( "{} fucntion with more than 0 arguments", srf @@ -1110,12 +1026,8 @@ pub fn translate_expr( ); }; for arg in args { - let reg = program.alloc_register(); let _ = - translate_expr(program, referenced_tables, arg, reg, resolver)?; - if let ast::Expr::Literal(_) = arg { - program.mark_last_insn_constant() - } + translate_and_mark(program, referenced_tables, arg, resolver); } program.emit_insn(Insn::Function { // Only constant patterns for LIKE are supported currently, so this @@ -1153,12 +1065,11 @@ pub fn translate_expr( srf.to_string() ); }; - - let regs = program.alloc_register(); - translate_expr(program, referenced_tables, &args[0], regs, resolver)?; + let reg = + translate_and_mark(program, referenced_tables, &args[0], resolver)?; program.emit_insn(Insn::Function { constant_mask: 0, - start_reg: regs, + start_reg: reg, dest: target_register, func: func_ctx, }); @@ -1184,12 +1095,10 @@ pub fn translate_expr( if let Some(args) = args { for arg in args.iter() { // register containing result of each argument expression - let target_reg = program.alloc_register(); - _ = translate_expr( + let _ = translate_and_mark( program, referenced_tables, arg, - target_reg, resolver, )?; } @@ -1221,15 +1130,14 @@ pub fn translate_expr( let str_reg = program.alloc_register(); let start_reg = program.alloc_register(); let length_reg = program.alloc_register(); - - translate_expr( + let str_reg = translate_expr( program, referenced_tables, &args[0], str_reg, resolver, )?; - translate_expr( + let _ = translate_expr( program, referenced_tables, &args[1], @@ -1245,7 +1153,6 @@ pub fn translate_expr( resolver, )?; } - program.emit_insn(Insn::Function { constant_mask: 0, start_reg: str_reg, @@ -1265,8 +1172,8 @@ pub fn translate_expr( } else { crate::bail_parse_error!("hex function with no arguments",); }; - let regs = program.alloc_register(); - translate_expr(program, referenced_tables, &args[0], regs, resolver)?; + let regs = + translate_and_mark(program, referenced_tables, &args[0], resolver)?; program.emit_insn(Insn::Function { constant_mask: 0, start_reg: regs, @@ -1306,12 +1213,10 @@ pub fn translate_expr( if let Some(args) = args { for arg in args.iter() { // register containing result of each argument expression - let target_reg = program.alloc_register(); - _ = translate_expr( + let _ = translate_and_mark( program, referenced_tables, arg, - target_reg, resolver, )?; } @@ -1325,7 +1230,7 @@ pub fn translate_expr( Ok(target_register) } ScalarFunc::TotalChanges => { - if let Some(_) = args { + if args.is_some() { crate::bail_parse_error!( "{} fucntion with more than 0 arguments", srf.to_string() @@ -1361,11 +1266,7 @@ pub fn translate_expr( }; for arg in args.iter() { - let reg = program.alloc_register(); - translate_expr(program, referenced_tables, arg, reg, resolver)?; - if let ast::Expr::Literal(_) = arg { - program.mark_last_insn_constant(); - } + translate_and_mark(program, referenced_tables, arg, resolver)?; } program.emit_insn(Insn::Function { constant_mask: 0, @@ -1387,12 +1288,7 @@ pub fn translate_expr( crate::bail_parse_error!("min function with no arguments"); }; for arg in args { - let reg = program.alloc_register(); - let _ = - translate_expr(program, referenced_tables, arg, reg, resolver)?; - if let ast::Expr::Literal(_) = arg { - program.mark_last_insn_constant() - } + translate_and_mark(program, referenced_tables, arg, resolver)?; } program.emit_insn(Insn::Function { @@ -1415,12 +1311,7 @@ pub fn translate_expr( crate::bail_parse_error!("max function with no arguments"); }; for arg in args { - let reg = program.alloc_register(); - let _ = - translate_expr(program, referenced_tables, arg, reg, resolver)?; - if let ast::Expr::Literal(_) = arg { - program.mark_last_insn_constant() - } + translate_and_mark(program, referenced_tables, arg, resolver)?; } program.emit_insn(Insn::Function { @@ -1456,7 +1347,7 @@ pub fn translate_expr( resolver, )?; let second_reg = program.alloc_register(); - translate_expr( + let _ = translate_expr( program, referenced_tables, &args[1], @@ -1507,34 +1398,30 @@ pub fn translate_expr( srf.to_string() ); }; - let str_reg = program.alloc_register(); let pattern_reg = program.alloc_register(); let replacement_reg = program.alloc_register(); - - translate_expr( + let _ = translate_expr( program, referenced_tables, &args[0], str_reg, resolver, )?; - translate_expr( + let _ = translate_expr( program, referenced_tables, &args[1], pattern_reg, resolver, )?; - - translate_expr( + let _ = translate_expr( program, referenced_tables, &args[2], replacement_reg, resolver, )?; - program.emit_insn(Insn::Function { constant_mask: 0, start_reg: str_reg, @@ -1601,12 +1488,12 @@ pub fn translate_expr( }; let mut start_reg = None; if let Some(arg) = args.first() { - let reg = program.alloc_register(); - start_reg = Some(reg); - translate_expr(program, referenced_tables, arg, reg, resolver)?; - if let ast::Expr::Literal(_) = arg { - program.mark_last_insn_constant() - } + start_reg = Some(translate_and_mark( + program, + referenced_tables, + arg, + resolver, + )?); } program.emit_insn(Insn::Function { constant_mask: 0, @@ -1648,10 +1535,8 @@ pub fn translate_expr( crate::bail_parse_error!("{} function with no arguments", math_func); }; - let reg = program.alloc_register(); - - translate_expr(program, referenced_tables, &args[0], reg, resolver)?; - + let reg = + translate_and_mark(program, referenced_tables, &args[0], resolver)?; program.emit_insn(Insn::Function { constant_mask: 0, start_reg: reg, @@ -1673,20 +1558,12 @@ pub fn translate_expr( } else { crate::bail_parse_error!("{} function with no arguments", math_func); }; - let reg1 = program.alloc_register(); + let _ = + translate_expr(program, referenced_tables, &args[0], reg1, resolver)?; let reg2 = program.alloc_register(); - - translate_expr(program, referenced_tables, &args[0], reg1, resolver)?; - if let ast::Expr::Literal(_) = &args[0] { - program.mark_last_insn_constant(); - } - - translate_expr(program, referenced_tables, &args[1], reg2, resolver)?; - if let ast::Expr::Literal(_) = &args[1] { - program.mark_last_insn_constant(); - } - + let _ = + translate_expr(program, referenced_tables, &args[1], reg2, resolver)?; program.emit_insn(Insn::Function { constant_mask: 0, start_reg: target_register + 1, @@ -1710,7 +1587,6 @@ pub fn translate_expr( }; let regs = program.alloc_registers(args.len()); - for (i, arg) in args.iter().enumerate() { translate_expr(program, referenced_tables, arg, regs + i, resolver)?; } @@ -1989,6 +1865,20 @@ pub fn maybe_apply_affinity(col_type: Type, target_register: usize, program: &mu } } +pub fn translate_and_mark( + program: &mut ProgramBuilder, + referenced_tables: Option<&[TableReference]>, + expr: &ast::Expr, + resolver: &Resolver, +) -> Result { + let target_register = program.alloc_register(); + translate_expr(program, referenced_tables, expr, target_register, resolver)?; + if matches!(expr, ast::Expr::Literal(_)) { + program.mark_last_insn_constant(); + } + Ok(target_register) +} + /// Get an appropriate name for an expression. /// If the query provides an alias (e.g. `SELECT a AS b FROM t`), use that (e.g. `b`). /// If the expression is a column from a table, use the column name (e.g. `a`).