diff --git a/src/datatypes/src/error.rs b/src/datatypes/src/error.rs index 4ca765222af..e897e984df3 100644 --- a/src/datatypes/src/error.rs +++ b/src/datatypes/src/error.rs @@ -288,6 +288,12 @@ pub enum Error { #[snafu(implicit)] location: Location, }, + + #[snafu(display("Non-object json is not supported currently"))] + UnsupportedJsonType { + #[snafu(implicit)] + location: Location, + }, } impl ErrorExt for Error { @@ -296,6 +302,7 @@ impl ErrorExt for Error { match self { UnsupportedOperation { .. } | UnsupportedArrowType { .. } + | UnsupportedJsonType { .. } | UnsupportedDefaultExpr { .. } => StatusCode::Unsupported, DuplicateColumn { .. } diff --git a/src/datatypes/src/json.rs b/src/datatypes/src/json.rs index 823ab2a6bb1..09238532c09 100644 --- a/src/datatypes/src/json.rs +++ b/src/datatypes/src/json.rs @@ -29,7 +29,7 @@ use serde_json::{Map, Value as Json}; use snafu::ResultExt; use crate::data_type::ConcreteDataType; -use crate::error::{self, Result}; +use crate::error::{self, Result, UnsupportedJsonTypeSnafu}; use crate::json::value::{JsonValue, JsonVariant}; use crate::schema::ColumnDefaultConstraint; use crate::types::json_type::JsonNativeType; @@ -120,6 +120,10 @@ impl<'a> JsonContext<'a> { /// Main encoding function with key path tracking pub fn encode_json_with_context<'a>(json: Json, context: &JsonContext<'a>) -> Result { + if context.path.is_empty() && !matches!(json, Json::Object(_)) { + return UnsupportedJsonTypeSnafu.fail(); + } + match json { Json::Object(json_object) => encode_json_object_with_context(json_object, context), Json::Array(json_array) => encode_json_array_with_context(json_array, context), @@ -536,61 +540,25 @@ mod tests { } #[test] - fn test_encode_json_null() { - let json = Json::Null; + fn test_encode_root_non_object_json() { let settings = JsonSettings::default(); - let result = settings.encode(json).unwrap().into_json_inner().unwrap(); - assert_eq!(result, Value::Null); - } + let cases = [ + ("null", Json::Null), + ("boolean", Json::Bool(true)), + ("integer", Json::from(42)), + ("float", Json::from(3.15)), + ("string", Json::String("hello".to_string())), + ("array", json!([1, 2, 3])), + ("mixed array", json!([1, "hello", true, 3.15])), + ("empty array", json!([])), + ]; - #[test] - fn test_encode_json_boolean() { - let json = Json::Bool(true); - let settings = JsonSettings::default(); - let result = settings.encode(json).unwrap().into_json_inner().unwrap(); - assert_eq!(result, Value::Boolean(true)); - } - - #[test] - fn test_encode_json_number_integer() { - let json = Json::from(42); - let settings = JsonSettings::default(); - let result = settings.encode(json).unwrap().into_json_inner().unwrap(); - assert_eq!(result, Value::Int64(42)); - } - - #[test] - fn test_encode_json_number_float() { - let json = Json::from(3.15); - let settings = JsonSettings::default(); - let result = settings.encode(json).unwrap().into_json_inner().unwrap(); - match result { - Value::Float64(f) => assert_eq!(f.0, 3.15), - _ => panic!("Expected Float64"), - } - } - - #[test] - fn test_encode_json_string() { - let json = Json::String("hello".to_string()); - let settings = JsonSettings::default(); - let result = settings.encode(json).unwrap().into_json_inner().unwrap(); - assert_eq!(result, Value::String("hello".into())); - } - - #[test] - fn test_encode_json_array() { - let json = json!([1, 2, 3]); - let settings = JsonSettings::default(); - let result = settings.encode(json).unwrap().into_json_inner().unwrap(); - - if let Value::List(list_value) = result { - assert_eq!(list_value.items().len(), 3); - assert_eq!(list_value.items()[0], Value::Int64(1)); - assert_eq!(list_value.items()[1], Value::Int64(2)); - assert_eq!(list_value.items()[2], Value::Int64(3)); - } else { - panic!("Expected List value"); + for (name, json) in cases { + let err = settings.encode(json).unwrap_err(); + assert!( + matches!(err, crate::error::Error::UnsupportedJsonType { .. }), + "{name}: {err:?}" + ); } } @@ -691,32 +659,6 @@ mod tests { } } - #[test] - fn test_encode_json_array_mixed_types() { - let json = json!([1, "hello", true, 3.15]); - let settings = JsonSettings::default(); - let value = settings.encode(json).unwrap(); - assert_eq!(value.data_type().to_string(), r#"Json2[""]"#); - } - - #[test] - fn test_encode_json_empty_array() { - let json = json!([]); - let settings = JsonSettings::default(); - let result = settings.encode(json).unwrap().into_json_inner().unwrap(); - - if let Value::List(list_value) = result { - assert_eq!(list_value.items().len(), 0); - // Empty arrays default to string type - assert_eq!( - list_value.datatype(), - Arc::new(ConcreteDataType::null_datatype()) - ); - } else { - panic!("Expected List value"); - } - } - #[test] fn test_encode_json_structured() { let json = json!({ @@ -1097,18 +1039,4 @@ mod tests { assert_eq!(result, expected); } } - - #[test] - fn test_encode_json_large_unsigned_integer() { - // Test unsigned integer that fits in i64 - let json = Json::from(u64::MAX / 2); - let settings = JsonSettings::default(); - let result = settings.encode(json).unwrap().into_json_inner().unwrap(); - assert_eq!(result, Value::Int64((u64::MAX / 2) as i64)); - - // Test unsigned integer that exceeds i64 range - let json = Json::from(u64::MAX); - let result = settings.encode(json).unwrap().into_json_inner().unwrap(); - assert_eq!(result, Value::UInt64(u64::MAX)); - } } diff --git a/src/datatypes/src/json/value.rs b/src/datatypes/src/json/value.rs index e23d25ddeee..b660894c2b7 100644 --- a/src/datatypes/src/json/value.rs +++ b/src/datatypes/src/json/value.rs @@ -791,7 +791,7 @@ impl<'a> JsonValueRef<'a> { } } - fn as_value_ref(&self) -> ValueRef<'_> { + pub fn as_value_ref(&self) -> ValueRef<'_> { fn helper<'a>(v: &'a JsonVariantRef) -> ValueRef<'a> { match v { JsonVariantRef::Null => ValueRef::Null, @@ -840,17 +840,6 @@ impl<'a> JsonValueRef<'a> { pub(crate) fn variant(&self) -> &JsonVariantRef<'a> { &self.json_variant } - - pub(crate) fn as_struct_value(&self) -> ValueRef<'_> { - if self.is_object() { - return self.as_value_ref(); - } - - ValueRef::Struct(StructValueRef::RefList { - val: vec![self.as_value_ref()], - fields: self.json_type().as_struct_type(), - }) - } } impl<'a, T: Into>> From for JsonValueRef<'a> { diff --git a/src/datatypes/src/types/json_type.rs b/src/datatypes/src/types/json_type.rs index d0a1f4bb6a0..38275279678 100644 --- a/src/datatypes/src/types/json_type.rs +++ b/src/datatypes/src/types/json_type.rs @@ -33,15 +33,13 @@ use crate::error::{ use crate::prelude::ConcreteDataType; use crate::scalars::ScalarVectorBuilder; use crate::type_id::LogicalTypeId; -use crate::types::{StructField, StructType}; +use crate::types::StructType; use crate::value::Value; use crate::vectors::json::builder::JsonVectorBuilder; use crate::vectors::{BinaryVectorBuilder, MutableVector}; pub const JSON_TYPE_NAME: &str = "Json"; const JSON2_TYPE_NAME: &str = "Json2"; -const JSON_PLAIN_FIELD_NAME: &str = "__json_plain__"; -const JSON_PLAIN_FIELD_METADATA_KEY: &str = "is_plain_json"; pub type JsonObjectType = BTreeMap; @@ -302,18 +300,16 @@ impl JsonType { } } - /// Make json type a struct type, by: - /// - if the json is an object, its entries are mapped to struct fields, obviously; - /// - if not, the json is one of bool, number, string or array, make it a special field - /// (see [plain_json_struct_type]). pub(crate) fn as_struct_type(&self) -> StructType { match &self.format { - JsonFormat::Jsonb => StructType::default(), JsonFormat::Json2(native_type) => match native_type.as_arrow_type() { // TODO(LFC): Direct use Arrow's Struct datatype here. ArrowDataType::Struct(fields) => StructType::from(&fields), - data_type => plain_json_struct_type(&data_type), + // FIXME(fys): Since writing with a non-object root is currently + // not supported, this temporarily returns default. + _ => StructType::default(), }, + JsonFormat::Jsonb => StructType::default(), } } @@ -370,18 +366,6 @@ fn is_include(this: &JsonNativeType, that: &JsonNativeType) -> bool { } } -/// A special struct type for denoting "plain"(not object) json value. It has only one field, with -/// fixed name [JSON_PLAIN_FIELD_NAME] and with metadata [JSON_PLAIN_FIELD_METADATA_KEY] = `"true"`. -fn plain_json_struct_type(data_type: &ArrowDataType) -> StructType { - let mut field = StructField::new( - JSON_PLAIN_FIELD_NAME.to_string(), - ConcreteDataType::from_arrow_type(data_type), - true, - ); - field.insert_metadata(JSON_PLAIN_FIELD_METADATA_KEY, true); - StructType::new(Arc::new(vec![field])) -} - impl From<&ArrowDataType> for JsonType { fn from(t: &ArrowDataType) -> Self { JsonType::new_json2(JsonNativeType::from(&ConcreteDataType::from_arrow_type(t))) @@ -517,7 +501,6 @@ pub fn parse_string_to_jsonb(s: &str) -> Result> { #[cfg(test)] mod tests { use super::*; - use crate::json::JsonSettings; #[test] fn test_fix_unicode_point() -> Result<()> { @@ -771,20 +754,11 @@ mod tests { #[test] fn test_merge_json_type() -> Result<()> { fn test( - json: &str, + other: JsonType, json_type: &mut JsonType, expected: std::result::Result<&str, &str>, ) -> Result<()> { - let json: serde_json::Value = serde_json::from_str(json).unwrap(); - - let settings = JsonSettings::default(); - let value = settings.encode(json)?; - let value_type = value.data_type(); - let Some(other) = value_type.as_json() else { - unreachable!() - }; - - let result = json_type.merge(other); + let result = json_type.merge(&other); match (result, expected) { (Ok(()), Ok(expected)) => { assert_eq!(json_type.native_type().to_string(), expected); @@ -798,32 +772,39 @@ mod tests { } // Null should be absorbed by a concrete scalar type. - test("true", &mut JsonType::null(), Ok(r#""""#))?; + test( + JsonType::new_json2(JsonNativeType::Bool), + &mut JsonType::null(), + Ok(r#""""#), + )?; // Merging a null value into an existing concrete type should keep the type unchanged. test( - "null", + JsonType::null(), &mut JsonType::new_json2(JsonNativeType::Bool), Ok(r#""""#), )?; // Identical number categories should stay as Number. test( - "1", + JsonType::new_json2(JsonNativeType::i64()), &mut JsonType::new_json2(JsonNativeType::i64()), Ok(r#""""#), )?; // Conflicting number categories should be lifted to Variant. test( - "1.5", + JsonType::new_json2(JsonNativeType::f64()), &mut JsonType::new_json2(JsonNativeType::i64()), Ok(r#""""#), )?; // Object merge should preserve existing fields and append missing fields. test( - r#"{"foo":"x"}"#, + JsonType::new_json2(JsonNativeType::Object(JsonObjectType::from([( + "foo".to_string(), + JsonNativeType::String, + )]))), &mut JsonType::new_json2(JsonNativeType::Object(JsonObjectType::from([( "bar".to_string(), JsonNativeType::i64(), @@ -833,7 +814,10 @@ mod tests { // Conflicting object field types should only lift that field to Variant. test( - r#"{"foo":1}"#, + JsonType::new_json2(JsonNativeType::Object(JsonObjectType::from([( + "foo".to_string(), + JsonNativeType::i64(), + )]))), &mut JsonType::new_json2(JsonNativeType::Object(JsonObjectType::from([( "foo".to_string(), JsonNativeType::Bool, @@ -843,7 +827,13 @@ mod tests { // Nested objects should merge recursively. test( - r#"{"nested":{"foo":"bar"}}"#, + JsonType::new_json2(JsonNativeType::Object(JsonObjectType::from([( + "nested".to_string(), + JsonNativeType::Object(JsonObjectType::from([( + "foo".to_string(), + JsonNativeType::String, + )])), + )]))), &mut JsonType::new_json2(JsonNativeType::Object(JsonObjectType::from([( "nested".to_string(), JsonNativeType::Object(JsonObjectType::from([( @@ -856,21 +846,24 @@ mod tests { // Arrays should merge their element types recursively. test( - r#"["foo"]"#, + JsonType::new_json2(JsonNativeType::Array(Box::new(JsonNativeType::String))), &mut JsonType::new_json2(JsonNativeType::Array(Box::new(JsonNativeType::u64()))), Ok(r#"[""]"#), )?; // Root-level incompatible types should be lifted to Variant. test( - r#"{"foo":"bar"}"#, + JsonType::new_json2(JsonNativeType::Object(JsonObjectType::from([( + "foo".to_string(), + JsonNativeType::String, + )]))), &mut JsonType::new_json2(JsonNativeType::Bool), Ok(r#""""#), )?; // Jsonb and Json2 should not be mergeable. test( - "true", + JsonType::new_json2(JsonNativeType::Bool), &mut JsonType::new(JsonFormat::Jsonb), Err("Failed to merge JSON datatype: json format not match"), )?; diff --git a/src/datatypes/src/types/struct_type.rs b/src/datatypes/src/types/struct_type.rs index 2fa7a9d91d4..ffd1d1c0b25 100644 --- a/src/datatypes/src/types/struct_type.rs +++ b/src/datatypes/src/types/struct_type.rs @@ -146,10 +146,6 @@ impl StructField { self.nullable } - pub(crate) fn insert_metadata(&mut self, key: impl ToString, value: impl ToString) { - self.metadata.insert(key.to_string(), value.to_string()); - } - #[expect(unused)] pub(crate) fn metadata(&self, key: &str) -> Option<&str> { self.metadata.get(key).map(String::as_str) diff --git a/src/datatypes/src/vectors/json/builder.rs b/src/datatypes/src/vectors/json/builder.rs index f98a4c9a436..c3a7474a80d 100644 --- a/src/datatypes/src/vectors/json/builder.rs +++ b/src/datatypes/src/vectors/json/builder.rs @@ -19,7 +19,7 @@ use crate::error::{Result, TryFromValueSnafu, UnsupportedOperationSnafu}; use crate::json::value::{JsonValue, JsonVariant}; use crate::prelude::{ValueRef, Vector, VectorRef}; use crate::types::JsonType; -use crate::types::json_type::JsonNativeType; +use crate::types::json_type::{JsonFormat, JsonNativeType}; use crate::vectors::{MutableVector, StructVectorBuilder}; #[derive(Clone)] @@ -30,6 +30,10 @@ pub(crate) struct JsonVectorBuilder { impl JsonVectorBuilder { pub(crate) fn new(initial_native_type: JsonNativeType, capacity: usize) -> Self { + debug_assert!(matches!( + initial_native_type, + JsonNativeType::Object(_) | JsonNativeType::Null + )); Self { merged_type: JsonType::new_json2(initial_native_type), values: Vec::with_capacity(capacity), @@ -43,14 +47,11 @@ impl JsonVectorBuilder { ); for value in self.values.iter_mut() { value.try_align(&self.merged_type)?; - if value.is_null() { builder.push_null(); continue; } - - let value = value.as_ref(); - builder.try_push_value_ref(&value.as_struct_value())?; + builder.try_push_value_ref(&value.as_ref().as_value_ref())?; } Ok(builder.to_vector()) } @@ -89,6 +90,16 @@ impl MutableVector for JsonVectorBuilder { .fail(); }; let json_type = value.json_type(); + if !matches!( + json_type.format, + JsonFormat::Json2(ref native_type) + if matches!(native_type.as_ref(), JsonNativeType::Object(_) | JsonNativeType::Null) + ) { + return TryFromValueSnafu { + reason: format!("expected json object value, got {value:?}"), + } + .fail(); + } if !self.merged_type.is_include(json_type) { self.merged_type.merge(json_type)?; } @@ -200,39 +211,24 @@ mod tests { )) ); - // Root-level conflicts should be lifted to a plain Variant field that preserves - // each original JSON payload. - let mut variant_builder = JsonVectorBuilder::new(JsonNativeType::Bool, 2); + // Non-object initial types are rejected by the builder invariant. + let result = std::panic::catch_unwind(|| JsonVectorBuilder::new(JsonNativeType::Bool, 2)); + assert!(result.is_err()); + + // Non-object root values should be rejected at push time. + let mut object_builder = + JsonVectorBuilder::new(JsonNativeType::Object(Default::default()), 2); let object = parse_json_value(r#"{"k":1}"#); let boolean = parse_json_value("true"); - variant_builder.try_push_value_ref(&boolean.as_value_ref())?; - variant_builder.try_push_value_ref(&object.as_value_ref())?; - - let variant_type = JsonType::new_json2(JsonNativeType::Variant); - assert_eq!( - variant_builder.data_type(), - ConcreteDataType::Json(variant_type.clone()) - ); - - let variant_struct_type = variant_type.as_struct_type(); - let vector = variant_builder.to_vector(); - assert_eq!( - vector.get(0), - Value::Struct(StructValue::new( - vec![Value::Binary(Bytes::from(b"true".to_vec()))], - variant_struct_type.clone(), - )) - ); - assert_eq!( - vector.get(1), - Value::Struct(StructValue::new( - vec![Value::Binary(Bytes::from(br#"{"k":1}"#.to_vec()))], - variant_struct_type, - )) - ); + let err = object_builder + .try_push_value_ref(&boolean.as_value_ref()) + .unwrap_err(); + assert!(err.to_string().contains("expected json object value")); + object_builder.try_push_value_ref(&object.as_value_ref())?; // Non-JSON values should be rejected at push time. - let mut invalid_builder = JsonVectorBuilder::new(JsonNativeType::Bool, 1); + let mut invalid_builder = + JsonVectorBuilder::new(JsonNativeType::Object(Default::default()), 1); let err = invalid_builder .try_push_value_ref(&ValueRef::Boolean(true)) .unwrap_err(); diff --git a/tests/cases/standalone/common/types/json/json2.result b/tests/cases/standalone/common/types/json/json2.result index 84539b50fb4..d397966c214 100644 --- a/tests/cases/standalone/common/types/json/json2.result +++ b/tests/cases/standalone/common/types/json/json2.result @@ -8,6 +8,26 @@ create table json2_table ( Affected Rows: 0 +insert into json2_table (ts, j) values (101, '[1, 2, 3]'); + +Error: 1001(Unsupported), Non-object json is not supported currently + +insert into json2_table (ts, j) values (102, '"hello"'); + +Error: 1001(Unsupported), Non-object json is not supported currently + +insert into json2_table (ts, j) values (103, '42'); + +Error: 1001(Unsupported), Non-object json is not supported currently + +insert into json2_table (ts, j) values (104, 'true'); + +Error: 1001(Unsupported), Non-object json is not supported currently + +insert into json2_table (ts, j) values (105, 'null'); + +Error: 1001(Unsupported), Non-object json is not supported currently + insert into json2_table (ts, j) values (1, '{"a": {"b": 1}, "c": "s1", "d": [{"e": {"f": 0.1}}]}'), (2, '{"a": {"b": -2}, "c": "s2", "d": [{"e": {"f": 0.2}}]}'); @@ -153,37 +173,11 @@ select j.c, j.y from json2_table order by ts; select j from json2_table order by ts; -+--------------------+ -| j | -+--------------------+ -| {__json_plain__: } | -| {__json_plain__: } | -| {__json_plain__: } | -| {__json_plain__: } | -| {__json_plain__: } | -| {__json_plain__: } | -| {__json_plain__: } | -| {__json_plain__: } | -| {__json_plain__: } | -| {__json_plain__: } | -+--------------------+ +Error: 3001(EngineExecuteQuery), Failed to align JSON array, reason: Invalid argument error: use StructArray::try_new_with_length or StructArray::new_empty_fields to create a struct array with no fields so that the length can be set correctly select * from json2_table order by ts; -+-------------------------+--------------------+ -| ts | j | -+-------------------------+--------------------+ -| 1970-01-01T00:00:00.001 | {__json_plain__: } | -| 1970-01-01T00:00:00.002 | {__json_plain__: } | -| 1970-01-01T00:00:00.003 | {__json_plain__: } | -| 1970-01-01T00:00:00.004 | {__json_plain__: } | -| 1970-01-01T00:00:00.005 | {__json_plain__: } | -| 1970-01-01T00:00:00.006 | {__json_plain__: } | -| 1970-01-01T00:00:00.007 | {__json_plain__: } | -| 1970-01-01T00:00:00.008 | {__json_plain__: } | -| 1970-01-01T00:00:00.009 | {__json_plain__: } | -| 1970-01-01T00:00:00.010 | {__json_plain__: } | -+-------------------------+--------------------+ +Error: 3001(EngineExecuteQuery), Failed to align JSON array, reason: Invalid argument error: use StructArray::try_new_with_length or StructArray::new_empty_fields to create a struct array with no fields so that the length can be set correctly select j.a.b + 1 from json2_table order by ts; diff --git a/tests/cases/standalone/common/types/json/json2.sql b/tests/cases/standalone/common/types/json/json2.sql index 51be5fbbe0c..e13f861e221 100644 --- a/tests/cases/standalone/common/types/json/json2.sql +++ b/tests/cases/standalone/common/types/json/json2.sql @@ -6,6 +6,16 @@ create table json2_table ( 'sst_format' = 'flat', ); +insert into json2_table (ts, j) values (101, '[1, 2, 3]'); + +insert into json2_table (ts, j) values (102, '"hello"'); + +insert into json2_table (ts, j) values (103, '42'); + +insert into json2_table (ts, j) values (104, 'true'); + +insert into json2_table (ts, j) values (105, 'null'); + insert into json2_table (ts, j) values (1, '{"a": {"b": 1}, "c": "s1", "d": [{"e": {"f": 0.1}}]}'), (2, '{"a": {"b": -2}, "c": "s2", "d": [{"e": {"f": 0.2}}]}');