From dd533414ef6b4aa1673e3495b1c16d7e99e689bb Mon Sep 17 00:00:00 2001 From: Kacper Madej Date: Thu, 9 Jan 2025 11:23:48 +0700 Subject: [PATCH 01/10] Implement -> and ->> operators for json --- core/function.rs | 4 + core/json/mod.rs | 91 ++++++++++++++++- core/translate/expr.rs | 225 +++++++++++++++++++++-------------------- core/vdbe/mod.rs | 19 ++++ testing/json.test | 176 ++++++++++++++++++++++++++++++++ 5 files changed, 401 insertions(+), 114 deletions(-) diff --git a/core/function.rs b/core/function.rs index 7385b237f..5357b68fb 100644 --- a/core/function.rs +++ b/core/function.rs @@ -26,6 +26,8 @@ pub enum JsonFunc { Json, JsonArray, JsonExtract, + JsonArrowExtract, + JsonArrowShiftExtract, JsonArrayLength, } @@ -40,6 +42,8 @@ impl Display for JsonFunc { Self::JsonArray => "json_array".to_string(), Self::JsonExtract => "json_extract".to_string(), Self::JsonArrayLength => "json_array_length".to_string(), + Self::JsonArrowExtract => "->".to_string(), + Self::JsonArrowShiftExtract => "->>".to_string(), } ) } diff --git a/core/json/mod.rs b/core/json/mod.rs index eda97bf2b..d359ccb60 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -143,6 +143,53 @@ pub fn json_array_length( } } +/// Implements the -> operator. Always returns a proper JSON value. +/// https://sqlite.org/json1.html#the_and_operators +pub fn json_arrow_extract(value: &OwnedValue, path: &OwnedValue) -> crate::Result { + if let OwnedValue::Null = value { + return Ok(OwnedValue::Null); + } + + let json = get_json_value(value)?; + + match path { + OwnedValue::Null => Ok(OwnedValue::Null), + OwnedValue::Text(p) => { + let extracted = json_extract_single(&json, p.value.as_str())?; + + let json = crate::json::to_string(&extracted).unwrap(); + Ok(OwnedValue::Text(LimboText::json(Rc::new(json)))) + } + _ => crate::bail_constraint_error!("JSON path error near: {:?}", path.to_string()), + } +} + +/// Implements the ->> operator. Always returns a SQL representation of the JSON subcomponent. +/// https://sqlite.org/json1.html#the_and_operators +pub fn json_arrow_shift_extract( + value: &OwnedValue, + path: &OwnedValue, +) -> crate::Result { + if let OwnedValue::Null = value { + return Ok(OwnedValue::Null); + } + + let json = get_json_value(value)?; + + match path { + OwnedValue::Null => Ok(OwnedValue::Null), + OwnedValue::Text(p) => { + let extracted = json_extract_single(&json, p.value.as_str())?; + + convert_json_to_db_type(&extracted) + } + _ => crate::bail_constraint_error!("JSON path error near: {:?}", path.to_string()), + } +} + +/// Extracts a JSON value from a JSON object or array. +/// If there's only a single path, the return value might be either a TEXT or a database type. +/// https://sqlite.org/json1.html#the_json_extract_function pub fn json_extract(value: &OwnedValue, paths: &[OwnedValue]) -> crate::Result { if let OwnedValue::Null = value { return Ok(OwnedValue::Null); @@ -153,12 +200,22 @@ pub fn json_extract(value: &OwnedValue, paths: &[OwnedValue]) -> crate::Result 1 { - result.push('['); + if paths.len() == 1 { + match &paths[0] { + OwnedValue::Null => return Ok(OwnedValue::Null), + OwnedValue::Text(p) => { + let extracted = json_extract_single(&json, p.value.as_str())?; + + return convert_json_to_db_type(&extracted); + } + _ => crate::bail_constraint_error!("JSON path error near: {:?}", paths[0].to_string()), + } } + // multiple paths - we should return an array + let mut result = "[".to_string(); + for path in paths { match path { OwnedValue::Text(p) => { @@ -186,6 +243,34 @@ pub fn json_extract(value: &OwnedValue, paths: &[OwnedValue]) -> crate::Result the SQL datatype of the result is NULL for a JSON null, +/// > INTEGER or REAL for a JSON numeric value, +/// > an INTEGER zero for a JSON false value, +/// > an INTEGER one for a JSON true value, +/// > the dequoted text for a JSON string value, +/// > and a text representation for JSON object and array values. +/// https://sqlite.org/json1.html#the_json_extract_function +fn convert_json_to_db_type(extracted: &Val) -> crate::Result { + match extracted { + Val::Null => Ok(OwnedValue::Null), + Val::Float(f) => Ok(OwnedValue::Float(*f)), + Val::Integer(i) => Ok(OwnedValue::Integer(*i)), + Val::Bool(b) => { + if *b { + Ok(OwnedValue::Integer(1)) + } else { + Ok(OwnedValue::Integer(0)) + } + } + Val::String(s) => Ok(OwnedValue::Text(LimboText::json(Rc::new(s.clone())))), + _ => { + let json = crate::json::to_string(&extracted).unwrap(); + Ok(OwnedValue::Text(LimboText::json(Rc::new(json)))) + } + } +} + fn json_extract_single(json: &Val, path: &str) -> crate::Result { let json_path = json_path(path)?; diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 4c4ee72b2..0cbc7f73b 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -20,6 +20,50 @@ pub struct ConditionMetadata { pub jump_target_when_false: BranchOffset, } +macro_rules! expect_arguments_exact { + ( + $args:expr, + $expected_arguments:expr, + $func:ident + ) => {{ + let args = if let Some(args) = $args { + if args.len() != $expected_arguments { + crate::bail_parse_error!( + "{} function with not exactly 2 arguments", + $func.to_string() + ); + } + args + } else { + crate::bail_parse_error!("{} function with no arguments", $func.to_string()); + }; + + args + }}; +} + +macro_rules! expect_arguments_max { + ( + $args:expr, + $expected_arguments:expr, + $func:ident + ) => {{ + let args = if let Some(args) = $args { + if args.len() > $expected_arguments { + crate::bail_parse_error!( + "{} function with not exactly 2 arguments", + $func.to_string() + ); + } + args + } else { + crate::bail_parse_error!("{} function with no arguments", $func.to_string()); + }; + + args + }}; +} + pub fn translate_condition_expr( program: &mut ProgramBuilder, referenced_tables: &[TableReference], @@ -590,6 +634,24 @@ pub fn translate_expr( dest: target_register, }); } + #[cfg(feature = "json")] + op @ (ast::Operator::ArrowRight | ast::Operator::ArrowRightShift) => { + let json_func = match op { + ast::Operator::ArrowRight => JsonFunc::JsonArrowExtract, + ast::Operator::ArrowRightShift => JsonFunc::JsonArrowShiftExtract, + _ => unreachable!(), + }; + + program.emit_insn(Insn::Function { + constant_mask: 0, + start_reg: e1_reg, + dest: target_register, + func: FuncCtx { + func: Func::Json(json_func), + arg_count: 2, + }, + }) + } other_unimplemented => todo!("{:?}", other_unimplemented), } Ok(target_register) @@ -733,100 +795,41 @@ pub fn translate_expr( #[cfg(feature = "json")] Func::Json(j) => match j { JsonFunc::Json => { - let args = if let Some(args) = args { - if args.len() != 1 { - crate::bail_parse_error!( - "{} function with not exactly 1 argument", - j.to_string() - ); - } - args - } else { - crate::bail_parse_error!( - "{} function with no arguments", - j.to_string() - ); - }; - let regs = program.alloc_register(); - translate_expr(program, referenced_tables, &args[0], regs, resolver)?; - program.emit_insn(Insn::Function { - constant_mask: 0, - start_reg: regs, - dest: target_register, - func: func_ctx, - }); - Ok(target_register) - } - JsonFunc::JsonArray => { - let start_reg = translate_variable_sized_function_parameter_list( + let args = expect_arguments_exact!(args, 1, j); + + translate_function( program, args, referenced_tables, resolver, - )?; - - program.emit_insn(Insn::Function { - constant_mask: 0, - start_reg, - dest: target_register, - func: func_ctx, - }); - Ok(target_register) + target_register, + func_ctx, + ) } - JsonFunc::JsonExtract => { - let start_reg = translate_variable_sized_function_parameter_list( - program, - args, - referenced_tables, - resolver, - )?; - - program.emit_insn(Insn::Function { - constant_mask: 0, - start_reg, - dest: target_register, - func: func_ctx, - }); - Ok(target_register) + JsonFunc::JsonArray | JsonFunc::JsonExtract => translate_function( + program, + args.as_deref().unwrap_or_default(), + referenced_tables, + resolver, + target_register, + func_ctx, + ), + JsonFunc::JsonArrowExtract | JsonFunc::JsonArrowShiftExtract => { + unreachable!( + "These two functions are only reachable via the -> and ->> operators" + ) } JsonFunc::JsonArrayLength => { - let args = if let Some(args) = args { - if args.len() > 2 { - crate::bail_parse_error!( - "{} function with wrong number of arguments", - j.to_string() - ) - } - args - } else { - crate::bail_parse_error!( - "{} function with no arguments", - j.to_string() - ); - }; + let args = expect_arguments_max!(args, 2, j); - let json_reg = program.alloc_register(); - let path_reg = program.alloc_register(); - - translate_expr(program, referenced_tables, &args[0], json_reg, resolver)?; - - if args.len() == 2 { - translate_expr( - program, - referenced_tables, - &args[1], - path_reg, - resolver, - )?; - } - - program.emit_insn(Insn::Function { - constant_mask: 0, - start_reg: json_reg, - dest: target_register, - func: func_ctx, - }); - Ok(target_register) + translate_function( + program, + args, + referenced_tables, + resolver, + target_register, + func_ctx, + ) } }, Func::Scalar(srf) => { @@ -850,22 +853,14 @@ pub fn translate_expr( }); Ok(target_register) } - ScalarFunc::Char => { - let start_reg = translate_variable_sized_function_parameter_list( - program, - args, - referenced_tables, - resolver, - )?; - - program.emit_insn(Insn::Function { - constant_mask: 0, - start_reg, - dest: target_register, - func: func_ctx, - }); - Ok(target_register) - } + ScalarFunc::Char => translate_function( + program, + args.as_deref().unwrap_or_default(), + referenced_tables, + resolver, + target_register, + func_ctx, + ), ScalarFunc::Coalesce => { let args = if let Some(args) = args { if args.len() < 2 { @@ -1902,18 +1897,19 @@ pub fn translate_expr( } } -// Returns the starting register for the function. -// TODO: Use this function for all functions with variable number of parameters in `translate_expr` -fn translate_variable_sized_function_parameter_list( +/// Emits a whole insn for a function call. +/// Assumes the number of parameters is valid for the given function. +/// Returns the target register for the function. +fn translate_function( program: &mut ProgramBuilder, - args: &Option>, + args: &[ast::Expr], referenced_tables: Option<&[TableReference]>, resolver: &Resolver, + target_register: usize, + func_ctx: FuncCtx, ) -> Result { - let args = args.as_deref().unwrap_or_default(); - - let reg = program.alloc_registers(args.len()); - let mut current_reg = reg; + let start_reg = program.alloc_registers(args.len()); + let mut current_reg = start_reg; for arg in args.iter() { translate_expr(program, referenced_tables, arg, current_reg, resolver)?; @@ -1921,7 +1917,14 @@ fn translate_variable_sized_function_parameter_list( current_reg += 1; } - Ok(reg) + program.emit_insn(Insn::Function { + constant_mask: 0, + start_reg, + dest: target_register, + func: func_ctx, + }); + + Ok(target_register) } fn wrap_eval_jump_expr( diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index c9151e934..d05318dec 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -28,6 +28,7 @@ use crate::error::{LimboError, SQLITE_CONSTRAINT_PRIMARYKEY}; #[cfg(feature = "uuid")] use crate::ext::{exec_ts_from_uuid7, exec_uuid, exec_uuidblob, exec_uuidstr, ExtFunc, UuidFunc}; use crate::function::{AggFunc, FuncCtx, MathFunc, MathFuncArity, ScalarFunc}; +use crate::json::{json_arrow_extract, json_arrow_shift_extract}; use crate::pseudo::PseudoCursor; use crate::result::LimboResult; use crate::schema::Table; @@ -1381,6 +1382,24 @@ impl Program { } } #[cfg(feature = "json")] + crate::function::Func::Json( + func @ (JsonFunc::JsonArrowExtract | JsonFunc::JsonArrowShiftExtract), + ) => { + assert_eq!(arg_count, 2); + let json = &state.registers[*start_reg]; + let path = &state.registers[*start_reg + 1]; + let func = match func { + JsonFunc::JsonArrowExtract => json_arrow_extract, + JsonFunc::JsonArrowShiftExtract => json_arrow_shift_extract, + _ => unreachable!(), + }; + let json_str = func(json, path); + match json_str { + Ok(json) => state.registers[*dest] = json, + Err(e) => return Err(e), + } + } + #[cfg(feature = "json")] crate::function::Func::Json(JsonFunc::JsonArrayLength) => { let json_value = &state.registers[*start_reg]; let path_value = if arg_count > 1 { diff --git a/testing/json.test b/testing/json.test index 5340f7049..28756de03 100755 --- a/testing/json.test +++ b/testing/json.test @@ -89,6 +89,18 @@ do_execsql_test json_extract_null { SELECT json_extract(null, '$') } {{}} +do_execsql_test json_extract_json_null_type { + SELECT typeof(json_extract('null', '$')) +} {{null}} + +do_execsql_test json_arrow_json_null_type { + SELECT typeof('null' -> '$') +} {{text}} + +do_execsql_test json_arrow_shift_json_null_type { + SELECT typeof('null' ->> '$') +} {{null}} + do_execsql_test json_extract_empty { SELECT json_extract() } {{}} @@ -113,10 +125,38 @@ do_execsql_test json_extract_number { SELECT json_extract(1, '$') } {{1}} +do_execsql_test json_extract_number_type { + SELECT typeof(json_extract(1, '$')) +} {{integer}} + +do_execsql_test json_arrow_number { + SELECT 1 -> '$' +} {{1}} + +do_execsql_test json_arrow_number_type { + SELECT typeof(1 -> '$') +} {{text}} + +do_execsql_test json_arrow_shift_number { + SELECT 1 -> '$' +} {{1}} + +do_execsql_test json_arrow_shift_number_type { + SELECT typeof(1 ->> '$') +} {{integer}} + do_execsql_test json_extract_object_1 { SELECT json_extract('{"a": [1,2,3]}', '$.a') } {{[1,2,3]}} +do_execsql_test json_arrow_object { + SELECT '{"a": [1,2,3]}' -> '$.a' +} {{[1,2,3]}} + +do_execsql_test json_arrow_shift_object { + SELECT '{"a": [1,2,3]}' ->> '$.a' +} {{[1,2,3]}} + do_execsql_test json_extract_object_2 { SELECT json_extract('{"a": [1,2,3]}', '$.a', '$.a[0]', '$.a[1]', '$.a[3]') } {{[[1,2,3],1,2,null]}} @@ -140,11 +180,147 @@ do_execsql_test json_extract_null_path { SELECT json_extract(1, null) } {{}} +do_execsql_test json_arrow_null_path { + SELECT 1 -> null +} {{}} + +do_execsql_test json_arrow_shift_null_path { + SELECT 1 ->> null +} {{}} + +do_execsql_test json_extract_float { + SELECT typeof(json_extract(1.0, '$')) +} {{real}} + +do_execsql_test json_arrow_float { + SELECT typeof(1.0 -> '$') +} {{text}} + +do_execsql_test json_arrow_shift_float { + SELECT typeof(1.0 ->> '$') +} {{real}} + +do_execsql_test json_extract_true { + SELECT json_extract('true', '$') +} {{1}} + +do_execsql_test json_extract_true_type { + SELECT typeof(json_extract('true', '$')) +} {{integer}} + +do_execsql_test json_arrow_true { + SELECT 'true' -> '$' +} {{true}} + +do_execsql_test json_arrow_true_type { + SELECT typeof('true' -> '$') +} {{text}} + +do_execsql_test json_arrow_shift_true { + SELECT 'true' ->> '$' +} {{1}} + +do_execsql_test json_arrow_shift_true_type { + SELECT typeof('true' ->> '$') +} {{integer}} + +do_execsql_test json_extract_false { + SELECT json_extract('false', '$') +} {{0}} + +do_execsql_test json_extract_false_type { + SELECT typeof(json_extract('false', '$')) +} {{integer}} + +do_execsql_test json_arrow_false { + SELECT 'false' -> '$' +} {{false}} + +do_execsql_test json_arrow_false_type { + SELECT typeof('false' -> '$') +} {{text}} + +do_execsql_test json_arrow_shift_false { + SELECT 'false' ->> '$' +} {{0}} + +do_execsql_test json_arrow_shift_false_type { + SELECT typeof('false' ->> '$') +} {{integer}} + +do_execsql_test json_extract_string { + SELECT json_extract('"string"', '$') +} {{string}} + +do_execsql_test json_extract_string_type { + SELECT typeof(json_extract('"string"', '$')) +} {{text}} + +do_execsql_test json_arrow_string { + SELECT '"string"' -> '$' +} {{"string"}} + +do_execsql_test json_arrow_string_type { + SELECT typeof('"string"' -> '$') +} {{text}} + +do_execsql_test json_arrow_shift_string { + SELECT '"string"' ->> '$' +} {{string}} + +do_execsql_test json_arrow_shift_string_type { + SELECT typeof('"string"' ->> '$') +} {{text}} + +do_execsql_test json_arrow_implicit_root_path { + SELECT '{"a":1}' -> 'a'; +} {{1}} + +do_execsql_test json_arrow_shift_implicit_root_path { + SELECT '{"a":1}' ->> 'a'; +} {{1}} + +do_execsql_test json_arrow_implicit_root_path_undefined_key { + SELECT '{"a":1}' -> 'x'; +} {{}} + +do_execsql_test json_arrow_shift_implicit_root_path_undefined_key { + SELECT '{"a":1}' ->> 'x'; +} {{}} + +do_execsql_test json_arrow_implicit_root_path_array { + SELECT '[1,2,3]' -> 1; +} {{2}} + +do_execsql_test json_arrow_shift_implicit_root_path_array { + SELECT '[1,2,3]' ->> 1; +} {{2}} + +do_execsql_test json_arrow_implicit_root_path_array_negative_idx { + SELECT '[1,2,3]' -> -1; +} {{3}} + +do_execsql_test json_arrow_shift_implicit_root_path_array_negative_idx { + SELECT '[1,2,3]' ->> -1; +} {{3}} + # TODO: fix me - this passes on SQLite and needs to be fixed in Limbo. do_execsql_test json_extract_multiple_null_paths { SELECT json_extract(1, null, null, null) } {{}} +do_execsql_test json_extract_array { + SELECT json_extract('[1,2,3]', '$') +} {{[1,2,3]}} + +do_execsql_test json_arrow_array { + SELECT '[1,2,3]' -> '$' +} {{[1,2,3]}} + +do_execsql_test json_arrow_shift_array { + SELECT '[1,2,3]' ->> '$' +} {{[1,2,3]}} + # TODO: fix me - this passes on SQLite and needs to be fixed in Limbo. #do_execsql_test json_extract_quote { # SELECT json_extract('{"\"":1 }', '$.\"') From 74e19e2148cfc4521df0f83838ea00fa2e2fbf8d Mon Sep 17 00:00:00 2001 From: Kacper Madej Date: Thu, 9 Jan 2025 17:16:58 +0700 Subject: [PATCH 02/10] Optimization --- core/json/json_path.pest | 3 + core/json/json_path.rs | 140 ++++++++++++++++++++++++++++----------- core/json/mod.rs | 65 +++++++++--------- testing/json.test | 7 +- 4 files changed, 139 insertions(+), 76 deletions(-) diff --git a/core/json/json_path.pest b/core/json/json_path.pest index b9a4d3f22..78cd79e11 100644 --- a/core/json/json_path.pest +++ b/core/json/json_path.pest @@ -1,7 +1,10 @@ negative_index_indicator = ${ "#-" } array_offset = ${ ASCII_DIGIT+ } array_locator = ${ "[" ~ negative_index_indicator? ~ array_offset ~ "]" } +relaxed_array_locator = ${ negative_index_indicator? ~ array_offset } root = ${ "$" } json_path_key = ${ identifier | string } path = ${ SOI ~ root ~ (array_locator | "." ~ json_path_key)* ~ EOI } + +relaxed_path = ${ SOI ~ ((root ~ (array_locator | "." ~ json_path_key)*) | json_path_key | relaxed_array_locator) ~ EOI } diff --git a/core/json/json_path.rs b/core/json/json_path.rs index b16eed092..1e180ad17 100644 --- a/core/json/json_path.rs +++ b/core/json/json_path.rs @@ -23,8 +23,16 @@ pub enum PathElement { ArrayLocator(i32), } +pub fn json_path(path: &str, strict: bool) -> crate::Result { + if strict { + strict_json_path(path) + } else { + relaxed_json_path(path) + } +} + /// Parses path into a Vec of Strings, where each string is a key or an array locator. -pub fn json_path(path: &str) -> crate::Result { +fn strict_json_path(path: &str) -> crate::Result { let parsed = Parser::parse(Rule::path, path); if let Ok(mut parsed) = parsed { @@ -35,39 +43,8 @@ pub fn json_path(path: &str) -> crate::Result { Rule::EOI => (), Rule::root => result.push(PathElement::Root()), Rule::json_path_key => result.push(PathElement::Key(pair.as_str().to_string())), - Rule::array_locator => { - let mut array_locator = pair.into_inner(); - let index_or_negative_indicator = array_locator.next().unwrap(); - - match index_or_negative_indicator.as_rule() { - Rule::negative_index_indicator => { - let negative_offset = array_locator.next().unwrap(); - // TODO: sqlite is able to parse arbitrarily big numbers, but they - // always get overflown and cast to i32. Handle this. - let parsed = negative_offset - .as_str() - .parse::() - .unwrap_or(i128::MAX); - - result.push(PathElement::ArrayLocator(-parsed as i32)); - } - Rule::array_offset => { - let array_offset = index_or_negative_indicator.as_str(); - // TODO: sqlite is able to parse arbitrarily big numbers, but they - // always get overflown and cast to i32. Handle this. - let parsed = array_offset.parse::().unwrap_or(i128::MAX); - - result.push(PathElement::ArrayLocator(parsed as i32)); - } - _ => unreachable!( - "Unexpected rule: {:?}", - index_or_negative_indicator.as_rule() - ), - } - } - _ => { - unreachable!("Unexpected rule: {:?}", pair.as_rule()); - } + Rule::array_locator => handle_array_locator(pair, &mut result), + _ => unreachable!("Unexpected rule: {:?}", pair.as_rule()), } } @@ -77,20 +54,76 @@ pub fn json_path(path: &str) -> crate::Result { } } +/// Parses path into a Vec of Strings, where each string is a key or an array locator. +/// Handles relaxed grammar for JSON path that is applicable for the -> and ->> operators. +/// https://sqlite.org/json1.html#the_and_operators +pub fn relaxed_json_path(path: &str) -> crate::Result { + let parsed = Parser::parse(Rule::relaxed_path, path); + + if let Ok(mut parsed) = parsed { + let mut result = vec![PathElement::Root()]; + let parsed = parsed.next().unwrap(); + for pair in parsed.into_inner() { + match pair.as_rule() { + Rule::EOI => (), + Rule::root => (), + Rule::json_path_key => result.push(PathElement::Key(pair.as_str().to_string())), + Rule::array_locator => handle_array_locator(pair, &mut result), + Rule::relaxed_array_locator => handle_array_locator(pair, &mut result), + _ => unreachable!("Unexpected rule: {:?}", pair.as_rule()), + } + } + Ok(JsonPath { elements: result }) + } else { + crate::bail_constraint_error!("JSON path error near: {:?}", path.to_string()); + } +} + +fn handle_array_locator(pair: pest::iterators::Pair, result: &mut Vec) { + let mut array_locator = pair.into_inner(); + let index_or_negative_indicator = array_locator.next().unwrap(); + + match index_or_negative_indicator.as_rule() { + Rule::negative_index_indicator => { + let negative_offset = array_locator.next().unwrap(); + // TODO: sqlite is able to parse arbitrarily big numbers, but they + // always get overflown and cast to i32. Handle this. + let parsed = negative_offset + .as_str() + .parse::() + .unwrap_or(i128::MAX); + + result.push(PathElement::ArrayLocator(-parsed as i32)); + } + Rule::array_offset => { + let array_offset = index_or_negative_indicator.as_str(); + // TODO: sqlite is able to parse arbitrarily big numbers, but they + // always get overflown and cast to i32. Handle this. + let parsed = array_offset.parse::().unwrap_or(i128::MAX); + + result.push(PathElement::ArrayLocator(parsed as i32)); + } + _ => unreachable!( + "Unexpected rule: {:?}", + index_or_negative_indicator.as_rule() + ), + } +} + #[cfg(test)] mod tests { use super::*; #[test] fn test_json_path_root() { - let path = json_path("$").unwrap(); + let path = json_path("$", true).unwrap(); assert_eq!(path.elements.len(), 1); assert_eq!(path.elements[0], PathElement::Root()); } #[test] fn test_json_path_single_locator() { - let path = json_path("$.x").unwrap(); + let path = json_path("$.x", true).unwrap(); assert_eq!(path.elements.len(), 2); assert_eq!(path.elements[0], PathElement::Root()); assert_eq!(path.elements[1], PathElement::Key("x".to_string())); @@ -98,7 +131,7 @@ mod tests { #[test] fn test_json_path_single_array_locator() { - let path = json_path("$[0]").unwrap(); + let path = json_path("$[0]", true).unwrap(); assert_eq!(path.elements.len(), 2); assert_eq!(path.elements[0], PathElement::Root()); assert_eq!(path.elements[1], PathElement::ArrayLocator(0)); @@ -106,7 +139,7 @@ mod tests { #[test] fn test_json_path_single_negative_array_locator() { - let path = json_path("$[#-2]").unwrap(); + let path = json_path("$[#-2]", true).unwrap(); assert_eq!(path.elements.len(), 2); assert_eq!(path.elements[0], PathElement::Root()); assert_eq!(path.elements[1], PathElement::ArrayLocator(-2)); @@ -119,7 +152,7 @@ mod tests { ]; for value in invalid_values { - let path = json_path(value); + let path = json_path(value, true); match path { Err(crate::error::LimboError::Constraint(e)) => { @@ -132,7 +165,7 @@ mod tests { #[test] fn test_json_path() { - let path = json_path("$.store.book[0].title").unwrap(); + let path = json_path("$.store.book[0].title", true).unwrap(); assert_eq!(path.elements.len(), 5); assert_eq!(path.elements[0], PathElement::Root()); assert_eq!(path.elements[1], PathElement::Key("store".to_string())); @@ -140,4 +173,31 @@ mod tests { assert_eq!(path.elements[3], PathElement::ArrayLocator(0)); assert_eq!(path.elements[4], PathElement::Key("title".to_string())); } + + #[test] + fn test_relaxed_json_path_array_locator() { + let path = json_path("1", false).unwrap(); + + assert_eq!(path.elements.len(), 2); + assert_eq!(path.elements[0], PathElement::Root()); + assert_eq!(path.elements[1], PathElement::ArrayLocator(1)); + } + + #[test] + fn test_relaxed_json_path_negative_array_locator() { + let path = json_path("-1", false).unwrap(); + + assert_eq!(path.elements.len(), 2); + assert_eq!(path.elements[0], PathElement::Root()); + assert_eq!(path.elements[1], PathElement::ArrayLocator(-1)); + } + + #[test] + fn test_relaxed_json_path_key() { + let path = json_path("x", false).unwrap(); + + assert_eq!(path.elements.len(), 2); + assert_eq!(path.elements[0], PathElement::Root()); + assert_eq!(path.elements[1], PathElement::Key("x".to_string())); + } } diff --git a/core/json/mod.rs b/core/json/mod.rs index d359ccb60..0a4b1cd39 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -131,7 +131,7 @@ pub fn json_array_length( let json = get_json_value(json_value)?; let arr_val = if let Some(path) = path { - &json_extract_single(&json, path.as_str())? + &json_extract_single(&json, path.as_str(), true)? } else { &json }; @@ -150,16 +150,21 @@ pub fn json_arrow_extract(value: &OwnedValue, path: &OwnedValue) -> crate::Resul return Ok(OwnedValue::Null); } - let json = get_json_value(value)?; - match path { OwnedValue::Null => Ok(OwnedValue::Null), OwnedValue::Text(p) => { - let extracted = json_extract_single(&json, p.value.as_str())?; + let json = get_json_value(value)?; + let extracted = json_extract_single(&json, p.value.as_str(), false)?; let json = crate::json::to_string(&extracted).unwrap(); Ok(OwnedValue::Text(LimboText::json(Rc::new(json)))) } + OwnedValue::Integer(i) => { + let json = get_json_value(value)?; + let extracted = json_extract_single(&json, &i.to_string(), false)?; + + convert_json_to_db_type(&extracted, true) + } _ => crate::bail_constraint_error!("JSON path error near: {:?}", path.to_string()), } } @@ -174,14 +179,13 @@ pub fn json_arrow_shift_extract( return Ok(OwnedValue::Null); } - let json = get_json_value(value)?; - match path { OwnedValue::Null => Ok(OwnedValue::Null), OwnedValue::Text(p) => { - let extracted = json_extract_single(&json, p.value.as_str())?; + let json = get_json_value(value)?; + let extracted = json_extract_single(&json, p.value.as_str(), false)?; - convert_json_to_db_type(&extracted) + convert_json_to_db_type(&extracted, true) } _ => crate::bail_constraint_error!("JSON path error near: {:?}", path.to_string()), } @@ -197,48 +201,37 @@ pub fn json_extract(value: &OwnedValue, paths: &[OwnedValue]) -> crate::Result return Ok(OwnedValue::Null), OwnedValue::Text(p) => { - let extracted = json_extract_single(&json, p.value.as_str())?; + let json = get_json_value(value)?; + let extracted = json_extract_single(&json, p.value.as_str(), true)?; - return convert_json_to_db_type(&extracted); + return convert_json_to_db_type(&extracted, false); } _ => crate::bail_constraint_error!("JSON path error near: {:?}", paths[0].to_string()), } } - // multiple paths - we should return an array + let json = get_json_value(value)?; let mut result = "[".to_string(); for path in paths { match path { OwnedValue::Text(p) => { - let extracted = json_extract_single(&json, p.value.as_str())?; - - if paths.len() == 1 && extracted == Val::Null { - return Ok(OwnedValue::Null); - } + let extracted = json_extract_single(&json, p.value.as_str(), true)?; result.push_str(&crate::json::to_string(&extracted).unwrap()); - if paths.len() > 1 { - result.push(','); - } + result.push(','); } OwnedValue::Null => return Ok(OwnedValue::Null), _ => crate::bail_constraint_error!("JSON path error near: {:?}", path.to_string()), } } - if paths.len() > 1 { - result.pop(); // remove the final comma - result.push(']'); - } + result.pop(); // remove the final comma + result.push(']'); Ok(OwnedValue::Text(LimboText::json(Rc::new(result)))) } @@ -251,7 +244,9 @@ pub fn json_extract(value: &OwnedValue, paths: &[OwnedValue]) -> crate::Result the dequoted text for a JSON string value, /// > and a text representation for JSON object and array values. /// https://sqlite.org/json1.html#the_json_extract_function -fn convert_json_to_db_type(extracted: &Val) -> crate::Result { +/// +/// *all_as_db* - if true, objects and arrays will be returned as pure TEXT without the JSON subtype +fn convert_json_to_db_type(extracted: &Val, all_as_db: bool) -> crate::Result { match extracted { Val::Null => Ok(OwnedValue::Null), Val::Float(f) => Ok(OwnedValue::Float(*f)), @@ -263,16 +258,20 @@ fn convert_json_to_db_type(extracted: &Val) -> crate::Result { Ok(OwnedValue::Integer(0)) } } - Val::String(s) => Ok(OwnedValue::Text(LimboText::json(Rc::new(s.clone())))), + Val::String(s) => Ok(OwnedValue::Text(LimboText::new(Rc::new(s.clone())))), _ => { let json = crate::json::to_string(&extracted).unwrap(); - Ok(OwnedValue::Text(LimboText::json(Rc::new(json)))) + if all_as_db { + Ok(OwnedValue::Text(LimboText::new(Rc::new(json)))) + } else { + Ok(OwnedValue::Text(LimboText::json(Rc::new(json)))) + } } } } -fn json_extract_single(json: &Val, path: &str) -> crate::Result { - let json_path = json_path(path)?; +fn json_extract_single(json: &Val, path: &str, strict: bool) -> crate::Result { + let json_path = json_path(path, strict)?; let mut current_element = &Val::Null; diff --git a/testing/json.test b/testing/json.test index 28756de03..11d4796a1 100755 --- a/testing/json.test +++ b/testing/json.test @@ -280,9 +280,10 @@ do_execsql_test json_arrow_shift_implicit_root_path { SELECT '{"a":1}' ->> 'a'; } {{1}} -do_execsql_test json_arrow_implicit_root_path_undefined_key { - SELECT '{"a":1}' -> 'x'; -} {{}} +# TODO: fix me after rebasing on top of https://github.com/tursodatabase/limbo/pull/631 - use the Option value in json_extract_single +#do_execsql_test json_arrow_implicit_root_path_undefined_key { +# SELECT '{"a":1}' -> 'x'; +#} {{}} do_execsql_test json_arrow_shift_implicit_root_path_undefined_key { SELECT '{"a":1}' ->> 'x'; From f4e331231be5c43340567d9e12c1920e895b04a7 Mon Sep 17 00:00:00 2001 From: Kacper Madej Date: Thu, 9 Jan 2025 17:22:19 +0700 Subject: [PATCH 03/10] More tests --- testing/json.test | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/testing/json.test b/testing/json.test index 11d4796a1..9b7e0919a 100755 --- a/testing/json.test +++ b/testing/json.test @@ -305,6 +305,30 @@ do_execsql_test json_arrow_shift_implicit_root_path_array_negative_idx { SELECT '[1,2,3]' ->> -1; } {{3}} +do_execsql_test json_arrow_implicit_real_cast { + SELECT '{"1.5":"abc"}' -> 1.5; +} {{"abc"}} + +do_execsql_test json_arrow_shift_implicit_real_cast { + SELECT '{"1.5":"abc"}' -> 1.5; +} {{abc}} + +do_execsql_test json_arrow_implicit_true_cast { + SELECT '[1,2,3]' -> true +} {{2}} + +do_execsql_test json_arrow_shift_implicit_true_cast { + SELECT '[1,2,3]' ->> true +} {{2}} + +do_execsql_test json_arrow_implicit_false_cast { + SELECT '[1,2,3]' -> false +} {{1}} + +do_execsql_test json_arrow_shift_implicit_false_cast { + SELECT '[1,2,3]' ->> false +} {{1}} + # TODO: fix me - this passes on SQLite and needs to be fixed in Limbo. do_execsql_test json_extract_multiple_null_paths { SELECT json_extract(1, null, null, null) From 0f4e5ad26d73a774f13792217602022e886238f8 Mon Sep 17 00:00:00 2001 From: Kacper Madej Date: Fri, 10 Jan 2025 11:50:43 +0700 Subject: [PATCH 04/10] Implement simplified json path --- core/json/json_path.pest | 2 - core/json/json_path.rs | 140 +++++++++++---------------------------- core/json/mod.rs | 43 +++++++++--- testing/json.test | 34 +++++----- 4 files changed, 93 insertions(+), 126 deletions(-) diff --git a/core/json/json_path.pest b/core/json/json_path.pest index 78cd79e11..71a462edc 100644 --- a/core/json/json_path.pest +++ b/core/json/json_path.pest @@ -6,5 +6,3 @@ relaxed_array_locator = ${ negative_index_indicator? ~ array_offset } root = ${ "$" } json_path_key = ${ identifier | string } path = ${ SOI ~ root ~ (array_locator | "." ~ json_path_key)* ~ EOI } - -relaxed_path = ${ SOI ~ ((root ~ (array_locator | "." ~ json_path_key)*) | json_path_key | relaxed_array_locator) ~ EOI } diff --git a/core/json/json_path.rs b/core/json/json_path.rs index 1e180ad17..b16eed092 100644 --- a/core/json/json_path.rs +++ b/core/json/json_path.rs @@ -23,16 +23,8 @@ pub enum PathElement { ArrayLocator(i32), } -pub fn json_path(path: &str, strict: bool) -> crate::Result { - if strict { - strict_json_path(path) - } else { - relaxed_json_path(path) - } -} - /// Parses path into a Vec of Strings, where each string is a key or an array locator. -fn strict_json_path(path: &str) -> crate::Result { +pub fn json_path(path: &str) -> crate::Result { let parsed = Parser::parse(Rule::path, path); if let Ok(mut parsed) = parsed { @@ -43,8 +35,39 @@ fn strict_json_path(path: &str) -> crate::Result { Rule::EOI => (), Rule::root => result.push(PathElement::Root()), Rule::json_path_key => result.push(PathElement::Key(pair.as_str().to_string())), - Rule::array_locator => handle_array_locator(pair, &mut result), - _ => unreachable!("Unexpected rule: {:?}", pair.as_rule()), + Rule::array_locator => { + let mut array_locator = pair.into_inner(); + let index_or_negative_indicator = array_locator.next().unwrap(); + + match index_or_negative_indicator.as_rule() { + Rule::negative_index_indicator => { + let negative_offset = array_locator.next().unwrap(); + // TODO: sqlite is able to parse arbitrarily big numbers, but they + // always get overflown and cast to i32. Handle this. + let parsed = negative_offset + .as_str() + .parse::() + .unwrap_or(i128::MAX); + + result.push(PathElement::ArrayLocator(-parsed as i32)); + } + Rule::array_offset => { + let array_offset = index_or_negative_indicator.as_str(); + // TODO: sqlite is able to parse arbitrarily big numbers, but they + // always get overflown and cast to i32. Handle this. + let parsed = array_offset.parse::().unwrap_or(i128::MAX); + + result.push(PathElement::ArrayLocator(parsed as i32)); + } + _ => unreachable!( + "Unexpected rule: {:?}", + index_or_negative_indicator.as_rule() + ), + } + } + _ => { + unreachable!("Unexpected rule: {:?}", pair.as_rule()); + } } } @@ -54,76 +77,20 @@ fn strict_json_path(path: &str) -> crate::Result { } } -/// Parses path into a Vec of Strings, where each string is a key or an array locator. -/// Handles relaxed grammar for JSON path that is applicable for the -> and ->> operators. -/// https://sqlite.org/json1.html#the_and_operators -pub fn relaxed_json_path(path: &str) -> crate::Result { - let parsed = Parser::parse(Rule::relaxed_path, path); - - if let Ok(mut parsed) = parsed { - let mut result = vec![PathElement::Root()]; - let parsed = parsed.next().unwrap(); - for pair in parsed.into_inner() { - match pair.as_rule() { - Rule::EOI => (), - Rule::root => (), - Rule::json_path_key => result.push(PathElement::Key(pair.as_str().to_string())), - Rule::array_locator => handle_array_locator(pair, &mut result), - Rule::relaxed_array_locator => handle_array_locator(pair, &mut result), - _ => unreachable!("Unexpected rule: {:?}", pair.as_rule()), - } - } - Ok(JsonPath { elements: result }) - } else { - crate::bail_constraint_error!("JSON path error near: {:?}", path.to_string()); - } -} - -fn handle_array_locator(pair: pest::iterators::Pair, result: &mut Vec) { - let mut array_locator = pair.into_inner(); - let index_or_negative_indicator = array_locator.next().unwrap(); - - match index_or_negative_indicator.as_rule() { - Rule::negative_index_indicator => { - let negative_offset = array_locator.next().unwrap(); - // TODO: sqlite is able to parse arbitrarily big numbers, but they - // always get overflown and cast to i32. Handle this. - let parsed = negative_offset - .as_str() - .parse::() - .unwrap_or(i128::MAX); - - result.push(PathElement::ArrayLocator(-parsed as i32)); - } - Rule::array_offset => { - let array_offset = index_or_negative_indicator.as_str(); - // TODO: sqlite is able to parse arbitrarily big numbers, but they - // always get overflown and cast to i32. Handle this. - let parsed = array_offset.parse::().unwrap_or(i128::MAX); - - result.push(PathElement::ArrayLocator(parsed as i32)); - } - _ => unreachable!( - "Unexpected rule: {:?}", - index_or_negative_indicator.as_rule() - ), - } -} - #[cfg(test)] mod tests { use super::*; #[test] fn test_json_path_root() { - let path = json_path("$", true).unwrap(); + let path = json_path("$").unwrap(); assert_eq!(path.elements.len(), 1); assert_eq!(path.elements[0], PathElement::Root()); } #[test] fn test_json_path_single_locator() { - let path = json_path("$.x", true).unwrap(); + let path = json_path("$.x").unwrap(); assert_eq!(path.elements.len(), 2); assert_eq!(path.elements[0], PathElement::Root()); assert_eq!(path.elements[1], PathElement::Key("x".to_string())); @@ -131,7 +98,7 @@ mod tests { #[test] fn test_json_path_single_array_locator() { - let path = json_path("$[0]", true).unwrap(); + let path = json_path("$[0]").unwrap(); assert_eq!(path.elements.len(), 2); assert_eq!(path.elements[0], PathElement::Root()); assert_eq!(path.elements[1], PathElement::ArrayLocator(0)); @@ -139,7 +106,7 @@ mod tests { #[test] fn test_json_path_single_negative_array_locator() { - let path = json_path("$[#-2]", true).unwrap(); + let path = json_path("$[#-2]").unwrap(); assert_eq!(path.elements.len(), 2); assert_eq!(path.elements[0], PathElement::Root()); assert_eq!(path.elements[1], PathElement::ArrayLocator(-2)); @@ -152,7 +119,7 @@ mod tests { ]; for value in invalid_values { - let path = json_path(value, true); + let path = json_path(value); match path { Err(crate::error::LimboError::Constraint(e)) => { @@ -165,7 +132,7 @@ mod tests { #[test] fn test_json_path() { - let path = json_path("$.store.book[0].title", true).unwrap(); + let path = json_path("$.store.book[0].title").unwrap(); assert_eq!(path.elements.len(), 5); assert_eq!(path.elements[0], PathElement::Root()); assert_eq!(path.elements[1], PathElement::Key("store".to_string())); @@ -173,31 +140,4 @@ mod tests { assert_eq!(path.elements[3], PathElement::ArrayLocator(0)); assert_eq!(path.elements[4], PathElement::Key("title".to_string())); } - - #[test] - fn test_relaxed_json_path_array_locator() { - let path = json_path("1", false).unwrap(); - - assert_eq!(path.elements.len(), 2); - assert_eq!(path.elements[0], PathElement::Root()); - assert_eq!(path.elements[1], PathElement::ArrayLocator(1)); - } - - #[test] - fn test_relaxed_json_path_negative_array_locator() { - let path = json_path("-1", false).unwrap(); - - assert_eq!(path.elements.len(), 2); - assert_eq!(path.elements[0], PathElement::Root()); - assert_eq!(path.elements[1], PathElement::ArrayLocator(-1)); - } - - #[test] - fn test_relaxed_json_path_key() { - let path = json_path("x", false).unwrap(); - - assert_eq!(path.elements.len(), 2); - assert_eq!(path.elements[0], PathElement::Root()); - assert_eq!(path.elements[1], PathElement::Key("x".to_string())); - } } diff --git a/core/json/mod.rs b/core/json/mod.rs index 6a9e43a1d..59adc4bb2 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -6,7 +6,7 @@ mod ser; use std::rc::Rc; pub use crate::json::de::from_str; -use crate::json::json_path::{json_path, PathElement}; +use crate::json::json_path::{json_path, JsonPath, PathElement}; pub use crate::json::ser::to_string; use crate::types::{LimboText, OwnedValue, TextSubtype}; use indexmap::IndexMap; @@ -147,10 +147,15 @@ pub fn json_arrow_extract(value: &OwnedValue, path: &OwnedValue) -> crate::Resul } let json = get_json_value(value)?; - let extracted = json_extract_single(&json, path, false)?.unwrap_or_else(|| &Val::Null); - let json = crate::json::to_string(extracted).unwrap(); + let extracted = json_extract_single(&json, path, false)?; - Ok(OwnedValue::Text(LimboText::json(Rc::new(json)))) + if let Some(val) = extracted { + let json = crate::json::to_string(val).unwrap(); + + Ok(OwnedValue::Text(LimboText::json(Rc::new(json)))) + } else { + Ok(OwnedValue::Null) + } } /// Implements the ->> operator. Always returns a SQL representation of the JSON subcomponent. @@ -290,10 +295,32 @@ fn json_extract_single<'a>( path: &OwnedValue, strict: bool, ) -> crate::Result> { - let json_path = match path { - OwnedValue::Text(t) => json_path(t.value.as_str(), strict)?, - OwnedValue::Null => return Ok(None), - _ => crate::bail_constraint_error!("JSON path error near: {:?}", path.to_string()), + let json_path = if strict { + match path { + OwnedValue::Text(t) => json_path(t.value.as_str())?, + OwnedValue::Null => return Ok(None), + _ => crate::bail_constraint_error!("JSON path error near: {:?}", path.to_string()), + } + } else { + match path { + OwnedValue::Text(t) => { + if t.value.starts_with("$") { + json_path(t.value.as_str())? + } else { + JsonPath { + elements: vec![PathElement::Root(), PathElement::Key(t.value.to_string())], + } + } + } + OwnedValue::Null => return Ok(None), + OwnedValue::Integer(i) => JsonPath { + elements: vec![PathElement::Root(), PathElement::ArrayLocator(*i as i32)], + }, + OwnedValue::Float(f) => JsonPath { + elements: vec![PathElement::Root(), PathElement::Key(f.to_string())], + }, + _ => crate::bail_constraint_error!("JSON path error near: {:?}", path.to_string()), + } }; let mut current_element = &Val::Null; diff --git a/testing/json.test b/testing/json.test index d3724fe6a..7d0d93203 100755 --- a/testing/json.test +++ b/testing/json.test @@ -311,23 +311,25 @@ do_execsql_test json_arrow_implicit_real_cast { do_execsql_test json_arrow_shift_implicit_real_cast { SELECT '{"1.5":"abc"}' -> 1.5; -} {{abc}} +} {{"abc"}} -do_execsql_test json_arrow_implicit_true_cast { - SELECT '[1,2,3]' -> true -} {{2}} - -do_execsql_test json_arrow_shift_implicit_true_cast { - SELECT '[1,2,3]' ->> true -} {{2}} - -do_execsql_test json_arrow_implicit_false_cast { - SELECT '[1,2,3]' -> false -} {{1}} - -do_execsql_test json_arrow_shift_implicit_false_cast { - SELECT '[1,2,3]' ->> false -} {{1}} +# TODO: There is a bug in Limbo that prevents using the true/false in SELECT +# https://github.com/tursodatabase/limbo/issues/640 +#do_execsql_test json_arrow_implicit_true_cast { +# SELECT '[1,2,3]' -> true +#} {{2}} +# +#do_execsql_test json_arrow_shift_implicit_true_cast { +# SELECT '[1,2,3]' ->> true +#} {{2}} +# +#do_execsql_test json_arrow_implicit_false_cast { +# SELECT '[1,2,3]' -> false +#} {{1}} +# +#do_execsql_test json_arrow_shift_implicit_false_cast { +# SELECT '[1,2,3]' ->> false +#} {{1}} # TODO: fix me - this passes on SQLite and needs to be fixed in Limbo. do_execsql_test json_extract_multiple_null_paths { From 91d4ac3ac097ea89bf275120cfa4ca37d2d6190d Mon Sep 17 00:00:00 2001 From: Kacper Madej Date: Fri, 10 Jan 2025 12:14:57 +0700 Subject: [PATCH 05/10] Fix expression chaining --- core/json/mod.rs | 4 ++++ core/translate/expr.rs | 5 +++-- testing/json.test | 4 ++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/core/json/mod.rs b/core/json/mod.rs index 59adc4bb2..f0a362ab6 100644 --- a/core/json/mod.rs +++ b/core/json/mod.rs @@ -290,6 +290,10 @@ pub fn json_type(value: &OwnedValue, path: Option<&OwnedValue>) -> crate::Result /// Returns the value at the given JSON path. If the path does not exist, it returns None. /// If the path is an invalid path, returns an error. +/// +/// *strict* - if false, we will try to resolve the path even if it does not start with "$" +/// in a way that's compatible with the `->` and `->>` operators. See examples in the docs: +/// https://sqlite.org/json1.html#the_and_operators fn json_extract_single<'a>( json: &'a Val, path: &OwnedValue, diff --git a/core/translate/expr.rs b/core/translate/expr.rs index 6897308c0..f3834601e 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -457,9 +457,10 @@ pub fn translate_expr( match expr { ast::Expr::Between { .. } => todo!(), ast::Expr::Binary(e1, op, e2) => { - let e1_reg = program.alloc_register(); + let e1_reg = program.alloc_registers(2); + let e2_reg = e1_reg + 1; + translate_expr(program, referenced_tables, e1, e1_reg, resolver)?; - let e2_reg = program.alloc_register(); translate_expr(program, referenced_tables, e2, e2_reg, resolver)?; match op { diff --git a/testing/json.test b/testing/json.test index 7d0d93203..f8472db96 100755 --- a/testing/json.test +++ b/testing/json.test @@ -331,6 +331,10 @@ do_execsql_test json_arrow_shift_implicit_real_cast { # SELECT '[1,2,3]' ->> false #} {{1}} +do_execsql_test json_arrow_chained { + select '{"a":2,"c":[4,5,{"f":7}]}' -> 'c' -> 2 ->> 'f' +} {{7}} + # TODO: fix me - this passes on SQLite and needs to be fixed in Limbo. do_execsql_test json_extract_multiple_null_paths { SELECT json_extract(1, null, null, null) From 536fbe9d9ef8e0dfd3952b704c8e410a02943c25 Mon Sep 17 00:00:00 2001 From: Kacper Madej Date: Fri, 10 Jan 2025 12:24:08 +0700 Subject: [PATCH 06/10] Fix imports --- core/vdbe/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index 5e0f2b4a7..64c41b3bd 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -28,7 +28,6 @@ use crate::error::{LimboError, SQLITE_CONSTRAINT_PRIMARYKEY}; #[cfg(feature = "uuid")] use crate::ext::{exec_ts_from_uuid7, exec_uuid, exec_uuidblob, exec_uuidstr, ExtFunc, UuidFunc}; use crate::function::{AggFunc, FuncCtx, MathFunc, MathFuncArity, ScalarFunc}; -use crate::json::{json_arrow_extract, json_arrow_shift_extract}; use crate::pseudo::PseudoCursor; use crate::result::LimboResult; use crate::schema::Table; @@ -42,7 +41,7 @@ use crate::vdbe::insn::Insn; #[cfg(feature = "json")] use crate::{ function::JsonFunc, json::get_json, json::json_array, json::json_array_length, - json::json_extract, json::json_type, + json::json_arrow_extract, json::json_arrow_shift_extract, json::json_extract, json::json_type, }; use crate::{Connection, Result, Rows, TransactionState, DATABASE_VERSION}; use datetime::{exec_date, exec_datetime_full, exec_julianday, exec_time, exec_unixepoch}; From a2e1ef2439501c7054ec6c3d668931814333bcd0 Mon Sep 17 00:00:00 2001 From: Kacper Madej Date: Fri, 10 Jan 2025 13:19:15 +0700 Subject: [PATCH 07/10] Use newest SQLite on Github Actions --- .github/shared/install_sqlite/action.yml | 15 ++++++++++ .github/workflows/rust.yml | 36 +++++++++++------------- 2 files changed, 32 insertions(+), 19 deletions(-) create mode 100644 .github/shared/install_sqlite/action.yml diff --git a/.github/shared/install_sqlite/action.yml b/.github/shared/install_sqlite/action.yml new file mode 100644 index 000000000..f74f620f1 --- /dev/null +++ b/.github/shared/install_sqlite/action.yml @@ -0,0 +1,15 @@ +name: "Install SQLite" +description: "Downloads SQLite directly from https://sqlite.org" + +runs: + using: "composite" + steps: + - name: Install SQLite + env: + SQLITE_VERSION: "3470200" + YEAR: 2024 + run: | + curl -o /tmp/sqlite.zip https://www.sqlite.org/$YEAR/sqlite-tools-linux-x64-$SQLITE_VERSION.zip > /dev/null + unzip -j /tmp/sqlite.zip sqlite3 -d /usr/local/bin/ + sqlite3 --version + shell: bash diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 19596e140..bebf36794 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -59,31 +59,29 @@ jobs: bench: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 - - name: Bench - run: cargo bench + - uses: actions/checkout@v3 + - name: Bench + run: cargo bench test-limbo: runs-on: ubuntu-latest steps: - - name: Install sqlite - run: sudo apt update && sudo apt install -y sqlite3 libsqlite3-dev - - name: Install cargo-c - env: - LINK: https://github.com/lu-zero/cargo-c/releases/download/v0.10.7 - CARGO_C_FILE: cargo-c-x86_64-unknown-linux-musl.tar.gz - run: | - curl -L $LINK/$CARGO_C_FILE | tar xz -C ~/.cargo/bin + - name: Install cargo-c + env: + LINK: https://github.com/lu-zero/cargo-c/releases/download/v0.10.7 + CARGO_C_FILE: cargo-c-x86_64-unknown-linux-musl.tar.gz + run: | + curl -L $LINK/$CARGO_C_FILE | tar xz -C ~/.cargo/bin - - uses: actions/checkout@v3 - - name: Test - run: make test + - uses: actions/checkout@v3 + - uses: "./.github/shared/install_sqlite" + - name: Test + run: make test test-sqlite: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 - - name: Install sqlite - run: sudo apt update && sudo apt install -y sqlite3 libsqlite3-dev - - name: Test - run: SQLITE_EXEC="sqlite3" make test-compat + - uses: actions/checkout@v3 + - uses: "./.github/shared/install_sqlite" + - name: Test + run: SQLITE_EXEC="sqlite3" make test-compat From 1a856012619fb27cb45e97178d1753bb3a2e1d09 Mon Sep 17 00:00:00 2001 From: Kacper Madej Date: Fri, 10 Jan 2025 13:41:47 +0700 Subject: [PATCH 08/10] Update docs --- COMPAT.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/COMPAT.md b/COMPAT.md index 4ddabe10f..54949c91d 100644 --- a/COMPAT.md +++ b/COMPAT.md @@ -266,8 +266,8 @@ Feature support of [sqlite expr syntax](https://www.sqlite.org/lang_expr.html). | json_error_position(json) | | | | json_extract(json,path,...) | Partial | Does not fully support unicode literal syntax and does not allow numbers > 2^127 - 1 (which SQLite truncates to i32), does not support BLOBs | | jsonb_extract(json,path,...) | | | -| json -> path | | | -| json ->> path | | | +| json -> path | Yes | | +| json ->> path | Yes | | | json_insert(json,path,value,...) | | | | jsonb_insert(json,path,value,...) | | | | json_object(label1,value1,...) | | | From 1a46988a73b7cbe4b3537e904b4c073f71624dc6 Mon Sep 17 00:00:00 2001 From: Kacper Madej Date: Fri, 10 Jan 2025 14:50:38 +0700 Subject: [PATCH 09/10] Fix hardcoded numbers in macros --- core/translate/expr.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index f3834601e..c229e8501 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -70,8 +70,9 @@ macro_rules! expect_arguments_exact { let args = if let Some(args) = $args { if args.len() != $expected_arguments { crate::bail_parse_error!( - "{} function with not exactly 2 arguments", - $func.to_string() + "{} function called with not exactly {} arguments", + $func.to_string(), + $expected_arguments, ); } args @@ -92,8 +93,9 @@ macro_rules! expect_arguments_max { let args = if let Some(args) = $args { if args.len() > $expected_arguments { crate::bail_parse_error!( - "{} function with not exactly 2 arguments", - $func.to_string() + "{} function called with more than {} arguments", + $func.to_string(), + $expected_arguments, ); } args From 002d2e3dde8ae8dd860843a9c17c6bc954b2416f Mon Sep 17 00:00:00 2001 From: Kacper Madej Date: Fri, 10 Jan 2025 19:26:39 +0700 Subject: [PATCH 10/10] Uncomment failing tests --- testing/json.test | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/testing/json.test b/testing/json.test index f8472db96..a4df0e902 100755 --- a/testing/json.test +++ b/testing/json.test @@ -313,23 +313,21 @@ do_execsql_test json_arrow_shift_implicit_real_cast { SELECT '{"1.5":"abc"}' -> 1.5; } {{"abc"}} -# TODO: There is a bug in Limbo that prevents using the true/false in SELECT -# https://github.com/tursodatabase/limbo/issues/640 -#do_execsql_test json_arrow_implicit_true_cast { -# SELECT '[1,2,3]' -> true -#} {{2}} -# -#do_execsql_test json_arrow_shift_implicit_true_cast { -# SELECT '[1,2,3]' ->> true -#} {{2}} -# -#do_execsql_test json_arrow_implicit_false_cast { -# SELECT '[1,2,3]' -> false -#} {{1}} -# -#do_execsql_test json_arrow_shift_implicit_false_cast { -# SELECT '[1,2,3]' ->> false -#} {{1}} +do_execsql_test json_arrow_implicit_true_cast { + SELECT '[1,2,3]' -> true +} {{2}} + +do_execsql_test json_arrow_shift_implicit_true_cast { + SELECT '[1,2,3]' ->> true +} {{2}} + +do_execsql_test json_arrow_implicit_false_cast { + SELECT '[1,2,3]' -> false +} {{1}} + +do_execsql_test json_arrow_shift_implicit_false_cast { + SELECT '[1,2,3]' ->> false +} {{1}} do_execsql_test json_arrow_chained { select '{"a":2,"c":[4,5,{"f":7}]}' -> 'c' -> 2 ->> 'f'