From 697cc5fa29e43aed63f4a72531ce5ac83a6d8e21 Mon Sep 17 00:00:00 2001 From: LFC <990479+MichaelScofield@users.noreply.github.com> Date: Fri, 18 Sep 2026 04:55:02 +0000 Subject: [PATCH] fix(json): fix JSONPath panic with jsonb 0.5.6 (follow-up to #9192) (#9228) * fix(json): upgrade jsonb to fix unterminated JSONPath panic Signed-off-by: luofucong * fix(json): align integer extraction with JSON2 conversions Signed-off-by: luofucong * style: format jsonb dependency declaration Signed-off-by: luofucong * fix(otlp): report concrete unsupported JSONB type names Signed-off-by: luofucong --------- Signed-off-by: luofucong --- Cargo.lock | 82 ++++++++- Cargo.toml | 4 +- .../function/src/scalars/json/json_get.rs | 160 +++++++++++++----- .../function/src/scalars/json/json_is.rs | 21 ++- .../src/scalars/json/json_path_exists.rs | 2 +- .../src/scalars/json/json_path_match.rs | 10 +- .../function/src/scalars/json/parse_json.rs | 2 +- src/common/recordbatch/src/adapter.rs | 2 +- src/datatypes/src/json.rs | 2 +- src/datatypes/src/types/json_type.rs | 13 +- src/operator/src/req_convert/common.rs | 2 +- src/servers/src/otlp/logs.rs | 110 ++++++++++++ .../common/function/json/json_get.result | 81 ++++++++- .../common/function/json/json_get.sql | 33 ++++ tests/compatibility/cases/jsonb_0_5/case.toml | 8 + tests/compatibility/cases/jsonb_0_5/setup.sql | 7 + .../cases/jsonb_0_5/verify.result | 49 ++++++ .../compatibility/cases/jsonb_0_5/verify.sql | 20 +++ 18 files changed, 535 insertions(+), 73 deletions(-) create mode 100644 tests/compatibility/cases/jsonb_0_5/case.toml create mode 100644 tests/compatibility/cases/jsonb_0_5/setup.sql create mode 100644 tests/compatibility/cases/jsonb_0_5/verify.result create mode 100644 tests/compatibility/cases/jsonb_0_5/verify.sql diff --git a/Cargo.lock b/Cargo.lock index e8c03e616cf..b2a7b3e1035 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5202,6 +5202,12 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "ethnum" +version = "1.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "40404c3f5f511ec4da6fe866ddf6a717c309fdbb69fbbad7b0f3edab8f2e835f" + [[package]] name = "event-listener" version = "2.5.3" @@ -6355,6 +6361,12 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" +[[package]] +name = "hifijson" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "242402749acf71e6f32f5857598b7002c4058a4e3c3b22b4c7d51cab9aea754e" + [[package]] name = "hkdf" version = "0.12.4" @@ -7269,6 +7281,46 @@ version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4a5f13b858c8d314ee3e8f639011f7ccefe71f97f96e50151fb991f267928e2c" +[[package]] +name = "jaq-core" +version = "3.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f72ba7158fe9fc1bc4baace1b71e6d0d3d27c6839dd87dc1f233dd408f5dd85b" +dependencies = [ + "dyn-clone", + "once_cell", + "typed-arena", +] + +[[package]] +name = "jaq-json" +version = "2.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4be2f53207ffb1313a2d2ad819cfa8bfaa7936cb85a110eda10fd5da822cec94" +dependencies = [ + "bstr", + "bytes", + "foldhash 0.1.5", + "hifijson", + "indexmap 2.14.2", + "jaq-core", + "jaq-std", + "num-bigint 0.4.6", + "num-traits", + "ryu", + "self_cell", +] + +[[package]] +name = "jaq-std" +version = "3.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f90e485abfd3189510c996e77be55056a51038b0fa5805e6c97b5dcf0d9ab55" +dependencies = [ + "bstr", + "jaq-core", +] + [[package]] name = "jieba-macros" version = "0.10.1" @@ -7432,18 +7484,22 @@ dependencies = [ [[package]] name = "jsonb" -version = "0.4.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "acd7dc2490b13d09367f5dc4bf202a5d70958dd5b9b2758e2708ee062752a824" +version = "0.5.6" +source = "git+https://github.com/GreptimeTeam/jsonb?rev=dd1e2341e3df35e9de38959328caa7628e81b6b1#dd1e2341e3df35e9de38959328caa7628e81b6b1" dependencies = [ "byteorder", + "ethnum", "fast-float2", "itoa", - "nom 7.1.3", - "ordered-float 4.6.0", - "rand 0.8.5", - "ryu", + "jaq-json", + "jiff", + "nom 8.0.0", + "num-traits", + "ordered-float 5.3.0", + "rand 0.9.4", + "serde", "serde_json", + "zmij", ] [[package]] @@ -13290,6 +13346,12 @@ dependencies = [ "libc", ] +[[package]] +name = "self_cell" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2ab42ca02749e120097e328d91d415325bdf43b1c72c4c8badf37375fe40a813" + [[package]] name = "semver" version = "1.0.27" @@ -15910,6 +15972,12 @@ version = "2.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b907da542cbced5261bd3256de1b3a1bf340a3d37f93425a07362a1d687de56" +[[package]] +name = "typed-arena" +version = "2.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6af6ae20167a9ece4bcb41af5b80f8a1f1df981f6391189ce00fd257af04126a" + [[package]] name = "typeid" version = "1.0.3" diff --git a/Cargo.toml b/Cargo.toml index 52b98de5557..f1b5577d391 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -172,7 +172,9 @@ hyper-util = "0.1" icu_properties = "2.0.1" indicatif = "0.17" itertools = "0.14" -jsonb = { version = "0.4.4", default-features = false } +jsonb = { git = "https://github.com/GreptimeTeam/jsonb", rev = "dd1e2341e3df35e9de38959328caa7628e81b6b1", default-features = false, features = [ + "databend", +] } lazy_static = "1.4" local-ip-address = "0.6" loki-proto = { git = "https://github.com/GreptimeTeam/loki-proto.git", rev = "f69c8924c4babe516373e26a4118be82d976629c" } diff --git a/src/common/function/src/scalars/json/json_get.rs b/src/common/function/src/scalars/json/json_get.rs index ddddf11f010..9a3a1ec8616 100644 --- a/src/common/function/src/scalars/json/json_get.rs +++ b/src/common/function/src/scalars/json/json_get.rs @@ -18,8 +18,7 @@ use arrow::array::{ArrayRef, BinaryViewArray, new_null_array}; use arrow::compute; use arrow_schema::Field; use datafusion_common::arrow::array::{ - Array, AsArray, BinaryViewBuilder, BooleanBuilder, Float64Builder, Int64Builder, - StringViewBuilder, + Array, AsArray, BinaryViewBuilder, BooleanBuilder, Float64Builder, StringViewBuilder, }; use datafusion_common::arrow::datatypes::DataType; use datafusion_common::{DataFusionError, Result, ScalarValue, exec_datafusion_err, exec_err}; @@ -40,8 +39,6 @@ pub fn parse_json_get_path(path: &str) -> std::result::Result, json paths: vec![Path::Root], }); } - // TODO(LFC): Fix jsonb's unterminated quoted-field panic (e.g. `$."a`) and - // update the dependency; malformed paths must return InvalidJsonPath. parse_json_path(path.as_bytes()) } @@ -74,7 +71,6 @@ fn result_builder(len: usize, with_type: &DataType) -> Result, - DataType::Int64 => Box::new(IntResultBuilder(Int64Builder::with_capacity(len))), DataType::Float64 => Box::new(FloatResultBuilder(Float64Builder::with_capacity(len))), DataType::Boolean => Box::new(BoolResultBuilder(BooleanBuilder::with_capacity(len))), t => { @@ -91,7 +87,8 @@ struct StringResultBuilder { impl JsonGetResultBuilder for StringResultBuilder { fn append_value(&mut self, value: &[u8]) -> Result<()> { - self.inner.append_option(jsonb::to_str(value).ok()); + self.inner + .append_option(jsonb::RawJsonb::new(value).to_str().ok()); Ok(()) } @@ -147,23 +144,6 @@ impl Function for JsonGetString { } } -struct IntResultBuilder(Int64Builder); - -impl JsonGetResultBuilder for IntResultBuilder { - fn append_value(&mut self, value: &[u8]) -> Result<()> { - self.0.append_option(jsonb::to_i64(value).ok()); - Ok(()) - } - - fn append_null(&mut self) { - self.0.append_null(); - } - - fn build(&mut self) -> ArrayRef { - Arc::new(self.0.finish()) - } -} - #[derive(Default, Display, Debug)] #[display("{}", Self::NAME.to_ascii_uppercase())] pub struct JsonGetInt(JsonGetWithType); @@ -196,7 +176,8 @@ struct FloatResultBuilder(Float64Builder); impl JsonGetResultBuilder for FloatResultBuilder { fn append_value(&mut self, value: &[u8]) -> Result<()> { - self.0.append_option(jsonb::to_f64(value).ok()); + self.0 + .append_option(jsonb::RawJsonb::new(value).to_f64().ok()); Ok(()) } @@ -241,7 +222,8 @@ struct BoolResultBuilder(BooleanBuilder); impl JsonGetResultBuilder for BoolResultBuilder { fn append_value(&mut self, value: &[u8]) -> Result<()> { - self.0.append_option(jsonb::to_bool(value).ok()); + self.0 + .append_option(jsonb::RawJsonb::new(value).to_bool().ok()); Ok(()) } @@ -295,14 +277,13 @@ fn jsonb_get( builder.append_null(); continue; } - let mut value = Vec::new(); - let mut offsets = Vec::new(); - jsonb::get_by_path(jsons.value(i), json_path.clone(), &mut value, &mut offsets) + let value = jsonb::RawJsonb::new(jsons.value(i)) + .select_value_by_path(&json_path) .map_err(|e| exec_datafusion_err!("Failed to evaluate JSONPath {path:?}: {e}"))?; - if value.is_empty() { - builder.append_null(); + if let Some(value) = value { + builder.append_value(value.as_ref())?; } else { - builder.append_value(&value)?; + builder.append_null(); } } Ok(()) @@ -440,7 +421,8 @@ impl Function for JsonGetWithType { let arg0 = compute::cast(&arg0, &DataType::BinaryView)?; let is_json2 = args.arg_fields.first().is_some_and(is_json2_extension_type); - if is_json2 { + // Share integer conversion with JSON2: truncate floats and reject decimal strings. + if is_json2 || with_type == DataType::Int64 { let mut builder = BinaryViewBuilder::with_capacity(len); jsonb_get(arg0.as_binary_view(), path, &mut builder)?; JsonArray::from(&builder.build()) @@ -519,10 +501,14 @@ impl Function for JsonGetObject { let path = paths.is_valid(i).then(|| paths.value(i)); let result = if let (Some(json), Some(path)) = (json, path) { let result = jsonb::jsonpath::parse_json_path(path.as_bytes()).and_then(|path| { - let mut data = Vec::new(); - let mut offset = Vec::new(); - jsonb::get_by_path(json, path, &mut data, &mut offset) - .map(|()| jsonb::is_object(&data).then_some(data)) + let Some(value) = jsonb::RawJsonb::new(json).select_value_by_path(&path)? + else { + return Ok(None); + }; + value + .as_raw() + .is_object() + .map(|is_object| is_object.then(|| value.to_vec())) }); result.map_err(|e| DataFusionError::Execution(e.to_string()))? } else { @@ -550,6 +536,55 @@ mod tests { use super::*; + #[test] + fn test_json_get_int_conversion() -> Result<()> { + let cases = [ + ("1", Some(1)), + ("1.0", Some(1)), + ("1.5", Some(1)), + ("-1.5", Some(-1)), + ("9223372036854775807", Some(i64::MAX)), + ("-9223372036854775808", Some(i64::MIN)), + ("18446744073709551615", None), + (r#""42""#, Some(42)), + (r#""1.5""#, None), + ("true", Some(1)), + ("null", None), + ]; + let jsons = cases + .iter() + .map(|(input, _)| { + jsonb::parse_value_standard_mode(input.as_bytes()) + .map(|value| value.to_vec()) + .map_err(|e| exec_datafusion_err!("{e}")) + }) + .collect::>>()?; + let array: ArrayRef = Arc::new(BinaryArray::from_iter_values(jsons)); + for is_json2 in [false, true] { + let result = JsonGetInt::default() + .invoke_with_args(ScalarFunctionArgs { + args: vec![ + ColumnarValue::Array(array.clone()), + ColumnarValue::Scalar("$".into()), + ], + arg_fields: vec![test_json_field(&array, is_json2)], + number_rows: array.len(), + return_field: Arc::new(Field::new("x", DataType::Int64, true)), + config_options: Arc::new(Default::default()), + })? + .into_array(array.len())?; + let actual = result.as_primitive::(); + for (i, (input, expected)) in cases.iter().enumerate() { + assert_eq!( + actual.is_valid(i).then(|| actual.value(i)), + *expected, + "{input}, is_json2={is_json2}" + ); + } + } + Ok(()) + } + #[test] fn test_parse_json_get_path_expression() -> std::result::Result<(), Box> { @@ -594,6 +629,28 @@ mod tests { Ok(()) } + #[test] + fn test_parse_json_get_path_unterminated_quotes() { + for path in [ + r#"$."a"#, + r#""a"#, + r#"$["a"#, + r#"$."a\""#, + r#"$."a\\"#, + r#"$."\u0061"#, + r#"$."中文"#, + r#"$.a ? (@ == "value"#, + ] { + assert!( + matches!( + parse_json_get_path(path), + Err(jsonb::Error::InvalidJsonPath) + ), + "{path}", + ); + } + } + #[test] fn test_parse_json_get_path_quoted_keys() { for path in [ @@ -760,7 +817,7 @@ mod tests { fn test_jsonb_get_evaluation_error() -> std::result::Result<(), Box> { let json = parse_string_to_jsonb(r#"{"a": 1}"#)?; let truncated = BinaryViewArray::from_iter_values([&json[..1]]); - let mut builder = IntResultBuilder(Int64Builder::new()); + let mut builder = BinaryViewBuilder::new(); let err = jsonb_get(&truncated, "$.a", &mut builder).unwrap_err(); assert!( err.to_string().contains("Failed to evaluate JSONPath"), @@ -908,21 +965,25 @@ mod tests { } #[test] - fn test_json_get_bool() { + fn test_json_get_bool() -> Result<()> { let json_get_bool = JsonGetBool::default(); assert_eq!("json_get_bool", json_get_bool.name()); assert_eq!( DataType::Boolean, - json_get_bool - .return_type(&[DataType::Binary, DataType::Utf8]) - .unwrap() + json_get_bool.return_type(&[DataType::Binary, DataType::Utf8])? ); let json_strings = [ r#"{"a": {"b": true}, "b": false, "c": true}"#, r#"{"a": false, "b": {"c": true}, "c": false}"#, r#"{"a": true, "b": false, "c": {"a": true}}"#, + r#""yes""#, + r#""no""#, + r#""YeS""#, + r#""nO""#, + r#""invalid""#, + r#""""#, ]; let json_struct = test_json_struct(); @@ -930,6 +991,12 @@ mod tests { ("$.a.b", Some(true)), ("$.a", Some(false)), ("$.c", None), + ("$", Some(true)), + ("$", Some(false)), + ("$", Some(true)), + ("$", Some(false)), + ("$", None), + ("$", None), ("$.kind", None), ("$.payload.code", Some(true)), ("$.payload.success", Some(false)), @@ -942,10 +1009,11 @@ mod tests { let mut jsons = json_strings .iter() .map(|s| { - let value = jsonb::parse_value(s.as_bytes()).unwrap(); - Arc::new(BinaryArray::from_iter_values([value.to_vec()])) as ArrayRef + let value = jsonb::parse_value_standard_mode(s.as_bytes()) + .map_err(|e| exec_datafusion_err!("{e}"))?; + Ok(Arc::new(BinaryArray::from_iter_values([value.to_vec()])) as ArrayRef) }) - .collect::>(); + .collect::>>()?; let json_struct_arrays = std::iter::repeat_n(json_struct, path_expects.len() - jsons.len()).collect::>(); jsons.extend(json_struct_arrays); @@ -966,14 +1034,14 @@ mod tests { }; let result = json_get_bool .invoke_with_args(args) - .and_then(|x| x.to_array(1)) - .unwrap(); + .and_then(|x| x.to_array(1))?; let result = result.as_boolean(); assert_eq!(1, result.len()); let actual = result.is_valid(0).then(|| result.value(0)); assert_eq!(actual, expect); } + Ok(()) } #[test] diff --git a/src/common/function/src/scalars/json/json_is.rs b/src/common/function/src/scalars/json/json_is.rs index 40a36e642de..7f3604ccdf0 100644 --- a/src/common/function/src/scalars/json/json_is.rs +++ b/src/common/function/src/scalars/json/json_is.rs @@ -25,6 +25,11 @@ use crate::function::{Function, extract_args}; /// Checks if the input is a JSON object of the given type. macro_rules! json_is { ($name:ident, $json_type:ident, $doc:expr) => { + paste::paste! { + json_is!($name, $doc, jsonb::RawJsonb::[]); + } + }; + ($name:ident, $doc:expr, $check:path) => { paste::paste! { #[derive(Clone, Debug)] pub(crate) struct $name { @@ -72,7 +77,7 @@ macro_rules! json_is { let json = jsons.is_valid(i).then(|| jsons.value(i)); let result = match json { Some(json) => { - Some(jsonb::[](json)) + Some($check(&jsonb::RawJsonb::new(json)).map_err(|e| datafusion_common::DataFusionError::Execution(e.to_string()))?) } _ => None, }; @@ -92,6 +97,16 @@ macro_rules! json_is { }; } +// `RawJsonb::is_i64` also accepts integral floats in jsonb 0.5. Preserve the +// SQL type predicate: 1.0 is a float even when it can be converted to i64. +fn is_jsonb_integer(value: &jsonb::RawJsonb<'_>) -> Result { + Ok(match value.as_number()? { + Some(jsonb::Number::Int64(_)) => true, + Some(jsonb::Number::UInt64(value)) => i64::try_from(value).is_ok(), + _ => false, + }) +} + json_is!(JsonIsNull, null, "Checks if the input JSONB is null"); json_is!( JsonIsBool, @@ -100,8 +115,8 @@ json_is!( ); json_is!( JsonIsInt, - i64, - "Checks if the input JSONB is a integer type JSON value" + "Checks if the input JSONB is an integer type JSON value", + is_jsonb_integer ); json_is!( JsonIsFloat, diff --git a/src/common/function/src/scalars/json/json_path_exists.rs b/src/common/function/src/scalars/json/json_path_exists.rs index ad55480f6ff..4f86764a8b8 100644 --- a/src/common/function/src/scalars/json/json_path_exists.rs +++ b/src/common/function/src/scalars/json/json_path_exists.rs @@ -88,7 +88,7 @@ impl Function for JsonPathExistsFunction { ))); } }; - jsonb::path_exists(json, json_path).ok() + jsonb::RawJsonb::new(json).path_exists(&json_path).ok() } _ => None, }; diff --git a/src/common/function/src/scalars/json/json_path_match.rs b/src/common/function/src/scalars/json/json_path_match.rs index 2c8a6e8aea6..0a2cf69354b 100644 --- a/src/common/function/src/scalars/json/json_path_match.rs +++ b/src/common/function/src/scalars/json/json_path_match.rs @@ -75,10 +75,16 @@ impl Function for JsonPathMatchFunction { let result = match (json, path) { (Some(json), Some(path)) => { - if !jsonb::is_null(json) { + if !jsonb::RawJsonb::new(json) + .is_null() + .map_err(|e| datafusion_common::DataFusionError::Execution(e.to_string()))? + { let json_path = jsonb::jsonpath::parse_json_path(path.as_bytes()); match json_path { - Ok(json_path) => jsonb::path_match(json, json_path).ok(), + Ok(json_path) => jsonb::RawJsonb::new(json) + .path_match(&json_path) + .ok() + .flatten(), Err(_) => None, } } else { diff --git a/src/common/function/src/scalars/json/parse_json.rs b/src/common/function/src/scalars/json/parse_json.rs index fb1d1095cdb..0a327481415 100644 --- a/src/common/function/src/scalars/json/parse_json.rs +++ b/src/common/function/src/scalars/json/parse_json.rs @@ -67,7 +67,7 @@ impl Function for ParseJsonFunction { let s = json_strings.is_valid(i).then(|| json_strings.value(i)); let result = s .map(|s| { - jsonb::parse_value(s.as_bytes()) + jsonb::parse_value_standard_mode(s.as_bytes()) .map(|x| x.to_vec()) .map_err(|e| DataFusionError::Execution(format!("cannot parse '{s}': {e}"))) }) diff --git a/src/common/recordbatch/src/adapter.rs b/src/common/recordbatch/src/adapter.rs index e10bf440833..a7f6e4a10ac 100644 --- a/src/common/recordbatch/src/adapter.rs +++ b/src/common/recordbatch/src/adapter.rs @@ -907,7 +907,7 @@ fn convert_map_to_json_binary( ))); } }; - match jsonb::parse_value(json_string.as_bytes()) { + match jsonb::parse_value_standard_mode(json_string.as_bytes()) { Ok(jsonb_value) => jsonb_value.to_vec(), Err(e) => { return Err(ArrowError::CastError(format!( diff --git a/src/datatypes/src/json.rs b/src/datatypes/src/json.rs index 0afe5637340..e156e387330 100644 --- a/src/datatypes/src/json.rs +++ b/src/datatypes/src/json.rs @@ -604,7 +604,7 @@ fn decode_primitive_value(value: Value) -> Result { Value::Float32(v) => Ok(Json::from(v.0)), Value::Float64(v) => Ok(Json::from(v.0)), Value::String(s) => Ok(Json::String(s.as_utf8().to_string())), - Value::Binary(b) => serde_json::to_value(b.as_ref()).context(error::SerializeSnafu), + Value::Binary(b) => serde_json::to_value(b).context(error::SerializeSnafu), Value::Date(v) => Ok(Json::from(v.val())), Value::Timestamp(v) => serde_json::to_value(v.value()).context(error::SerializeSnafu), Value::Time(v) => serde_json::to_value(v.value()).context(error::SerializeSnafu), diff --git a/src/datatypes/src/types/json_type.rs b/src/datatypes/src/types/json_type.rs index 337ac415187..3e629c2bfd1 100644 --- a/src/datatypes/src/types/json_type.rs +++ b/src/datatypes/src/types/json_type.rs @@ -404,7 +404,9 @@ pub fn jsonb_to_string(val: &[u8]) -> Result { /// Converts a json type value to serde_json::Value pub fn jsonb_to_serde_json(val: &[u8]) -> Result { - jsonb::to_serde_json(val).map_err(|error| InvalidJsonbSnafu { error }.build()) + jsonb::from_slice(val) + .map(Into::into) + .map_err(|error| InvalidJsonbSnafu { error }.build()) } /// Normalizes a JSON string by converting Rust-style Unicode escape sequences to JSON-compatible format. @@ -462,13 +464,20 @@ fn fix_unicode_point(json: &str) -> Result { /// Parses a string to a json type value pub fn parse_string_to_jsonb(s: &str) -> Result> { - jsonb::parse_value(s.as_bytes()) + jsonb::parse_value_standard_mode(s.as_bytes()) .map_err(|_| InvalidJsonSnafu { value: s }.build()) .map(|json| json.to_vec()) } #[cfg(test)] mod tests { + #[test] + fn test_parse_json_rejects_extended_syntax() { + for input in ["+1", "01", ".5", "[1,,2]", "{'a':1}"] { + assert!(parse_string_to_jsonb(input).is_err(), "{input}"); + } + } + #[test] fn test_v2_jsonb_to_serde_json_escaped_keys() -> std::result::Result<(), Box> { diff --git a/src/operator/src/req_convert/common.rs b/src/operator/src/req_convert/common.rs index e41c3505be2..d10a57ccb95 100644 --- a/src/operator/src/req_convert/common.rs +++ b/src/operator/src/req_convert/common.rs @@ -39,7 +39,7 @@ use crate::error::{ /// Encodes a string value as JSONB binary data if the value is of `StringValue` type. fn encode_string_to_jsonb_binary(value_data: ValueData) -> Result { if let ValueData::StringValue(json) = &value_data { - let binary = jsonb::parse_value(json.as_bytes()) + let binary = jsonb::parse_value_standard_mode(json.as_bytes()) .map_err(|_| InvalidJsonFormatSnafu { json }.build()) .map(|jsonb| jsonb.to_vec())?; Ok(ValueData::BinaryValue(binary)) diff --git a/src/servers/src/otlp/logs.rs b/src/servers/src/otlp/logs.rs index 74f1a3d03a6..f32816fbe36 100644 --- a/src/servers/src/otlp/logs.rs +++ b/src/servers/src/otlp/logs.rs @@ -549,6 +549,13 @@ fn decide_column_schema_and_convert_value( key: column_name, } .fail(), + JsonbNumber::Decimal64(_) | JsonbNumber::Decimal128(_) | JsonbNumber::Decimal256(_) => { + UnsupportedJsonDataTypeForTagSnafu { + ty: "DECIMAL".to_string(), + key: column_name, + } + .fail() + } JsonbNumber::UInt64(u) => { let value = jsonb_uint64_to_log_value(u, column_name)?; Ok(Some(( @@ -574,6 +581,31 @@ fn decide_column_schema_and_convert_value( key: column_name, } .fail(), + JsonbValue::Binary(_) => UnsupportedJsonDataTypeForTagSnafu { + ty: "Binary".to_string(), + key: column_name, + } + .fail(), + JsonbValue::Date(_) => UnsupportedJsonDataTypeForTagSnafu { + ty: "Date".to_string(), + key: column_name, + } + .fail(), + JsonbValue::Timestamp(_) => UnsupportedJsonDataTypeForTagSnafu { + ty: "Timestamp".to_string(), + key: column_name, + } + .fail(), + JsonbValue::TimestampTz(_) => UnsupportedJsonDataTypeForTagSnafu { + ty: "TimestampTz".to_string(), + key: column_name, + } + .fail(), + JsonbValue::Interval(_) => UnsupportedJsonDataTypeForTagSnafu { + ty: "Interval".to_string(), + key: column_name, + } + .fail(), JsonbValue::Null => Ok(None), }; column_info.map(|c| { @@ -653,6 +685,13 @@ fn jsonb_value_to_log_value_data( key: column_name, } .fail(), + JsonbNumber::Decimal64(_) | JsonbNumber::Decimal128(_) | JsonbNumber::Decimal256(_) => { + UnsupportedJsonDataTypeForTagSnafu { + ty: "DECIMAL".to_string(), + key: column_name, + } + .fail() + } JsonbNumber::UInt64(u) => Ok(Some(( ValueData::I64Value(jsonb_uint64_to_log_value(u, column_name)?), ColumnDataType::Int64, @@ -664,6 +703,31 @@ fn jsonb_value_to_log_value_data( key: column_name, } .fail(), + JsonbValue::Binary(_) => UnsupportedJsonDataTypeForTagSnafu { + ty: "Binary".to_string(), + key: column_name, + } + .fail(), + JsonbValue::Date(_) => UnsupportedJsonDataTypeForTagSnafu { + ty: "Date".to_string(), + key: column_name, + } + .fail(), + JsonbValue::Timestamp(_) => UnsupportedJsonDataTypeForTagSnafu { + ty: "Timestamp".to_string(), + key: column_name, + } + .fail(), + JsonbValue::TimestampTz(_) => UnsupportedJsonDataTypeForTagSnafu { + ty: "TimestampTz".to_string(), + key: column_name, + } + .fail(), + JsonbValue::Interval(_) => UnsupportedJsonDataTypeForTagSnafu { + ty: "Interval".to_string(), + key: column_name, + } + .fail(), JsonbValue::Null => Ok(None), } } @@ -1484,6 +1548,52 @@ mod tests { ); } + #[test] + fn test_jsonb_extended_log_values_rejected() { + for (value, expected_type) in [ + ( + JsonbValue::Number(JsonbNumber::Decimal64(jsonb::Decimal64 { + value: 123, + scale: 2, + })), + "DECIMAL", + ), + (JsonbValue::Binary(&[1, 2, 3]), "Binary"), + (JsonbValue::Date(jsonb::Date { value: 0 }), "Date"), + ( + JsonbValue::Timestamp(jsonb::Timestamp { value: 0 }), + "Timestamp", + ), + ( + JsonbValue::TimestampTz(jsonb::TimestampTz { + value: 0, + offset: 0, + }), + "TimestampTz", + ), + ( + JsonbValue::Interval(jsonb::Interval { + months: 0, + days: 0, + micros: 0, + }), + "Interval", + ), + ] { + let schema_error = + decide_column_schema_and_convert_value("attr", value.clone(), None, "logs") + .expect_err("extended JSONB type must be rejected"); + let value_error = jsonb_value_to_log_value_data("attr", value, true) + .expect_err("extended JSONB type must be rejected"); + for error in [schema_error, value_error] { + assert!(matches!(error, + Error::UnsupportedJsonDataTypeForTag { key, ty, .. } + if key == "attr" && ty == expected_type + )); + } + } + } + #[test] fn test_jsonb_uint64_exceeding_i64_range_rejected() { // A JSON number that only fits in u64 must be rejected, not wrapped: diff --git a/tests/cases/standalone/common/function/json/json_get.result b/tests/cases/standalone/common/function/json/json_get.result index 0eb5aa05c97..d730902ad5f 100644 --- a/tests/cases/standalone/common/function/json/json_get.result +++ b/tests/cases/standalone/common/function/json/json_get.result @@ -12,6 +12,71 @@ SELECT json_get_int(parse_json('{"a": 1}'), '$.missing'); | | +--------------------------------------------------------------+ +-- Unterminated quoted fields must return errors without panicking. +SELECT json_get_int(parse_json('{"a": 1}'), '$."a'); + +Error: 3001(EngineExecuteQuery), Execution error: Invalid JSONPath "$.\"a": InvalidJsonPath + +SELECT json_get_object(parse_json('{"a": {}}'), '$["a'); + +Error: 3001(EngineExecuteQuery), Execution error: InvalidJsonPath + +CREATE TABLE jsonpath_unterminated(j JSON2, ts TIMESTAMP TIME INDEX) WITH (append_mode='true'); + +Affected Rows: 0 + +INSERT INTO jsonpath_unterminated VALUES ('{"a": 1}', 1); + +Affected Rows: 1 + +SELECT json_get(j, '$."a') FROM jsonpath_unterminated; + +Error: 3000(PlanQuery), Error during planning: Invalid JSONPath "$.\"a": InvalidJsonPath + +SELECT json_get(j, '$."a"')::BIGINT FROM jsonpath_unterminated; + ++-------------------------------------------------+ +| json_get(jsonpath_unterminated.j,Utf8("$."a"")) | ++-------------------------------------------------+ +| 1 | ++-------------------------------------------------+ + +DROP TABLE jsonpath_unterminated; + +Affected Rows: 0 + +-- JSON and JSON2 share integer conversion rules. +CREATE TABLE json_integer_conversion(j JSON, j2 JSON2, ts TIMESTAMP TIME INDEX) +WITH (append_mode='true'); + +Affected Rows: 0 + +INSERT INTO json_integer_conversion VALUES + ('{"a":1.5}', '{"a":1.5}', 1), + ('{"a":-1.5}', '{"a":-1.5}', 2), + ('{"a":"1.5"}', '{"a":"1.5"}', 3); + +Affected Rows: 3 + +SELECT + json_get(j, 'a')::BIGINT AS json_int, + json_get(j2, 'a')::BIGINT AS json2_int, + json_get_int(j, 'a') AS json_get_int, + json_get_int(j2, 'a') AS json2_get_int +FROM json_integer_conversion ORDER BY ts; + ++----------+-----------+--------------+---------------+ +| json_int | json2_int | json_get_int | json2_get_int | ++----------+-----------+--------------+---------------+ +| 1 | 1 | 1 | 1 | +| -1 | -1 | -1 | -1 | +| | | | | ++----------+-----------+--------------+---------------+ + +DROP TABLE json_integer_conversion; + +Affected Rows: 0 + SELECT json_get_int(parse_json('{"a": {"b": {"c": 1}}}'), 'a.b.c'); +------------------------------------------------------------------------+ @@ -111,7 +176,7 @@ SELECT json_get_int(j, 'a.b.c') FROM jsons; | json_get_int(jsons.j,Utf8("a.b.c")) | +-------------------------------------+ | 1 | -| | +| 1 | | | | 1 | +-------------------------------------+ @@ -155,7 +220,7 @@ SELECT json_get_int(j, 'a.b["c"]') FROM jsons; | json_get_int(jsons.j,Utf8("a.b["c"]")) | +----------------------------------------+ | 1 | -| | +| 1 | | | | 1 | +----------------------------------------+ @@ -230,7 +295,7 @@ SELECT json_get_int(j, '[0]') FROM jsons; | | | 1 | | 1 | -| | +| 1 | | | +-----------------------------------+ @@ -266,7 +331,7 @@ SELECT json_get_string(j, '[3]') FROM jsons; | Long time ago, there is a little pig flying in the sky | | false | | 2147483648 | -| 1e100 | +| 1e+100 | | | +--------------------------------------------------------+ @@ -314,7 +379,7 @@ SELECT json_get(j, '[0]')::int8 FROM jsons; | | | 1 | | 1 | -| | +| 1 | | | +---------------------------------------------------------+ @@ -338,7 +403,7 @@ SELECT json_get(j, '[1]')::int8 FROM jsons; | | | 0 | | 0 | -| | +| 3 | | | +---------------------------------------------------------+ @@ -374,7 +439,7 @@ SELECT json_get(j, '[3]')::string FROM jsons; | Long time ago, there is a little pig flying in the sky | | false | | 2147483648 | -| 1e100 | +| 1e+100 | | | +--------------------------------------------------------+ @@ -409,6 +474,7 @@ SELECT json_to_string(j) FROM jsons WHERE json_get_int(j, 'a.b.c') = 1; | json_to_string(jsons.j) | +-------------------------+ | {"a":{"b":{"c":1}}} | +| {"a":{"b":{"c":1.234}}} | | {"a":{"b":{"c":true}}} | +-------------------------+ @@ -442,6 +508,7 @@ SELECT json_to_string(j) FROM jsons WHERE CAST(json_get_int(j, 'a.b.c') AS BOOLE | json_to_string(jsons.j) | +-------------------------+ | {"a":{"b":{"c":1}}} | +| {"a":{"b":{"c":1.234}}} | | {"a":{"b":{"c":true}}} | +-------------------------+ diff --git a/tests/cases/standalone/common/function/json/json_get.sql b/tests/cases/standalone/common/function/json/json_get.sql index 18734e60ef8..7ced0169b15 100644 --- a/tests/cases/standalone/common/function/json/json_get.sql +++ b/tests/cases/standalone/common/function/json/json_get.sql @@ -4,6 +4,39 @@ SELECT json_get_int(parse_json('{"a": 1}'), '$.a['); SELECT json_get_int(parse_json('{"a": 1}'), '$.missing'); +-- Unterminated quoted fields must return errors without panicking. +SELECT json_get_int(parse_json('{"a": 1}'), '$."a'); + +SELECT json_get_object(parse_json('{"a": {}}'), '$["a'); + +CREATE TABLE jsonpath_unterminated(j JSON2, ts TIMESTAMP TIME INDEX) WITH (append_mode='true'); + +INSERT INTO jsonpath_unterminated VALUES ('{"a": 1}', 1); + +SELECT json_get(j, '$."a') FROM jsonpath_unterminated; + +SELECT json_get(j, '$."a"')::BIGINT FROM jsonpath_unterminated; + +DROP TABLE jsonpath_unterminated; + +-- JSON and JSON2 share integer conversion rules. +CREATE TABLE json_integer_conversion(j JSON, j2 JSON2, ts TIMESTAMP TIME INDEX) +WITH (append_mode='true'); + +INSERT INTO json_integer_conversion VALUES + ('{"a":1.5}', '{"a":1.5}', 1), + ('{"a":-1.5}', '{"a":-1.5}', 2), + ('{"a":"1.5"}', '{"a":"1.5"}', 3); + +SELECT + json_get(j, 'a')::BIGINT AS json_int, + json_get(j2, 'a')::BIGINT AS json2_int, + json_get_int(j, 'a') AS json_get_int, + json_get_int(j2, 'a') AS json2_get_int +FROM json_integer_conversion ORDER BY ts; + +DROP TABLE json_integer_conversion; + SELECT json_get_int(parse_json('{"a": {"b": {"c": 1}}}'), 'a.b.c'); SELECT json_get_float(parse_json('{"a": {"b": {"c": 1.234}}}'), 'a:b.c'); diff --git a/tests/compatibility/cases/jsonb_0_5/case.toml b/tests/compatibility/cases/jsonb_0_5/case.toml new file mode 100644 index 00000000000..8803e2112d7 --- /dev/null +++ b/tests/compatibility/cases/jsonb_0_5/case.toml @@ -0,0 +1,8 @@ +name = "jsonb_0_5" +reason = "Verify jsonb 0.5 can read persisted JSONB from older binaries and query old and newly written rows together." +introduced_by = "jsonb 0.5 upgrade" +topologies = ["standalone", "distributed"] +from_range = ["*"] +to_range = [">=v1.3.0-alpha.1"] +features = ["json", "table"] +owner = "query" diff --git a/tests/compatibility/cases/jsonb_0_5/setup.sql b/tests/compatibility/cases/jsonb_0_5/setup.sql new file mode 100644 index 00000000000..107de8fd0b4 --- /dev/null +++ b/tests/compatibility/cases/jsonb_0_5/setup.sql @@ -0,0 +1,7 @@ +CREATE TABLE jsonb_upgrade (ts TIMESTAMP TIME INDEX, j JSON); + +INSERT INTO jsonb_upgrade VALUES + (1, parse_json('{"a.b":{"name":"旧数据","items":[1,2,3]},"n":-42,"f":1.5,"big":18446744073709551615,"flag":true,"nil":null}')), + (2, parse_json('{"a.b":{"name":"escaped \"quote\"","items":[]},"n":0,"f":0.125,"big":0,"flag":false,"nil":null}')); + +ADMIN FLUSH_TABLE('jsonb_upgrade'); diff --git a/tests/compatibility/cases/jsonb_0_5/verify.result b/tests/compatibility/cases/jsonb_0_5/verify.result new file mode 100644 index 00000000000..f5e0e1d6310 --- /dev/null +++ b/tests/compatibility/cases/jsonb_0_5/verify.result @@ -0,0 +1,49 @@ +SELECT ts, json_get_int(j, 'n') AS n, + json_get_float(j, 'f') AS f, + json_get_string(j, 'big') AS big, + json_get_bool(j, 'flag') AS flag, + json_to_string(json_get_object(j, '$."a.b"')) AS nested +FROM jsonb_upgrade ORDER BY ts; + ++-------------------------+-----+-------+----------------------+-------+-----------------------------------------+ +| ts | n | f | big | flag | nested | ++-------------------------+-----+-------+----------------------+-------+-----------------------------------------+ +| 1970-01-01T00:00:00.001 | -42 | 1.5 | 18446744073709551615 | true | {"items":[1,2,3],"name":"旧数据"} | +| 1970-01-01T00:00:00.002 | 0 | 0.125 | 0 | false | {"items":[],"name":"escaped \"quote\""} | ++-------------------------+-----+-------+----------------------+-------+-----------------------------------------+ + +INSERT INTO jsonb_upgrade VALUES + (3, parse_json('{"a.b":{"name":"new","items":[4,5]},"n":42,"f":2.5,"big":18446744073709551615,"flag":true,"nil":null}')); + +Affected Rows: 1 + +ADMIN FLUSH_TABLE('jsonb_upgrade'); + ++------------------------------------+ +| ADMIN FLUSH_TABLE('jsonb_upgrade') | ++------------------------------------+ +| 0 | ++------------------------------------+ + +SELECT ts, json_get_int(j, 'n') AS n, + json_get_float(j, 'f') AS f, + json_get_string(j, 'big') AS big, + json_get_bool(j, 'flag') AS flag, + json_to_string(json_get_object(j, '$."a.b"')) AS nested +FROM jsonb_upgrade ORDER BY ts; + ++-------------------------+-----+-------+----------------------+-------+-----------------------------------------+ +| ts | n | f | big | flag | nested | ++-------------------------+-----+-------+----------------------+-------+-----------------------------------------+ +| 1970-01-01T00:00:00.001 | -42 | 1.5 | 18446744073709551615 | true | {"items":[1,2,3],"name":"旧数据"} | +| 1970-01-01T00:00:00.002 | 0 | 0.125 | 0 | false | {"items":[],"name":"escaped \"quote\""} | +| 1970-01-01T00:00:00.003 | 42 | 2.5 | 18446744073709551615 | true | {"items":[4,5],"name":"new"} | ++-------------------------+-----+-------+----------------------+-------+-----------------------------------------+ + +SELECT SUM(json_get_int(j, 'n')) AS total FROM jsonb_upgrade; + ++-------+ +| total | ++-------+ +| 0 | ++-------+ diff --git a/tests/compatibility/cases/jsonb_0_5/verify.sql b/tests/compatibility/cases/jsonb_0_5/verify.sql new file mode 100644 index 00000000000..56986d3d516 --- /dev/null +++ b/tests/compatibility/cases/jsonb_0_5/verify.sql @@ -0,0 +1,20 @@ +SELECT ts, json_get_int(j, 'n') AS n, + json_get_float(j, 'f') AS f, + json_get_string(j, 'big') AS big, + json_get_bool(j, 'flag') AS flag, + json_to_string(json_get_object(j, '$."a.b"')) AS nested +FROM jsonb_upgrade ORDER BY ts; + +INSERT INTO jsonb_upgrade VALUES + (3, parse_json('{"a.b":{"name":"new","items":[4,5]},"n":42,"f":2.5,"big":18446744073709551615,"flag":true,"nil":null}')); + +ADMIN FLUSH_TABLE('jsonb_upgrade'); + +SELECT ts, json_get_int(j, 'n') AS n, + json_get_float(j, 'f') AS f, + json_get_string(j, 'big') AS big, + json_get_bool(j, 'flag') AS flag, + json_to_string(json_get_object(j, '$."a.b"')) AS nested +FROM jsonb_upgrade ORDER BY ts; + +SELECT SUM(json_get_int(j, 'n')) AS total FROM jsonb_upgrade;