Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement json_error_position #564

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion COMPAT.md
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ Feature support of [sqlite expr syntax](https://www.sqlite.org/lang_expr.html).
| jsonb_array(value1,value2,...) | | |
| json_array_length(json) | Yes | |
| json_array_length(json,path) | Yes | |
| json_error_position(json) | | |
| json_error_position(json) | Yes | |
| json_extract(json,path,...) | | |
| jsonb_extract(json,path,...) | | |
| json -> path | | |
Expand Down
4 changes: 4 additions & 0 deletions core/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub enum JsonFunc {
Json,
JsonArray,
JsonArrayLength,
JsonErrorPosition,
}

#[cfg(feature = "json")]
Expand All @@ -19,6 +20,7 @@ impl Display for JsonFunc {
Self::Json => "json".to_string(),
Self::JsonArray => "json_array".to_string(),
Self::JsonArrayLength => "json_array_length".to_string(),
Self::JsonErrorPosition => "json_error_position".to_string(),
}
)
}
Expand Down Expand Up @@ -335,6 +337,8 @@ impl Func {
"json_array_length" => Ok(Self::Json(JsonFunc::JsonArrayLength)),
#[cfg(feature = "json")]
"json_array" => Ok(Self::Json(JsonFunc::JsonArray)),
#[cfg(feature = "json")]
"json_error_position" => Ok(Self::Json(JsonFunc::JsonErrorPosition)),
"unixepoch" => Ok(Self::Scalar(ScalarFunc::UnixEpoch)),
"hex" => Ok(Self::Scalar(ScalarFunc::Hex)),
"unhex" => Ok(Self::Scalar(ScalarFunc::Unhex)),
Expand Down
84 changes: 84 additions & 0 deletions core/json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ mod ser;
use std::rc::Rc;

pub use crate::json::de::from_str;
use crate::json::error::Error as JsonError;
pub use crate::json::ser::to_string;
use crate::types::{LimboText, OwnedValue, TextSubtype};
use indexmap::IndexMap;
use jsonb::Error as JsonbError;
use path::get_json_val_by_path;
use serde::{Deserialize, Serialize};

Expand Down Expand Up @@ -133,6 +135,32 @@ pub fn json_array_length(
Ok(OwnedValue::Integer(0))
}

pub fn json_error_position(json: &OwnedValue) -> crate::Result<OwnedValue> {
match json {
OwnedValue::Text(t) => match crate::json::from_str::<Val>(&t.value) {
Ok(_) => Ok(OwnedValue::Integer(0)),
Err(JsonError::Message { location, .. }) => {
if let Some(loc) = location {
Ok(OwnedValue::Integer(loc.column as i64))
} else {
Err(crate::error::LimboError::InternalError(
"failed to determine json error position".into(),
))
}
}
},
OwnedValue::Blob(b) => match jsonb::from_slice(b) {
Ok(_) => Ok(OwnedValue::Integer(0)),
Err(JsonbError::Syntax(_, pos)) => Ok(OwnedValue::Integer(pos as i64)),
_ => Err(crate::error::LimboError::InternalError(
"failed to determine json error position".into(),
)),
},
OwnedValue::Null => Ok(OwnedValue::Null),
_ => Ok(OwnedValue::Integer(0)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how will this handle OwnedValue::Agg and OwnedValue:Record here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my testing, sqlite just returns 0 for anything not text (or blob that could be interpreted as text). I stuck to the same plan, but perhaps using unreachable! would be better for these other, unsupported types. That's how json_array handles it.

sqlite> SELECT json_error_position(avg(price)) FROM products;
0

}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -428,4 +456,60 @@ mod tests {
panic!("Expected OwnedValue::Integer");
}
}

#[test]
fn test_json_error_position_no_error() {
let input = OwnedValue::build_text(Rc::new("[1,2,3]".to_string()));
let result = json_error_position(&input).unwrap();
assert_eq!(result, OwnedValue::Integer(0));
}

#[test]
fn test_json_error_position_no_error_more() {
let input = OwnedValue::build_text(Rc::new(r#"{"a":55,"b":72 , }"#.to_string()));
let result = json_error_position(&input).unwrap();
assert_eq!(result, OwnedValue::Integer(0));
}

#[test]
fn test_json_error_position_object() {
let input = OwnedValue::build_text(Rc::new(r#"{"a":55,"b":72,,}"#.to_string()));
let result = json_error_position(&input).unwrap();
assert_eq!(result, OwnedValue::Integer(16));
}

#[test]
fn test_json_error_position_array() {
let input = OwnedValue::build_text(Rc::new(r#"["a",55,"b",72,,]"#.to_string()));
let result = json_error_position(&input).unwrap();
assert_eq!(result, OwnedValue::Integer(16));
}

petersooley marked this conversation as resolved.
Show resolved Hide resolved
#[test]
fn test_json_error_position_null() {
let input = OwnedValue::Null;
let result = json_error_position(&input).unwrap();
assert_eq!(result, OwnedValue::Null);
}

#[test]
fn test_json_error_position_integer() {
let input = OwnedValue::Integer(5);
let result = json_error_position(&input).unwrap();
assert_eq!(result, OwnedValue::Integer(0));
}

#[test]
fn test_json_error_position_float() {
let input = OwnedValue::Float(-5.5);
let result = json_error_position(&input).unwrap();
assert_eq!(result, OwnedValue::Integer(0));
}

#[test]
fn test_json_error_position_blob() {
let input = OwnedValue::Blob(Rc::new(r#"["a",55,"b",72,,]"#.as_bytes().to_owned()));
let result = json_error_position(&input).unwrap();
assert_eq!(result, OwnedValue::Integer(16));
}
}
31 changes: 31 additions & 0 deletions core/translate/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,37 @@ pub fn translate_expr(
});
Ok(target_register)
}
JsonFunc::JsonErrorPosition => {
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 json_reg = program.alloc_register();
translate_expr(
program,
referenced_tables,
&args[0],
json_reg,
precomputed_exprs_to_registers,
)?;
program.emit_insn(Insn::Function {
constant_mask: 0,
start_reg: json_reg,
dest: target_register,
func: func_ctx,
});
Ok(target_register)
}
},
Func::Scalar(srf) => {
match srf {
Expand Down
13 changes: 12 additions & 1 deletion core/vdbe/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ use crate::types::{
use crate::util::parse_schema_rows;
use crate::vdbe::insn::Insn;
#[cfg(feature = "json")]
use crate::{function::JsonFunc, json::get_json, json::json_array, json::json_array_length};
use crate::{
function::JsonFunc, json::get_json, json::json_array, json::json_array_length,
json::json_error_position,
};
use crate::{Connection, Result, Rows, TransactionState, DATABASE_VERSION};
use datetime::{exec_date, exec_time, exec_unixepoch};
use likeop::{construct_like_escape_arg, exec_like_with_escape};
Expand Down Expand Up @@ -1920,6 +1923,14 @@ impl Program {
Err(e) => return Err(e),
}
}
#[cfg(feature = "json")]
crate::function::Func::Json(JsonFunc::JsonErrorPosition) => {
let json_value = &state.registers[*start_reg];
match json_error_position(json_value) {
Ok(pos) => state.registers[*dest] = pos,
Err(e) => return Err(e),
}
}
crate::function::Func::Scalar(scalar_func) => match scalar_func {
ScalarFunc::Cast => {
assert!(arg_count == 2);
Expand Down
34 changes: 33 additions & 1 deletion testing/json.test
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,36 @@ do_execsql_test json_array_length_via_index_not_array {

do_execsql_test json_array_length_via_bad_prop {
SELECT json_array_length('{"one":[1,2,3]}', '$.two');
} {{}}
} {{}}

do_execsql_test json_error_position_valid {
SELECT json_error_position('{"a":55,"b":72,}');
} {{0}}

do_execsql_test json_error_position_valid_ws {
SELECT json_error_position('{"a":55,"b":72 , }');
} {{0}}

do_execsql_test json_error_position_object {
SELECT json_error_position('{"a":55,"b":72,,}');
} {{16}}

do_execsql_test json_error_position_array_valid {
SELECT json_error_position('["a",55,"b",72,]');
} {{0}}

do_execsql_test json_error_position_array_valid_ws {
SELECT json_error_position('["a",55,"b",72 , ]');
} {{0}}

do_execsql_test json_error_position_array {
SELECT json_error_position('["a",55,"b",72,,]');
} {{16}}

do_execsql_test json_error_position_null {
SELECT json_error_position(NULL);
} {{}}

do_execsql_test json_error_position_complex {
SELECT json_error_position('{a:null,{"h":[1,[1,2,3]],"j":"abc"}:true}');
} {{9}}
Loading