From da53cc3821b87c9667ef18c3d240c1a485d5096e Mon Sep 17 00:00:00 2001 From: Harin Date: Mon, 20 Jan 2025 23:49:50 +0530 Subject: [PATCH 1/2] Added Concat Opcode --- COMPAT.md | 2 +- core/translate/expr.rs | 7 +++++++ core/types.rs | 12 ++++++++++++ core/vdbe/explain.rs | 9 +++++++++ core/vdbe/insn.rs | 19 +++++++++++++++++++ core/vdbe/mod.rs | 9 +++++++-- testing/concat.test | 25 +++++++++++++++++++++++++ 7 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 testing/concat.test diff --git a/COMPAT.md b/COMPAT.md index 86ab1975..2383612f 100644 --- a/COMPAT.md +++ b/COMPAT.md @@ -332,7 +332,7 @@ Modifiers: | CollSeq | No | | Column | Yes | | Compare | Yes | -| Concat | No | +| Concat | Yes | | Copy | Yes | | Count | No | | CreateIndex | No | diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 349a5f2b..476b78d7 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -679,6 +679,13 @@ pub fn translate_expr( }, }) } + ast::Operator::Concat => { + program.emit_insn(Insn::Concat { + lhs: e1_reg, + rhs: e2_reg, + dest: target_register, + }); + } other_unimplemented => todo!("{:?}", other_unimplemented), } Ok(target_register) diff --git a/core/types.rs b/core/types.rs index a3c31e1d..705ed993 100644 --- a/core/types.rs +++ b/core/types.rs @@ -72,6 +72,18 @@ impl OwnedValue { pub fn build_text(text: Rc) -> Self { Self::Text(LimboText::new(text)) } + + pub fn value_to_string(value: &OwnedValue) -> String { + match value { + OwnedValue::Text(text) => text.value.as_ref().clone(), + OwnedValue::Integer(i) => i.to_string(), + OwnedValue::Float(f) => f.to_string(), + OwnedValue::Agg(aggctx) => aggctx.final_value().to_string(), + OwnedValue::Null => String::new(), + OwnedValue::Blob(_) => todo!("TODO: Handle Blob conversion to String"), + OwnedValue::Record(_) => unreachable!(), + } + } } #[derive(Debug, Clone, PartialEq)] diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index 1264f918..67f00823 100644 --- a/core/vdbe/explain.rs +++ b/core/vdbe/explain.rs @@ -1111,6 +1111,15 @@ pub fn insn_to_str( 0, format!("r[{}]=!r[{}]", dest, reg), ), + Insn::Concat { lhs, rhs, dest } => ( + "Concat", + *rhs as i32, + *lhs as i32, + *dest as i32, + OwnedValue::build_text(Rc::new("".to_string())), + 0, + format!("r[{}]=r[{}] << r[{}]", dest, lhs, rhs) + ), }; format!( "{:<4} {:<17} {:<4} {:<4} {:<4} {:<13} {:<2} {}", diff --git a/core/vdbe/insn.rs b/core/vdbe/insn.rs index 399aea8a..3f8baf88 100644 --- a/core/vdbe/insn.rs +++ b/core/vdbe/insn.rs @@ -1,4 +1,5 @@ use std::num::NonZero; +use std::rc::Rc; use super::{AggFunc, BranchOffset, CursorID, FuncCtx, PageIdx}; use crate::storage::wal::CheckpointMode; @@ -544,6 +545,12 @@ pub enum Insn { reg: usize, dest: usize, }, + /// Concatenates the `rhs` and `lhs` values and stores the result in the third register. + Concat { + lhs: usize, + rhs: usize, + dest: usize, + }, } fn cast_text_to_numerical(value: &str) -> OwnedValue { @@ -886,3 +893,15 @@ pub fn exec_boolean_not(mut reg: &OwnedValue) -> OwnedValue { _ => todo!(), } } + +pub fn exec_concat(lhs: &OwnedValue, rhs: &OwnedValue) -> OwnedValue { + let lhs_value = OwnedValue::value_to_string(lhs); + let rhs_value = OwnedValue::value_to_string(rhs); + + if lhs_value.is_empty() || rhs_value.is_empty() { + OwnedValue::Null + } else { + let result = lhs_value + &rhs_value; + OwnedValue::build_text(Rc::new(result)) + } +} diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index b8c5f947..5c8694be 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -48,6 +48,7 @@ use datetime::{exec_date, exec_datetime_full, exec_julianday, exec_time, exec_un use insn::{ exec_add, exec_bit_and, exec_bit_not, exec_bit_or, exec_boolean_not, exec_divide, exec_multiply, exec_remainder, exec_shift_left, exec_shift_right, exec_subtract, + exec_concat }; use likeop::{construct_like_escape_arg, exec_glob, exec_like_with_escape}; use rand::distributions::{Distribution, Uniform}; @@ -1724,7 +1725,7 @@ impl Program { } ScalarFunc::Coalesce => {} ScalarFunc::Concat => { - let result = exec_concat( + let result = exec_concat_strings( &state.registers[*start_reg..*start_reg + arg_count], ); state.registers[*dest] = result; @@ -2341,6 +2342,10 @@ impl Program { state.registers[*dest] = exec_boolean_not(&state.registers[*reg]); state.pc += 1; } + Insn::Concat { lhs, rhs, dest } => { + state.registers[*dest] = exec_concat(&state.registers[*lhs], &state.registers[*rhs]); + state.pc += 1; + } } } } @@ -2476,7 +2481,7 @@ fn exec_upper(reg: &OwnedValue) -> Option { } } -fn exec_concat(registers: &[OwnedValue]) -> OwnedValue { +fn exec_concat_strings(registers: &[OwnedValue]) -> OwnedValue { let mut result = String::new(); for reg in registers { match reg { diff --git a/testing/concat.test b/testing/concat.test new file mode 100644 index 00000000..ee8127b2 --- /dev/null +++ b/testing/concat.test @@ -0,0 +1,25 @@ +#!/usr/bin/env tclsh + +set testdir [file dirname $argv0] +source $testdir/tester.tcl + +do_execsql_test concat { + SELECT 'Hello' || ' ' || 'World'; +} {"Hello World"} + +do_execsql_test concat-2 { + SELECT 'Hello' || NULL; +} {} + +do_execsql_test concat-3 { + SELECT 'Hello' || NULL; +} {} + +do_execsql_test concat-4 { + SELECT 'A good ' || name FROM products WHERE name = 'hat'; +} {"A good hat"} + + +do_execsql_test concat-4 { + SELECT 'A good ' || name FROM products WHERE name = 'watch'; +} {} From ab3a15e48906a2f6b030b70f8a036067d60cee80 Mon Sep 17 00:00:00 2001 From: Harin Date: Tue, 21 Jan 2025 10:02:19 +0530 Subject: [PATCH 2/2] Code refactor --- core/types.rs | 12 --------- core/vdbe/explain.rs | 2 +- core/vdbe/insn.rs | 64 +++++++++++++++++++++++++++++++++++++++----- core/vdbe/mod.rs | 6 ++--- 4 files changed, 61 insertions(+), 23 deletions(-) diff --git a/core/types.rs b/core/types.rs index 705ed993..a3c31e1d 100644 --- a/core/types.rs +++ b/core/types.rs @@ -72,18 +72,6 @@ impl OwnedValue { pub fn build_text(text: Rc) -> Self { Self::Text(LimboText::new(text)) } - - pub fn value_to_string(value: &OwnedValue) -> String { - match value { - OwnedValue::Text(text) => text.value.as_ref().clone(), - OwnedValue::Integer(i) => i.to_string(), - OwnedValue::Float(f) => f.to_string(), - OwnedValue::Agg(aggctx) => aggctx.final_value().to_string(), - OwnedValue::Null => String::new(), - OwnedValue::Blob(_) => todo!("TODO: Handle Blob conversion to String"), - OwnedValue::Record(_) => unreachable!(), - } - } } #[derive(Debug, Clone, PartialEq)] diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index 67f00823..80f419ed 100644 --- a/core/vdbe/explain.rs +++ b/core/vdbe/explain.rs @@ -1118,7 +1118,7 @@ pub fn insn_to_str( *dest as i32, OwnedValue::build_text(Rc::new("".to_string())), 0, - format!("r[{}]=r[{}] << r[{}]", dest, lhs, rhs) + format!("r[{}]=r[{}] + r[{}]", dest, lhs, rhs), ), }; format!( diff --git a/core/vdbe/insn.rs b/core/vdbe/insn.rs index 3f8baf88..f3f5e36a 100644 --- a/core/vdbe/insn.rs +++ b/core/vdbe/insn.rs @@ -895,13 +895,63 @@ pub fn exec_boolean_not(mut reg: &OwnedValue) -> OwnedValue { } pub fn exec_concat(lhs: &OwnedValue, rhs: &OwnedValue) -> OwnedValue { - let lhs_value = OwnedValue::value_to_string(lhs); - let rhs_value = OwnedValue::value_to_string(rhs); + match (lhs, rhs) { + (OwnedValue::Text(lhs_text), OwnedValue::Text(rhs_text)) => { + OwnedValue::build_text(Rc::new(lhs_text.value.as_ref().clone() + &rhs_text.value)) + } + (OwnedValue::Text(lhs_text), OwnedValue::Integer(rhs_int)) => OwnedValue::build_text( + Rc::new(lhs_text.value.as_ref().clone() + &rhs_int.to_string()), + ), + (OwnedValue::Text(lhs_text), OwnedValue::Float(rhs_float)) => OwnedValue::build_text( + Rc::new(lhs_text.value.as_ref().clone() + &rhs_float.to_string()), + ), + (OwnedValue::Text(lhs_text), OwnedValue::Agg(rhs_agg)) => OwnedValue::build_text(Rc::new( + lhs_text.value.as_ref().clone() + &rhs_agg.final_value().to_string(), + )), - if lhs_value.is_empty() || rhs_value.is_empty() { - OwnedValue::Null - } else { - let result = lhs_value + &rhs_value; - OwnedValue::build_text(Rc::new(result)) + (OwnedValue::Integer(lhs_int), OwnedValue::Text(rhs_text)) => { + OwnedValue::build_text(Rc::new(lhs_int.to_string() + &rhs_text.value)) + } + (OwnedValue::Integer(lhs_int), OwnedValue::Integer(rhs_int)) => { + OwnedValue::build_text(Rc::new(lhs_int.to_string() + &rhs_int.to_string())) + } + (OwnedValue::Integer(lhs_int), OwnedValue::Float(rhs_float)) => { + OwnedValue::build_text(Rc::new(lhs_int.to_string() + &rhs_float.to_string())) + } + (OwnedValue::Integer(lhs_int), OwnedValue::Agg(rhs_agg)) => OwnedValue::build_text( + Rc::new(lhs_int.to_string() + &rhs_agg.final_value().to_string()), + ), + + (OwnedValue::Float(lhs_float), OwnedValue::Text(rhs_text)) => { + OwnedValue::build_text(Rc::new(lhs_float.to_string() + &rhs_text.value)) + } + (OwnedValue::Float(lhs_float), OwnedValue::Integer(rhs_int)) => { + OwnedValue::build_text(Rc::new(lhs_float.to_string() + &rhs_int.to_string())) + } + (OwnedValue::Float(lhs_float), OwnedValue::Float(rhs_float)) => { + OwnedValue::build_text(Rc::new(lhs_float.to_string() + &rhs_float.to_string())) + } + (OwnedValue::Float(lhs_float), OwnedValue::Agg(rhs_agg)) => OwnedValue::build_text( + Rc::new(lhs_float.to_string() + &rhs_agg.final_value().to_string()), + ), + + (OwnedValue::Agg(lhs_agg), OwnedValue::Text(rhs_text)) => { + OwnedValue::build_text(Rc::new(lhs_agg.final_value().to_string() + &rhs_text.value)) + } + (OwnedValue::Agg(lhs_agg), OwnedValue::Integer(rhs_int)) => OwnedValue::build_text( + Rc::new(lhs_agg.final_value().to_string() + &rhs_int.to_string()), + ), + (OwnedValue::Agg(lhs_agg), OwnedValue::Float(rhs_float)) => OwnedValue::build_text( + Rc::new(lhs_agg.final_value().to_string() + &rhs_float.to_string()), + ), + (OwnedValue::Agg(lhs_agg), OwnedValue::Agg(rhs_agg)) => OwnedValue::build_text(Rc::new( + lhs_agg.final_value().to_string() + &rhs_agg.final_value().to_string(), + )), + + (OwnedValue::Null, _) | (_, OwnedValue::Null) => OwnedValue::Null, + (OwnedValue::Blob(_), _) | (_, OwnedValue::Blob(_)) => { + todo!("TODO: Handle Blob conversion to String") + } + (OwnedValue::Record(_), _) | (_, OwnedValue::Record(_)) => unreachable!(), } } diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index 5c8694be..98b328b7 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -46,9 +46,8 @@ use crate::{ use crate::{resolve_ext_path, Connection, Result, Rows, TransactionState, DATABASE_VERSION}; use datetime::{exec_date, exec_datetime_full, exec_julianday, exec_time, exec_unixepoch}; use insn::{ - exec_add, exec_bit_and, exec_bit_not, exec_bit_or, exec_boolean_not, exec_divide, + exec_add, exec_bit_and, exec_bit_not, exec_bit_or, exec_boolean_not, exec_concat, exec_divide, exec_multiply, exec_remainder, exec_shift_left, exec_shift_right, exec_subtract, - exec_concat }; use likeop::{construct_like_escape_arg, exec_glob, exec_like_with_escape}; use rand::distributions::{Distribution, Uniform}; @@ -2343,7 +2342,8 @@ impl Program { state.pc += 1; } Insn::Concat { lhs, rhs, dest } => { - state.registers[*dest] = exec_concat(&state.registers[*lhs], &state.registers[*rhs]); + state.registers[*dest] = + exec_concat(&state.registers[*lhs], &state.registers[*rhs]); state.pc += 1; } }