Merge 'Implement Concat opcode' from Harin

This adds the Concat opcode to the VDBE. Closes #758
tursodatabase · Jan 21, 2025 · c282b23 · c282b23
2 parents db3fa25 + ab3a15e
commit c282b23
Show file tree

Hide file tree

Showing 6 changed files with 119 additions and 4 deletions.
diff --git a/COMPAT.md b/COMPAT.md
@@ -412,7 +412,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
@@ -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/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
@@ -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,65 @@ pub fn exec_boolean_not(mut reg: &OwnedValue) -> OwnedValue {
         _ => todo!(),
     }
 }
+
+pub fn exec_concat(lhs: &OwnedValue, rhs: &OwnedValue) -> OwnedValue {
+    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(),
+        )),
+
+        (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
@@ -46,7 +46,7 @@ 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,
 };
 use likeop::{construct_like_escape_arg, exec_glob, exec_like_with_escape};
@@ -1724,7 +1724,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 +2341,11 @@ 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<OwnedValue> {
     }
 }
 
-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
@@ -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';
+} {}