From 554244209d3d3b3a28d7483b206596f19a72831d Mon Sep 17 00:00:00 2001 From: psvri Date: Sun, 19 Jan 2025 00:10:34 +0530 Subject: [PATCH 1/2] Add Null comparision tests --- testing/compare.test | 72 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/testing/compare.test b/testing/compare.test index a9f6bb477..54633c93d 100644 --- a/testing/compare.test +++ b/testing/compare.test @@ -6,6 +6,7 @@ source $testdir/tester.tcl foreach {testname lhs rhs ans} { int-int-1 8 1 0 int-int-2 8 8 1 + int-null 8 NULL {} } { do_execsql_test compare-eq-$testname "SELECT $lhs = $rhs" $::ans } @@ -13,6 +14,7 @@ foreach {testname lhs rhs ans} { foreach {testname lhs rhs ans} { float-float-1 8.0 1.0 0 float-float-2 8.0 8.0 1 + float-null 8.0 NULL {} } { do_execsql_test compare-eq-$testname "SELECT $lhs = $rhs" $::ans } @@ -20,6 +22,16 @@ foreach {testname lhs rhs ans} { foreach {testname lhs rhs ans} { text-text-1 'a' 'b' 0 text-text-2 'a' 'a' 1 + text-null 'a' NULL {} +} { + do_execsql_test compare-eq-$testname "SELECT $lhs = $rhs" $::ans +} + +foreach {testname lhs rhs ans} { + null-int NULL 1 {} + null-float NULL 1.0 {} + null-text NULL 'a' {} + null-null NULL NULL {} } { do_execsql_test compare-eq-$testname "SELECT $lhs = $rhs" $::ans } @@ -27,6 +39,7 @@ foreach {testname lhs rhs ans} { foreach {testname lhs rhs ans} { int-int-1 8 1 1 int-int-2 8 8 0 + int-null 8 NULL {} } { do_execsql_test compare-neq-$testname "SELECT $lhs <> $rhs" $::ans } @@ -34,6 +47,7 @@ foreach {testname lhs rhs ans} { foreach {testname lhs rhs ans} { float-float-1 8.0 1.0 1 float-float-2 8.0 8.0 0 + float-null 8.0 NULL {} } { do_execsql_test compare-neq-$testname "SELECT $lhs <> $rhs" $::ans } @@ -41,6 +55,16 @@ foreach {testname lhs rhs ans} { foreach {testname lhs rhs ans} { text-text-1 'a' 'b' 1 text-text-2 'a' 'a' 0 + text-null 'a' NULL {} +} { + do_execsql_test compare-neq-$testname "SELECT $lhs <> $rhs" $::ans +} + +foreach {testname lhs rhs ans} { + null-int NULL 1 {} + null-float NULL 1.0 {} + null-text NULL 'a' {} + null-null NULL NULL {} } { do_execsql_test compare-neq-$testname "SELECT $lhs <> $rhs" $::ans } @@ -49,6 +73,7 @@ foreach {testname lhs rhs ans} { int-int-1 1 8 0 int-int-2 1 1 0 int-int-3 8 0 1 + int-null 8 NULL {} } { do_execsql_test compare-gt-$testname "SELECT $lhs > $rhs" $::ans } @@ -57,6 +82,7 @@ foreach {testname lhs rhs ans} { float-float-1 1.0 2.0 0 float-float-2 1.0 1.0 0 float-float-3 7.0 6.0 1 + float-null 8.0 NULL {} } { do_execsql_test compare-gt-$testname "SELECT $lhs > $rhs" $::ans } @@ -65,6 +91,16 @@ foreach {testname lhs rhs ans} { text-text-1 'b' 'c' 0 text-text-2 'b' 'b' 0 text-text-3 'b' 'a' 1 + text-null 'a' NULL {} +} { + do_execsql_test compare-gt-$testname "SELECT $lhs > $rhs" $::ans +} + +foreach {testname lhs rhs ans} { + null-int NULL 1 {} + null-float NULL 1.0 {} + null-text NULL 'a' {} + null-null NULL NULL {} } { do_execsql_test compare-gt-$testname "SELECT $lhs > $rhs" $::ans } @@ -73,6 +109,7 @@ foreach {testname lhs rhs ans} { int-int-1 1 8 0 int-int-2 1 1 1 int-int-3 8 0 1 + int-null 8 NULL {} } { do_execsql_test compare-gte-$testname "SELECT $lhs >= $rhs" $::ans } @@ -81,6 +118,7 @@ foreach {testname lhs rhs ans} { float-float-1 1.0 2.0 0 float-float-2 1.0 1.0 1 float-float-3 7.0 6.0 1 + float-null 8.0 NULL {} } { do_execsql_test compare-gte-$testname "SELECT $lhs >= $rhs" $::ans } @@ -89,6 +127,16 @@ foreach {testname lhs rhs ans} { text-text-1 'b' 'c' 0 text-text-2 'b' 'b' 1 text-text-3 'b' 'a' 1 + text-null 'a' NULL {} +} { + do_execsql_test compare-gte-$testname "SELECT $lhs >= $rhs" $::ans +} + +foreach {testname lhs rhs ans} { + null-int NULL 1 {} + null-float NULL 1.0 {} + null-text NULL 'a' {} + null-null NULL NULL {} } { do_execsql_test compare-gte-$testname "SELECT $lhs >= $rhs" $::ans } @@ -97,6 +145,7 @@ foreach {testname lhs rhs ans} { int-int-1 1 8 1 int-int-2 1 1 0 int-int-3 8 0 0 + int-null 8 NULL {} } { do_execsql_test compare-lt-$testname "SELECT $lhs < $rhs" $::ans } @@ -105,6 +154,7 @@ foreach {testname lhs rhs ans} { float-float-1 1.0 2.0 1 float-float-2 1.0 1.0 0 float-float-3 7.0 6.0 0 + float-null 8.0 NULL {} } { do_execsql_test compare-lt-$testname "SELECT $lhs < $rhs" $::ans } @@ -113,6 +163,16 @@ foreach {testname lhs rhs ans} { text-text-1 'b' 'c' 1 text-text-2 'b' 'b' 0 text-text-3 'b' 'a' 0 + text-null 'a' NULL {} +} { + do_execsql_test compare-lt-$testname "SELECT $lhs < $rhs" $::ans +} + +foreach {testname lhs rhs ans} { + null-int NULL 1 {} + null-float NULL 1.0 {} + null-text NULL 'a' {} + null-null NULL NULL {} } { do_execsql_test compare-lt-$testname "SELECT $lhs < $rhs" $::ans } @@ -121,6 +181,7 @@ foreach {testname lhs rhs ans} { int-int-1 1 8 1 int-int-2 1 1 1 int-int-3 8 0 0 + int-null 8 NULL {} } { do_execsql_test compare-lte-$testname "SELECT $lhs <= $rhs" $::ans } @@ -129,6 +190,7 @@ foreach {testname lhs rhs ans} { float-float-1 1.0 2.0 1 float-float-2 1.0 1.0 1 float-float-3 7.0 6.0 0 + float-null 8.0 NULL {} } { do_execsql_test compare-lte-$testname "SELECT $lhs <= $rhs" $::ans } @@ -137,6 +199,16 @@ foreach {testname lhs rhs ans} { text-text-1 'b' 'c' 1 text-text-2 'b' 'b' 1 text-text-3 'b' 'a' 0 + text-null 'a' NULL {} +} { + do_execsql_test compare-lte-$testname "SELECT $lhs <= $rhs" $::ans +} + +foreach {testname lhs rhs ans} { + null-int NULL 1 {} + null-float NULL 1.0 {} + null-text NULL 'a' {} + null-null NULL NULL {} } { do_execsql_test compare-lte-$testname "SELECT $lhs <= $rhs" $::ans } From e16b3491c491277da215c183280478db13f6480b Mon Sep 17 00:00:00 2001 From: psvri Date: Sun, 19 Jan 2025 00:44:38 +0530 Subject: [PATCH 2/2] Fix null compares giving incorrect results --- COMPAT.md | 1 + core/translate/expr.rs | 45 ++++++++++++++++++++++++++++++++++++------ core/vdbe/explain.rs | 12 +++++++++++ core/vdbe/insn.rs | 7 +++++++ core/vdbe/mod.rs | 22 +++++++++++++++------ 5 files changed, 75 insertions(+), 12 deletions(-) diff --git a/COMPAT.md b/COMPAT.md index 07e9ccf2f..d7d059264 100644 --- a/COMPAT.md +++ b/COMPAT.md @@ -483,6 +483,7 @@ Modifiers: | Variable | No | | VerifyCookie | No | | Yield | Yes | +| ZeroOrNull | Yes | ## Extensions diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 3cb2a631a..0de0d7b05 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -453,7 +453,7 @@ pub fn translate_expr( match op { ast::Operator::NotEquals => { let if_true_label = program.allocate_label(); - wrap_eval_jump_expr( + wrap_eval_jump_expr_zero_or_null( program, Insn::Ne { lhs: e1_reg, @@ -462,11 +462,13 @@ pub fn translate_expr( }, target_register, if_true_label, + e1_reg, + e2_reg, ); } ast::Operator::Equals => { let if_true_label = program.allocate_label(); - wrap_eval_jump_expr( + wrap_eval_jump_expr_zero_or_null( program, Insn::Eq { lhs: e1_reg, @@ -475,11 +477,13 @@ pub fn translate_expr( }, target_register, if_true_label, + e1_reg, + e2_reg, ); } ast::Operator::Less => { let if_true_label = program.allocate_label(); - wrap_eval_jump_expr( + wrap_eval_jump_expr_zero_or_null( program, Insn::Lt { lhs: e1_reg, @@ -488,11 +492,13 @@ pub fn translate_expr( }, target_register, if_true_label, + e1_reg, + e2_reg, ); } ast::Operator::LessEquals => { let if_true_label = program.allocate_label(); - wrap_eval_jump_expr( + wrap_eval_jump_expr_zero_or_null( program, Insn::Le { lhs: e1_reg, @@ -501,11 +507,13 @@ pub fn translate_expr( }, target_register, if_true_label, + e1_reg, + e2_reg, ); } ast::Operator::Greater => { let if_true_label = program.allocate_label(); - wrap_eval_jump_expr( + wrap_eval_jump_expr_zero_or_null( program, Insn::Gt { lhs: e1_reg, @@ -514,11 +522,13 @@ pub fn translate_expr( }, target_register, if_true_label, + e1_reg, + e2_reg, ); } ast::Operator::GreaterEquals => { let if_true_label = program.allocate_label(); - wrap_eval_jump_expr( + wrap_eval_jump_expr_zero_or_null( program, Insn::Ge { lhs: e1_reg, @@ -527,6 +537,8 @@ pub fn translate_expr( }, target_register, if_true_label, + e1_reg, + e2_reg, ); } ast::Operator::Add => { @@ -1796,6 +1808,27 @@ fn wrap_eval_jump_expr( program.preassign_label_to_next_insn(if_true_label); } +fn wrap_eval_jump_expr_zero_or_null( + program: &mut ProgramBuilder, + insn: Insn, + target_register: usize, + if_true_label: BranchOffset, + e1_reg: usize, + e2_reg: usize, +) { + program.emit_insn(Insn::Integer { + value: 1, // emit True by default + dest: target_register, + }); + program.emit_insn(insn); + program.emit_insn(Insn::ZeroOrNull { + rg1: e1_reg, + rg2: e2_reg, + dest: target_register, + }); + program.preassign_label_to_next_insn(if_true_label); +} + pub fn maybe_apply_affinity(col_type: Type, target_register: usize, program: &mut ProgramBuilder) { if col_type == crate::schema::Type::Real { program.emit_insn(Insn::RealAffinity { diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index 22b154809..8ab330f24 100644 --- a/core/vdbe/explain.rs +++ b/core/vdbe/explain.rs @@ -1071,6 +1071,18 @@ pub fn insn_to_str( 0, format!("r[{}]=parameter({})", *dest, *index), ), + Insn::ZeroOrNull { rg1, rg2, dest } => ( + "ZeroOrNull", + *rg1 as i32, + *dest as i32, + *rg2 as i32, + OwnedValue::build_text(Rc::new("".to_string())), + 0, + format!( + "((r[{}]=NULL)|(r[{}]=NULL)) ? r[{}]=NULL : r[{}]=0", + rg1, rg2, dest, dest + ), + ), }; format!( "{:<4} {:<17} {:<4} {:<4} {:<4} {:<13} {:<2} {}", diff --git a/core/vdbe/insn.rs b/core/vdbe/insn.rs index 3066a399c..8d812846b 100644 --- a/core/vdbe/insn.rs +++ b/core/vdbe/insn.rs @@ -509,6 +509,13 @@ pub enum Insn { index: NonZero, dest: usize, }, + /// If either register is null put null else put 0 + ZeroOrNull { + /// Source register (P1). + rg1: usize, + rg2: usize, + dest: usize, + }, } fn cast_text_to_numerical(value: &str) -> OwnedValue { diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index 63716d87e..de63812fd 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -495,7 +495,7 @@ impl Program { let target_pc = *target_pc; match (&state.registers[lhs], &state.registers[rhs]) { (_, OwnedValue::Null) | (OwnedValue::Null, _) => { - state.pc = target_pc.to_offset_int(); + state.pc += 1; } _ => { if state.registers[lhs] == state.registers[rhs] { @@ -517,7 +517,7 @@ impl Program { let target_pc = *target_pc; match (&state.registers[lhs], &state.registers[rhs]) { (_, OwnedValue::Null) | (OwnedValue::Null, _) => { - state.pc = target_pc.to_offset_int(); + state.pc += 1; } _ => { if state.registers[lhs] != state.registers[rhs] { @@ -539,7 +539,7 @@ impl Program { let target_pc = *target_pc; match (&state.registers[lhs], &state.registers[rhs]) { (_, OwnedValue::Null) | (OwnedValue::Null, _) => { - state.pc = target_pc.to_offset_int(); + state.pc += 1; } _ => { if state.registers[lhs] < state.registers[rhs] { @@ -561,7 +561,7 @@ impl Program { let target_pc = *target_pc; match (&state.registers[lhs], &state.registers[rhs]) { (_, OwnedValue::Null) | (OwnedValue::Null, _) => { - state.pc = target_pc.to_offset_int(); + state.pc += 1; } _ => { if state.registers[lhs] <= state.registers[rhs] { @@ -583,7 +583,7 @@ impl Program { let target_pc = *target_pc; match (&state.registers[lhs], &state.registers[rhs]) { (_, OwnedValue::Null) | (OwnedValue::Null, _) => { - state.pc = target_pc.to_offset_int(); + state.pc += 1; } _ => { if state.registers[lhs] > state.registers[rhs] { @@ -605,7 +605,7 @@ impl Program { let target_pc = *target_pc; match (&state.registers[lhs], &state.registers[rhs]) { (_, OwnedValue::Null) | (OwnedValue::Null, _) => { - state.pc = target_pc.to_offset_int(); + state.pc += 1; } _ => { if state.registers[lhs] >= state.registers[rhs] { @@ -2250,6 +2250,16 @@ impl Program { .clone(); state.pc += 1; } + Insn::ZeroOrNull { rg1, rg2, dest } => { + if state.registers[*rg1] == OwnedValue::Null + || state.registers[*rg2] == OwnedValue::Null + { + state.registers[*dest] = OwnedValue::Null + } else { + state.registers[*dest] = OwnedValue::Integer(0); + } + state.pc += 1; + } } } }