From 183ea8e362801a78b892a4e53761601196f135dd Mon Sep 17 00:00:00 2001 From: Alex Miller Date: Sat, 7 Dec 2024 21:04:03 -0800 Subject: [PATCH 1/3] Implement support for iif(). In sqlite, iif() looks like: sqlite> create table iiftest(a int, b int, c int); sqlite> explain select iif(a,b,c) from iiftest; addr opcode p1 p2 p3 p4 p5 comment ---- ------------- ---- ---- ---- ------------- -- ------------- 0 Init 0 11 0 0 Start at 11 1 OpenRead 0 2 0 3 0 root=2 iDb=0; iiftest 2 Rewind 0 10 0 0 3 Column 0 0 2 0 r[2]= cursor 0 column 0 4 IfNot 2 7 1 0 5 Column 0 1 1 0 r[1]= cursor 0 column 1 6 Goto 0 8 0 0 7 Column 0 2 1 0 r[1]= cursor 0 column 2 8 ResultRow 1 1 0 0 output=r[1] 9 Next 0 3 0 1 10 Halt 0 0 0 0 11 Transaction 0 0 1 0 1 usesStmtJournal=0 12 Goto 0 1 0 0 And with this change, in limbo it looks like: addr opcode p1 p2 p3 p4 p5 comment ---- ----------------- ---- ---- ---- ------------- -- ------- 0 Init 0 14 0 0 Start at 14 1 OpenReadAsync 0 2 0 0 table=iiftest, root=2 2 OpenReadAwait 0 0 0 0 3 RewindAsync 0 0 0 0 4 RewindAwait 0 13 0 0 Rewind table iiftest 5 Column 0 0 2 0 r[2]=iiftest.a 6 IfNot 2 9 1 0 if !r[2] goto 9 7 Column 0 1 1 0 r[1]=iiftest.b 8 Goto 0 10 0 0 9 Column 0 2 1 0 r[1]=iiftest.c 10 ResultRow 1 1 0 0 output=r[1] 11 NextAsync 0 0 0 0 12 NextAwait 0 5 0 0 13 Halt 0 0 0 0 14 Transaction 0 0 0 0 15 Goto 0 1 0 0 --- COMPAT.md | 2 +- core/function.rs | 3 ++ core/translate/expr.rs | 52 +++++++++++++++++++++++++++++++++++ core/vdbe/mod.rs | 1 + testing/scalar-functions.test | 8 ++++++ 5 files changed, 65 insertions(+), 1 deletion(-) diff --git a/COMPAT.md b/COMPAT.md index e909c794e..7501c5f7d 100644 --- a/COMPAT.md +++ b/COMPAT.md @@ -76,7 +76,7 @@ This document describes the SQLite compatibility status of Limbo: | glob(X,Y) | Yes | | | hex(X) | Yes | | | ifnull(X,Y) | Yes | | -| iif(X,Y,Z) | No | | +| iif(X,Y,Z) | Yes | | | instr(X,Y) | Yes | | | last_insert_rowid() | No | | | length(X) | Yes | | diff --git a/core/function.rs b/core/function.rs index c0baf807a..9591b1e01 100644 --- a/core/function.rs +++ b/core/function.rs @@ -56,6 +56,7 @@ pub enum ScalarFunc { ConcatWs, Glob, IfNull, + Iif, Instr, Like, Abs, @@ -96,6 +97,7 @@ impl Display for ScalarFunc { ScalarFunc::ConcatWs => "concat_ws".to_string(), ScalarFunc::Glob => "glob".to_string(), ScalarFunc::IfNull => "ifnull".to_string(), + ScalarFunc::Iif => "iif".to_string(), ScalarFunc::Instr => "instr".to_string(), ScalarFunc::Like => "like(2)".to_string(), ScalarFunc::Abs => "abs".to_string(), @@ -174,6 +176,7 @@ impl Func { "concat_ws" => Ok(Func::Scalar(ScalarFunc::ConcatWs)), "glob" => Ok(Func::Scalar(ScalarFunc::Glob)), "ifnull" => Ok(Func::Scalar(ScalarFunc::IfNull)), + "iif" => Ok(Func::Scalar(ScalarFunc::Iif)), "instr" => Ok(Func::Scalar(ScalarFunc::Instr)), "like" => Ok(Func::Scalar(ScalarFunc::Like)), "abs" => Ok(Func::Scalar(ScalarFunc::Abs)), diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 6c0b4437d..e7324db64 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -967,6 +967,58 @@ pub fn translate_expr( Ok(target_register) } + ScalarFunc::Iif => { + let args = if args.as_ref().map(|args| args.len() != 3).unwrap_or(true) + { + crate::bail_parse_error!( + "{} requires exactly 3 arguments", + srf.to_string() + ); + } else { + args.as_ref().unwrap() + }; + let temp_reg = program.alloc_register(); + translate_expr( + program, + referenced_tables, + &args[0], + temp_reg, + precomputed_exprs_to_registers, + )?; + let jump_target_when_false = program.allocate_label(); + program.emit_insn_with_label_dependency( + Insn::IfNot { + reg: temp_reg, + target_pc: jump_target_when_false, + null_reg: 1, + }, + jump_target_when_false, + ); + translate_expr( + program, + referenced_tables, + &args[1], + target_register, + precomputed_exprs_to_registers, + )?; + let jump_target_result = program.allocate_label(); + program.emit_insn_with_label_dependency( + Insn::Goto { + target_pc: jump_target_result, + }, + jump_target_result, + ); + program.preassign_label_to_next_insn(jump_target_when_false); + translate_expr( + program, + referenced_tables, + &args[2], + target_register, + precomputed_exprs_to_registers, + )?; + program.preassign_label_to_next_insn(jump_target_result); + Ok(target_register) + } ScalarFunc::Glob | ScalarFunc::Like => { let args = if let Some(args) = args { if args.len() < 2 { diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index 906f43f73..ff5134f81 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -2105,6 +2105,7 @@ impl Program { state.registers[*dest] = result; } ScalarFunc::IfNull => {} + ScalarFunc::Iif => {} ScalarFunc::Instr => { let reg_value = &state.registers[*start_reg]; let pattern_value = &state.registers[*start_reg + 1]; diff --git a/testing/scalar-functions.test b/testing/scalar-functions.test index 64a9b9e5f..b0488bfae 100755 --- a/testing/scalar-functions.test +++ b/testing/scalar-functions.test @@ -75,6 +75,14 @@ do_execsql_test ifnull-2 { select ifnull(null, 2); } {2} +do_execsql_test iif-1 { + select iif(1, 1, 0); +} {1} + +do_execsql_test iif-2 { + select iif(0, 1, 0); +} {0} + do_execsql_test instr-str { select instr('limbo', 'im'); } {2} From 27ef95e0ab9e5c6069680edce807cef1148f5036 Mon Sep 17 00:00:00 2001 From: Alex Miller Date: Sat, 7 Dec 2024 21:15:04 -0800 Subject: [PATCH 2/3] LLM recommended a better way to write args matching --- core/translate/expr.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index e7324db64..79a9d8ad1 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -968,14 +968,12 @@ pub fn translate_expr( Ok(target_register) } ScalarFunc::Iif => { - let args = if args.as_ref().map(|args| args.len() != 3).unwrap_or(true) - { - crate::bail_parse_error!( + let args = match args { + Some(args) if args.len() == 3 => args, + _ => crate::bail_parse_error!( "{} requires exactly 3 arguments", srf.to_string() - ); - } else { - args.as_ref().unwrap() + ), }; let temp_reg = program.alloc_register(); translate_expr( From e85df1c895e3b9bcf70927186cc62d3c39cf8fac Mon Sep 17 00:00:00 2001 From: Alex Miller Date: Tue, 10 Dec 2024 19:36:54 -0800 Subject: [PATCH 3/3] resolve labels to current offset. make test clearer. --- core/translate/expr.rs | 4 ++-- testing/scalar-functions.test | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 79a9d8ad1..6b1e81ce6 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -1006,7 +1006,7 @@ pub fn translate_expr( }, jump_target_result, ); - program.preassign_label_to_next_insn(jump_target_when_false); + program.resolve_label(jump_target_when_false, program.offset()); translate_expr( program, referenced_tables, @@ -1014,7 +1014,7 @@ pub fn translate_expr( target_register, precomputed_exprs_to_registers, )?; - program.preassign_label_to_next_insn(jump_target_result); + program.resolve_label(jump_target_result, program.offset()); Ok(target_register) } ScalarFunc::Glob | ScalarFunc::Like => { diff --git a/testing/scalar-functions.test b/testing/scalar-functions.test index b0488bfae..177fe88b6 100755 --- a/testing/scalar-functions.test +++ b/testing/scalar-functions.test @@ -75,13 +75,13 @@ do_execsql_test ifnull-2 { select ifnull(null, 2); } {2} -do_execsql_test iif-1 { - select iif(1, 1, 0); -} {1} +do_execsql_test iif-true { + select iif(1, 'pass', 'fail'); +} {pass} -do_execsql_test iif-2 { - select iif(0, 1, 0); -} {0} +do_execsql_test iif-false { + select iif(0, 'fail', 'pass'); +} {pass} do_execsql_test instr-str { select instr('limbo', 'im');