Merge 'Fix concat_ws to match sqlite behavior' from bit-aloo

closes: #2101
Refactors exec_concat_ws to skip null and blob arguments instead of
inserting separators for them. Also adds a fuzz test.

Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>

Closes #2338

core/vdbe/execute.rs

@@ -7506,24 +7506,21 @@ fn exec_concat_ws(registers: &[Register]) -> Value {
         return Value::Null;
     }
 
-    let separator = match &registers[0].get_owned_value() {
+    let separator = match registers[0].get_owned_value() {
         Value::Null | Value::Blob(_) => return Value::Null,
         v => format!("{v}"),
     };
 
-    let mut result = String::new();
-    for (i, reg) in registers.iter().enumerate().skip(1) {
-        if i > 1 {
-            result.push_str(&separator);
-        }
-        match reg.get_owned_value() {
-            v if matches!(v, Value::Text(_) | Value::Integer(_) | Value::Float(_)) => {
-                result.push_str(&format!("{v}"))
+    let parts = registers[1..]
+        .iter()
+        .filter_map(|reg| match reg.get_owned_value() {
+            Value::Text(_) | Value::Integer(_) | Value::Float(_) => {
+                Some(format!("{}", reg.get_owned_value()))
             }
-            _ => continue,
-        }
-    }
+            _ => None,
+        });
 
+    let result = parts.collect::<Vec<_>>().join(&separator);
     Value::build_text(result)
 }
 

tests/integration/fuzz/mod.rs

@@ -1508,6 +1508,51 @@ mod tests {
         }
     }
 
+    #[test]
+    fn concat_ws_fuzz() {
+        let _ = env_logger::try_init();
+
+        let (mut rng, seed) = rng_from_time();
+        log::info!("seed: {seed}");
+
+        for _ in 0..100 {
+            let db = TempDatabase::new_empty(false);
+            let limbo_conn = db.connect_limbo();
+            let sqlite_conn = rusqlite::Connection::open_in_memory().unwrap();
+
+            let num_args = rng.random_range(7..=17);
+            let mut args = Vec::new();
+            for _ in 0..num_args {
+                let arg = match rng.random_range(0..3) {
+                    0 => rng.random_range(-100..100).to_string(),
+                    1 => format!(
+                        "'{}'",
+                        (0..rng.random_range(1..=5))
+                            .map(|_| rng.random_range(b'a'..=b'z') as char)
+                            .collect::<String>()
+                    ),
+                    2 => "NULL".to_string(),
+                    _ => unreachable!(),
+                };
+                args.push(arg);
+            }
+
+            let sep = match rng.random_range(0..=2) {
+                0 => "','",
+                1 => "'-'",
+                2 => "NULL",
+                _ => unreachable!(),
+            };
+
+            let query = format!("SELECT concat_ws({}, {})", sep, args.join(", "));
+
+            let limbo = limbo_exec_rows(&db, &limbo_conn, &query);
+            let sqlite = sqlite_exec_rows(&sqlite_conn, &query);
+
+            assert_eq!(limbo, sqlite, "seed: {seed}, sep: {sep}, args: {args:?}");
+        }
+    }
+
     #[test]
     // Simple fuzz test for TOTAL with mixed numeric/non-numeric values
     pub fn total_agg_fuzz() {