From 73764e198e8dafef9e7f55ca2d26c704755143e2 Mon Sep 17 00:00:00 2001 From: Diego Reis Date: Sat, 12 Apr 2025 16:34:39 -0300 Subject: [PATCH 1/3] core: Fix equivalence between variable expressions to be always false Since until the bind to a value they are treated as NULL. https://sqlite.org/lang_expr.html#varparam --- core/util.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/core/util.rs b/core/util.rs index 3d12a2c6e..aa13e2edc 100644 --- a/core/util.rs +++ b/core/util.rs @@ -399,7 +399,9 @@ pub fn exprs_are_equivalent(expr1: &Expr, expr2: &Expr) -> bool { (Expr::Unary(op1, expr1), Expr::Unary(op2, expr2)) => { op1 == op2 && exprs_are_equivalent(expr1, expr2) } - (Expr::Variable(var1), Expr::Variable(var2)) => var1 == var2, + // Variables that are not bound to a specific value, are treated as NULL + // https://sqlite.org/lang_expr.html#varparam + (Expr::Variable(..), Expr::Variable(..)) => false, (Expr::Parenthesized(exprs1), Expr::Parenthesized(exprs2)) => { exprs1.len() == exprs2.len() && exprs1 @@ -945,6 +947,13 @@ pub mod tests { assert_eq!(normalize_ident("\"foo\""), "foo"); } + #[test] + fn test_variable_comparison() { + let expr1 = Expr::Variable("?".to_string()); + let expr2 = Expr::Variable("?".to_string()); + assert!(!exprs_are_equivalent(&expr1, &expr2)); + } + #[test] fn test_basic_addition_exprs_are_equivalent() { let expr1 = Expr::Binary( From db0f07499da129d447b93c7faca5c8217bff3b5c Mon Sep 17 00:00:00 2001 From: Diego Reis Date: Sat, 12 Apr 2025 16:39:30 -0300 Subject: [PATCH 2/3] core/translate: Fix naive comparison between Binary expressions during register optimization --- core/translate/expr.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 958005259..6a9b7040b 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -4,7 +4,7 @@ use limbo_sqlite3_parser::ast::{self, UnaryOperator}; use crate::function::JsonFunc; use crate::function::{Func, FuncCtx, MathFuncArity, ScalarFunc, VectorFunc}; use crate::schema::{Table, Type}; -use crate::util::normalize_ident; +use crate::util::{exprs_are_equivalent, normalize_ident}; use crate::vdbe::{ builder::ProgramBuilder, insn::{CmpInsFlags, Insn}, @@ -494,8 +494,8 @@ pub fn translate_expr( match expr { ast::Expr::Between { .. } => todo!(), ast::Expr::Binary(e1, op, e2) => { - // Check if both sides of the expression are identical and reuse the same register if so - if e1 == e2 { + // Check if both sides of the expression are equivalent and reuse the same register if so + if exprs_are_equivalent(e1, e2) { let shared_reg = program.alloc_register(); translate_expr(program, referenced_tables, e1, shared_reg, resolver)?; From ff7a4e8297164455b9b5708298cbffc7acd56db8 Mon Sep 17 00:00:00 2001 From: Diego Reis Date: Sat, 12 Apr 2025 16:51:47 -0300 Subject: [PATCH 3/3] core: Change always falseness of equivalence between variables expressions to be only on anonymous variables Named variables are compared by name --- core/util.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/core/util.rs b/core/util.rs index aa13e2edc..80a52f387 100644 --- a/core/util.rs +++ b/core/util.rs @@ -401,7 +401,9 @@ pub fn exprs_are_equivalent(expr1: &Expr, expr2: &Expr) -> bool { } // Variables that are not bound to a specific value, are treated as NULL // https://sqlite.org/lang_expr.html#varparam - (Expr::Variable(..), Expr::Variable(..)) => false, + (Expr::Variable(var), Expr::Variable(var2)) if var == "" && var2 == "" => false, + // Named variables can be compared by their name + (Expr::Variable(val), Expr::Variable(val2)) => val == val2, (Expr::Parenthesized(exprs1), Expr::Parenthesized(exprs2)) => { exprs1.len() == exprs2.len() && exprs1 @@ -948,9 +950,20 @@ pub mod tests { } #[test] - fn test_variable_comparison() { - let expr1 = Expr::Variable("?".to_string()); - let expr2 = Expr::Variable("?".to_string()); + fn test_anonymous_variable_comparison() { + let expr1 = Expr::Variable("".to_string()); + let expr2 = Expr::Variable("".to_string()); + assert!(!exprs_are_equivalent(&expr1, &expr2)); + } + + #[test] + fn test_named_variable_comparison() { + let expr1 = Expr::Variable("1".to_string()); + let expr2 = Expr::Variable("1".to_string()); + assert!(exprs_are_equivalent(&expr1, &expr2)); + + let expr1 = Expr::Variable("1".to_string()); + let expr2 = Expr::Variable("2".to_string()); assert!(!exprs_are_equivalent(&expr1, &expr2)); }