From da28ed51ca7c368bad96f54ebaf8d92a74de53a5 Mon Sep 17 00:00:00 2001 From: alpaylan Date: Wed, 11 Dec 2024 16:23:13 -0500 Subject: [PATCH 1/4] add implementation and tests for replace scalar function --- COMPAT.md | 3 +- core/function.rs | 3 + core/translate/expr.rs | 51 ++++++++++++ core/vdbe/mod.rs | 145 +++++++++++++++++++++++++++++++++- testing/scalar-functions.test | 12 +++ 5 files changed, 212 insertions(+), 2 deletions(-) diff --git a/COMPAT.md b/COMPAT.md index 0bacfc0e3..197f7ba18 100644 --- a/COMPAT.md +++ b/COMPAT.md @@ -5,6 +5,7 @@ This document describes the SQLite compatibility status of Limbo: - [SQLite Compatibility](#sqlite-compatibility) - [Limitations](#limitations) - [SQL statements](#sql-statements) + - [SELECT Expressions](#select-expressions) - [SQL functions](#sql-functions) - [Scalar functions](#scalar-functions) - [Aggregate functions](#aggregate-functions) @@ -132,7 +133,7 @@ Feature support of [sqlite expr syntax](https://www.sqlite.org/lang_expr.html). | quote(X) | Yes | | | random() | Yes | | | randomblob(N) | Yes | | -| replace(X,Y,Z) | No | | +| replace(X,Y,Z) | Yes | | | round(X) | Yes | | | round(X,Y) | Yes | | | rtrim(X) | Yes | | diff --git a/core/function.rs b/core/function.rs index 33ea43eb1..0f94feade 100644 --- a/core/function.rs +++ b/core/function.rs @@ -86,6 +86,7 @@ pub enum ScalarFunc { Unhex, ZeroBlob, LastInsertRowid, + Replace, } impl Display for ScalarFunc { @@ -128,6 +129,7 @@ impl Display for ScalarFunc { ScalarFunc::Unhex => "unhex".to_string(), ScalarFunc::ZeroBlob => "zeroblob".to_string(), ScalarFunc::LastInsertRowid => "last_insert_rowid".to_string(), + ScalarFunc::Replace => "replace".to_string(), }; write!(f, "{}", str) } @@ -201,6 +203,7 @@ impl Func { "unicode" => Ok(Func::Scalar(ScalarFunc::Unicode)), "quote" => Ok(Func::Scalar(ScalarFunc::Quote)), "sqlite_version" => Ok(Func::Scalar(ScalarFunc::SqliteVersion)), + "replace" => Ok(Func::Scalar(ScalarFunc::Replace)), #[cfg(feature = "json")] "json" => Ok(Func::Json(JsonFunc::Json)), "unixepoch" => Ok(Func::Scalar(ScalarFunc::UnixEpoch)), diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 0d84815a5..d14c0ace5 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -1485,6 +1485,57 @@ pub fn translate_expr( }); Ok(target_register) } + ScalarFunc::Replace => { + let args = if let Some(args) = args { + if !args.len() == 3 { + crate::bail_parse_error!( + "{} function with wrong number of arguments", + srf.to_string() + ) + } + args + } else { + crate::bail_parse_error!( + "{} function with no arguments", + srf.to_string() + ); + }; + + let str_reg = program.alloc_register(); + let pattern_reg = program.alloc_register(); + let replacement_reg = program.alloc_register(); + + translate_expr( + program, + referenced_tables, + &args[0], + str_reg, + precomputed_exprs_to_registers, + )?; + translate_expr( + program, + referenced_tables, + &args[1], + pattern_reg, + precomputed_exprs_to_registers, + )?; + + translate_expr( + program, + referenced_tables, + &args[2], + replacement_reg, + precomputed_exprs_to_registers, + )?; + + program.emit_insn(Insn::Function { + constant_mask: 0, + start_reg: str_reg, + dest: target_register, + func: func_ctx, + }); + Ok(target_register) + } } } } diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index ca6309a11..75ca713da 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -2273,6 +2273,13 @@ impl Program { let version = execute_sqlite_version(version_integer); state.registers[*dest] = OwnedValue::Text(Rc::new(version)); } + ScalarFunc::Replace => { + assert!(arg_count == 3); + let source = &state.registers[*start_reg]; + let pattern = &state.registers[*start_reg + 1]; + let replacement = &state.registers[*start_reg + 2]; + state.registers[*dest] = exec_replace(source, pattern, replacement); + } }, crate::function::Func::Agg(_) => { unreachable!("Aggregate functions should not be handled here") @@ -3122,6 +3129,37 @@ fn exec_cast(value: &OwnedValue, datatype: &str) -> OwnedValue { } } +fn exec_replace(source: &OwnedValue, pattern: &OwnedValue, replacement: &OwnedValue) -> OwnedValue { + // The replace(X,Y,Z) function returns a string formed by substituting string Z for every occurrence of + // string Y in string X. The BINARY collating sequence is used for comparisons. If Y is an empty string + // then return X unchanged. If Z is not initially a string, it is cast to a UTF-8 string prior to processing. + + // If any of the arguments is NULL, the result is NULL. + if matches!(source, OwnedValue::Null) + || matches!(pattern, OwnedValue::Null) + || matches!(replacement, OwnedValue::Null) + { + return OwnedValue::Null; + } + + let source = exec_cast(source, "TEXT"); + let pattern = exec_cast(pattern, "TEXT"); + let replacement = exec_cast(replacement, "TEXT"); + + // If any of the casts failed, return NULL + match (&source, &pattern, &replacement) { + (OwnedValue::Text(source), OwnedValue::Text(pattern), OwnedValue::Text(replacement)) => { + if pattern.is_empty() { + return OwnedValue::Text(source.clone()); + } + + let result = source.replace(pattern.as_str(), replacement); + OwnedValue::Text(Rc::new(result)) + } + _ => unreachable!("text cast should never fail"), + } +} + enum Affinity { Integer, Text, @@ -3249,7 +3287,10 @@ fn execute_sqlite_version(version_integer: i64) -> String { #[cfg(test)] mod tests { - use crate::types::{SeekKey, SeekOp}; + use crate::{ + types::{SeekKey, SeekOp}, + vdbe::exec_replace, + }; use super::{ exec_abs, exec_char, exec_hex, exec_if, exec_instr, exec_length, exec_like, exec_lower, @@ -4148,4 +4189,106 @@ mod tests { let expected = "3.46.1"; assert_eq!(execute_sqlite_version(version_integer), expected); } + + #[test] + fn test_replace() { + let input_str = OwnedValue::Text(Rc::new(String::from("bob"))); + let pattern_str = OwnedValue::Text(Rc::new(String::from("b"))); + let replace_str = OwnedValue::Text(Rc::new(String::from("a"))); + let expected_str = OwnedValue::Text(Rc::new(String::from("aoa"))); + assert_eq!( + exec_replace(&input_str, &pattern_str, &replace_str), + expected_str + ); + + let input_str = OwnedValue::Text(Rc::new(String::from("bob"))); + let pattern_str = OwnedValue::Text(Rc::new(String::from("b"))); + let replace_str = OwnedValue::Text(Rc::new(String::from(""))); + let expected_str = OwnedValue::Text(Rc::new(String::from("o"))); + assert_eq!( + exec_replace(&input_str, &pattern_str, &replace_str), + expected_str + ); + + let input_str = OwnedValue::Text(Rc::new(String::from("bob"))); + let pattern_str = OwnedValue::Text(Rc::new(String::from("b"))); + let replace_str = OwnedValue::Text(Rc::new(String::from("abc"))); + let expected_str = OwnedValue::Text(Rc::new(String::from("abcoabc"))); + assert_eq!( + exec_replace(&input_str, &pattern_str, &replace_str), + expected_str + ); + + let input_str = OwnedValue::Text(Rc::new(String::from("bob"))); + let pattern_str = OwnedValue::Text(Rc::new(String::from("a"))); + let replace_str = OwnedValue::Text(Rc::new(String::from("b"))); + let expected_str = OwnedValue::Text(Rc::new(String::from("bob"))); + assert_eq!( + exec_replace(&input_str, &pattern_str, &replace_str), + expected_str + ); + + let input_str = OwnedValue::Text(Rc::new(String::from("bob"))); + let pattern_str = OwnedValue::Text(Rc::new(String::from(""))); + let replace_str = OwnedValue::Text(Rc::new(String::from("a"))); + let expected_str = OwnedValue::Text(Rc::new(String::from("bob"))); + assert_eq!( + exec_replace(&input_str, &pattern_str, &replace_str), + expected_str + ); + + let input_str = OwnedValue::Text(Rc::new(String::from("bob"))); + let pattern_str = OwnedValue::Null; + let replace_str = OwnedValue::Text(Rc::new(String::from("a"))); + let expected_str = OwnedValue::Null; + assert_eq!( + exec_replace(&input_str, &pattern_str, &replace_str), + expected_str + ); + + let input_str = OwnedValue::Text(Rc::new(String::from("bo5"))); + let pattern_str = OwnedValue::Integer(5); + let replace_str = OwnedValue::Text(Rc::new(String::from("a"))); + let expected_str = OwnedValue::Text(Rc::new(String::from("boa"))); + assert_eq!( + exec_replace(&input_str, &pattern_str, &replace_str), + expected_str + ); + + let input_str = OwnedValue::Text(Rc::new(String::from("bo5.0"))); + let pattern_str = OwnedValue::Float(5.0); + let replace_str = OwnedValue::Text(Rc::new(String::from("a"))); + let expected_str = OwnedValue::Text(Rc::new(String::from("boa"))); + assert_eq!( + exec_replace(&input_str, &pattern_str, &replace_str), + expected_str + ); + + let input_str = OwnedValue::Text(Rc::new(String::from("bo5"))); + let pattern_str = OwnedValue::Float(5.0); + let replace_str = OwnedValue::Text(Rc::new(String::from("a"))); + let expected_str = OwnedValue::Text(Rc::new(String::from("bo5"))); + assert_eq!( + exec_replace(&input_str, &pattern_str, &replace_str), + expected_str + ); + + let input_str = OwnedValue::Text(Rc::new(String::from("bo5.0"))); + let pattern_str = OwnedValue::Float(5.0); + let replace_str = OwnedValue::Float(6.0); + let expected_str = OwnedValue::Text(Rc::new(String::from("bo6.0"))); + assert_eq!( + exec_replace(&input_str, &pattern_str, &replace_str), + expected_str + ); + + // let input_str = OwnedValue::Text(Rc::new(String::from("tes3"))); + // let pattern_str = OwnedValue::Integer(3); + // let replace_str = OwnedValue::Agg(Box::new(AggContext::Sum(OwnedValue::Float(0.1 + 0.2)))); + // let expected_str = OwnedValue::Text(Rc::new(String::from("tes0.3"))); + // assert_eq!( + // exec_replace(&input_str, &pattern_str, &replace_str), + // expected_str + // ); + } } diff --git a/testing/scalar-functions.test b/testing/scalar-functions.test index e0ff8c9f1..5dcec1e8f 100755 --- a/testing/scalar-functions.test +++ b/testing/scalar-functions.test @@ -159,6 +159,18 @@ do_execsql_test lower-null { select lower(null) } {} +do_execsql_test replace { + select replace('test', 'test', 'example') +} {example} + +do_execsql_test replace-number { + select replace('tes3', 3, 0.1 + 0.2) +} {tes0.3} + +do_execsql_test replace-null { + select replace('test', null, 'example') +} {} + do_execsql_test hex { select hex('limbo') } {6C696D626F} From 021456326efb3a4adda91a8572ad2ededec0fa49 Mon Sep 17 00:00:00 2001 From: alpaylan Date: Wed, 11 Dec 2024 16:32:06 -0500 Subject: [PATCH 2/4] change 0.1+0.2 test into 0.3 as limbo does not yet support decimals --- core/vdbe/mod.rs | 17 +++++++++-------- testing/scalar-functions.test | 2 +- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index 75ca713da..23f7d3bd6 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -4282,13 +4282,14 @@ mod tests { expected_str ); - // let input_str = OwnedValue::Text(Rc::new(String::from("tes3"))); - // let pattern_str = OwnedValue::Integer(3); - // let replace_str = OwnedValue::Agg(Box::new(AggContext::Sum(OwnedValue::Float(0.1 + 0.2)))); - // let expected_str = OwnedValue::Text(Rc::new(String::from("tes0.3"))); - // assert_eq!( - // exec_replace(&input_str, &pattern_str, &replace_str), - // expected_str - // ); + // todo: change this test to use (0.1 + 0.2) instead of 0.3 when decimals are implemented. + let input_str = OwnedValue::Text(Rc::new(String::from("tes3"))); + let pattern_str = OwnedValue::Integer(3); + let replace_str = OwnedValue::Agg(Box::new(AggContext::Sum(OwnedValue::Float(0.3)))); + let expected_str = OwnedValue::Text(Rc::new(String::from("tes0.3"))); + assert_eq!( + exec_replace(&input_str, &pattern_str, &replace_str), + expected_str + ); } } diff --git a/testing/scalar-functions.test b/testing/scalar-functions.test index 5dcec1e8f..c2be07fb9 100755 --- a/testing/scalar-functions.test +++ b/testing/scalar-functions.test @@ -164,7 +164,7 @@ do_execsql_test replace { } {example} do_execsql_test replace-number { - select replace('tes3', 3, 0.1 + 0.2) + select replace('tes3', 3, 0.3) } {tes0.3} do_execsql_test replace-null { From 5742c51c5ce04a778d1c6c60f6da111dda499a55 Mon Sep 17 00:00:00 2001 From: alpaylan Date: Thu, 12 Dec 2024 14:14:04 -0500 Subject: [PATCH 3/4] improve error messages on replace function call --- core/translate/expr.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 1868b5e11..21b6de5e3 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -1554,14 +1554,14 @@ pub fn translate_expr( let args = if let Some(args) = args { if !args.len() == 3 { crate::bail_parse_error!( - "{} function with wrong number of arguments", + "function {}() requires exactly 3 arguments", srf.to_string() ) } args } else { crate::bail_parse_error!( - "{} function with no arguments", + "function {}() requires exactly 3 arguments", srf.to_string() ); }; From 4f395623ab62ed0e67660d277a47b83193220c84 Mon Sep 17 00:00:00 2001 From: alpaylan Date: Thu, 12 Dec 2024 15:03:35 -0500 Subject: [PATCH 4/4] fix the comment on replace panic in case of text cast failure --- core/vdbe/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index d319b655d..e576f1cb7 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -3446,7 +3446,7 @@ fn exec_replace(source: &OwnedValue, pattern: &OwnedValue, replacement: &OwnedVa let pattern = exec_cast(pattern, "TEXT"); let replacement = exec_cast(replacement, "TEXT"); - // If any of the casts failed, return NULL + // If any of the casts failed, panic as text casting is not expected to fail. match (&source, &pattern, &replacement) { (OwnedValue::Text(source), OwnedValue::Text(pattern), OwnedValue::Text(replacement)) => { if pattern.is_empty() {