feat(json2): reject non-object JSON values on write (#8381)

* feat: disable non object json write

* remove unused code

* fix: sqlness test

* simply code

* add comment

* remove unused unit test

* fix: cargo fmt

* fix: unit test

* fix: unit test
This commit is contained in:
fys
2026-07-01 03:12:09 +00:00
committed by GitHub
parent f684479d9c
commit 1fea548c17
8 changed files with 128 additions and 215 deletions
+7
View File
@@ -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 { .. }
+22 -94
View File
@@ -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<JsonValue> {
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["<Variant>"]"#);
}
#[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));
}
}
+1 -12
View File
@@ -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<JsonVariantRef<'a>>> From<T> for JsonValueRef<'a> {
+36 -43
View File
@@ -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<String, JsonNativeType>;
@@ -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<Vec<u8>> {
#[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#""<Bool>""#))?;
test(
JsonType::new_json2(JsonNativeType::Bool),
&mut JsonType::null(),
Ok(r#""<Bool>""#),
)?;
// 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#""<Bool>""#),
)?;
// Identical number categories should stay as Number.
test(
"1",
JsonType::new_json2(JsonNativeType::i64()),
&mut JsonType::new_json2(JsonNativeType::i64()),
Ok(r#""<Number>""#),
)?;
// Conflicting number categories should be lifted to Variant.
test(
"1.5",
JsonType::new_json2(JsonNativeType::f64()),
&mut JsonType::new_json2(JsonNativeType::i64()),
Ok(r#""<Number>""#),
)?;
// 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#"["<Variant>"]"#),
)?;
// 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#""<Variant>""#),
)?;
// 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"),
)?;
-4
View File
@@ -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)
+30 -34
View File
@@ -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();
@@ -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;
@@ -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}}]}');