From d7f1233f775d54380318d6ad9ff62504a7cbcff1 Mon Sep 17 00:00:00 2001 From: LFC <990479+MichaelScofield@users.noreply.github.com> Date: Mon, 17 Aug 2026 11:24:44 +0000 Subject: [PATCH 01/10] refactor(json2): support JSON2 storage layout settings in DDL (#8895) * refactor(json2): add JSON2 storage layout settings Signed-off-by: luofucong * resolve PR comments Signed-off-by: luofucong --------- Signed-off-by: luofucong --- src/datatypes/src/error.rs | 8 + src/datatypes/src/extension/json.rs | 1 + src/datatypes/src/json.rs | 286 +++++++++++++++--- src/sql/src/error.rs | 11 +- src/sql/src/parsers/create_parser.rs | 4 +- src/sql/src/parsers/create_parser/json.rs | 154 ++++++++-- src/sql/src/statements.rs | 18 +- src/sql/src/statements/create.rs | 144 +++++---- .../json2_storage_layout_settings/case.toml | 8 + .../json2_storage_layout_settings/setup.sql | 11 + .../verify.result | 27 ++ .../json2_storage_layout_settings/verify.sql | 4 + 12 files changed, 532 insertions(+), 144 deletions(-) create mode 100644 tests/compatibility/cases/json2_storage_layout_settings/case.toml create mode 100644 tests/compatibility/cases/json2_storage_layout_settings/setup.sql create mode 100644 tests/compatibility/cases/json2_storage_layout_settings/verify.result create mode 100644 tests/compatibility/cases/json2_storage_layout_settings/verify.sql diff --git a/src/datatypes/src/error.rs b/src/datatypes/src/error.rs index cf76812c215..b34a5ffa9bf 100644 --- a/src/datatypes/src/error.rs +++ b/src/datatypes/src/error.rs @@ -196,6 +196,13 @@ pub enum Error { location: Location, }, + #[snafu(display("Invalid JSON2 layout: {reason}"))] + InvalidJson2Layout { + reason: String, + #[snafu(implicit)] + location: Location, + }, + #[snafu(display("Invalid Vector: {}", msg))] InvalidVector { msg: String, @@ -318,6 +325,7 @@ impl ErrorExt for Error { | InvalidTimestampPrecision { .. } | InvalidPrecisionOrScale { .. } | InvalidJson { .. } + | InvalidJson2Layout { .. } | InvalidJsonb { .. } | InvalidVector { .. } | InvalidFulltextOption { .. } diff --git a/src/datatypes/src/extension/json.rs b/src/datatypes/src/extension/json.rs index 26d470641f5..109aa853b21 100644 --- a/src/datatypes/src/extension/json.rs +++ b/src/datatypes/src/extension/json.rs @@ -22,6 +22,7 @@ use arrow_schema::{ArrowError, DataType, Field}; use serde::{Deserialize, Serialize}; use snafu::ResultExt; +pub use crate::json::JSON2_REMAINDER_FIELD_NAME; use crate::json::JsonSettings; const LEGACY_JSON_STRUCTURE_SETTINGS_KEY: &str = "json_structure_settings"; diff --git a/src/datatypes/src/json.rs b/src/datatypes/src/json.rs index 48a9174020e..ec9ca4ac789 100644 --- a/src/datatypes/src/json.rs +++ b/src/datatypes/src/json.rs @@ -29,21 +29,47 @@ use serde_json::{Map, Value as Json}; use snafu::ResultExt; use crate::data_type::ConcreteDataType; -use crate::error::{self, Result, UnsupportedJsonTypeSnafu}; +use crate::error::{self, InvalidJson2LayoutSnafu, Result, UnsupportedJsonTypeSnafu}; use crate::json::value::{JsonValue, JsonVariant, encode_serde_json_as_jsonb}; use crate::schema::ColumnDefaultConstraint; -use crate::types::json_type::JsonNativeType; +use crate::types::json_type::{JsonNativeType, JsonObjectType}; use crate::value::{ListValue, StructValue, Value}; /// Maximum number of JSON container levels represented as nested Arrow types. pub const JSON2_MAX_STRUCTURED_DEPTH: usize = 50; +/// Reserved physical field containing unexpanded JSON2 paths. +pub const JSON2_REMAINDER_FIELD_NAME: &str = "!__remainder__!"; /// JSON2 settings stored in column schema metadata and represented through /// Arrow extension metadata. -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize)] pub struct JsonSettings { #[serde(default)] - pub type_hints: Vec, + type_hints: Vec, + /// Maximum number of unhinted JSON paths expanded into Arrow fields. + /// + /// `None` preserves the legacy unlimited layout. + #[serde(default, skip_serializing_if = "Option::is_none")] + max_auto_expanded_paths: Option, +} + +#[derive(Deserialize)] +struct JsonSettingsSerde { + #[serde(default)] + type_hints: Vec, + #[serde(default)] + max_auto_expanded_paths: Option, +} + +impl<'de> Deserialize<'de> for JsonSettings { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::Deserializer<'de>, + { + let settings = JsonSettingsSerde::deserialize(deserializer)?; + Self::try_new(settings.type_hints, settings.max_auto_expanded_paths) + .map_err(serde::de::Error::custom) + } } /// Declares selected JSON2 subpaths as typed fields. @@ -79,8 +105,28 @@ pub struct JsonContext<'a> { } impl JsonSettings { - pub fn new(type_hints: Vec) -> Self { - Self { type_hints } + /// Creates and validates JSON2 settings. + pub fn try_new( + type_hints: Vec, + max_auto_expanded_paths: Option, + ) -> Result { + validate_type_hints(&type_hints)?; + Ok(Self { + type_hints, + max_auto_expanded_paths, + }) + } + + pub fn type_hints(&self) -> &[JsonTypeHint] { + &self.type_hints + } + + pub fn max_auto_expanded_paths(&self) -> Option { + self.max_auto_expanded_paths + } + + pub fn into_parts(self) -> (Vec, Option) { + (self.type_hints, self.max_auto_expanded_paths) } /// Decode an encoded StructValue back into a serde_json::Value. @@ -102,6 +148,87 @@ impl JsonSettings { } } +fn validate_type_hints(type_hints: &[JsonTypeHint]) -> Result<()> { + let mut object = JsonObjectType::new(); + for hint in type_hints { + if hint.path.len() > JSON2_MAX_STRUCTURED_DEPTH { + return InvalidJson2LayoutSnafu { + reason: format!( + "JSON2 type hint path cannot exceed {JSON2_MAX_STRUCTURED_DEPTH} segments" + ), + } + .fail(); + } + if hint + .path + .first() + .is_some_and(|x| x == JSON2_REMAINDER_FIELD_NAME) + { + return InvalidJson2LayoutSnafu { + reason: format!( + "JSON2 type hint path cannot be rooted at reserved field '{JSON2_REMAINDER_FIELD_NAME}'" + ), + } + .fail(); + } + let data_type = match &hint.data_type { + ConcreteDataType::Boolean(_) + | ConcreteDataType::UInt8(_) + | ConcreteDataType::UInt16(_) + | ConcreteDataType::UInt32(_) + | ConcreteDataType::UInt64(_) + | ConcreteDataType::Int8(_) + | ConcreteDataType::Int16(_) + | ConcreteDataType::Int32(_) + | ConcreteDataType::Int64(_) + | ConcreteDataType::Float32(_) + | ConcreteDataType::Float64(_) + | ConcreteDataType::String(_) => (&hint.data_type).into(), + data_type => { + return InvalidJson2LayoutSnafu { + reason: format!("unsupported JSON2 type hint data type: {data_type}"), + } + .fail(); + } + }; + validate_type_hint(&mut object, &hint.path, data_type)?; + } + Ok(()) +} + +fn validate_type_hint( + object: &mut JsonObjectType, + path: &[String], + data_type: JsonNativeType, +) -> Result<()> { + let Some((name, path)) = path.split_first() else { + return InvalidJson2LayoutSnafu { + reason: "JSON2 type hint path must not be empty".to_string(), + } + .fail(); + }; + if path.is_empty() { + if object.insert(name.clone(), data_type).is_some() { + return InvalidJson2LayoutSnafu { + reason: format!("duplicate JSON2 type hint path '{name}'"), + } + .fail(); + } + return Ok(()); + } + + let child = object + .entry(name.clone()) + .or_insert_with(|| JsonNativeType::Object(JsonObjectType::new())); + let JsonNativeType::Object(child) = child else { + return InvalidJson2LayoutSnafu { + reason: format!("conflicting JSON2 type hint path at '{name}'"), + } + .fail(); + }; + validate_type_hint(child, path, data_type) +} + impl<'a> JsonContext<'a> { fn type_hint(&self) -> Option<&'a JsonTypeHint> { self.settings @@ -483,7 +610,8 @@ mod tests { } #[test] - fn test_json_settings_forward_compatibility() { + fn test_json_settings_forward_compatibility() + -> std::result::Result<(), Box> { let json_str = r#"{ "type_hints": [ { @@ -512,11 +640,10 @@ mod tests { ] }"#; - let deserialized = serde_json::from_str::(json_str).unwrap(); + let deserialized = serde_json::from_str::(json_str)?; - assert_eq!( - deserialized, - JsonSettings::new(vec![ + let expected = JsonSettings::try_new( + vec![ JsonTypeHint { path: vec!["user".to_string(), "age".to_string()], data_type: ConcreteDataType::int64_datatype(), @@ -531,13 +658,16 @@ mod tests { default_constraint: None, inverted_index: false, }, - ]) - ); + ], + None, + )?; + assert_eq!(deserialized, expected); + Ok(()) } #[test] - fn test_json_settings_ser_de() { - let settings = JsonSettings::new(vec![ + fn test_json_settings_ser_de() -> std::result::Result<(), Box> { + let type_hints = vec![ JsonTypeHint { path: vec!["user".to_string(), "age".to_string()], data_type: ConcreteDataType::int64_datatype(), @@ -552,12 +682,76 @@ mod tests { default_constraint: None, inverted_index: false, }, - ]); + ]; - let serialized = serde_json::to_string(&settings).unwrap(); - let deserialized = serde_json::from_str::(&serialized).unwrap(); + for settings in [ + JsonSettings { + type_hints: type_hints.clone(), + max_auto_expanded_paths: None, + }, + JsonSettings { + type_hints, + max_auto_expanded_paths: Some(1), + }, + ] { + let serialized = serde_json::to_string(&settings)?; + let deserialized = serde_json::from_str::(&serialized)?; + assert_eq!(settings, deserialized); + } + Ok(()) + } - assert_eq!(settings, deserialized); + #[test] + fn test_json_settings_reject_invalid_type_hint_layout() { + for type_hints in [ + json!([{"path": [], "type": {"Int64": {}}, "nullable": true, "inverted_index": false}]), + json!([ + {"path": ["a"], "type": {"Int64": {}}, "nullable": true, "inverted_index": false}, + {"path": ["a", "b"], "type": {"Int64": {}}, "nullable": true, "inverted_index": false} + ]), + json!([{"path": [JSON2_REMAINDER_FIELD_NAME], "type": {"Int64": {}}, "nullable": true, "inverted_index": false}]), + ] { + let settings = json!({"type_hints": type_hints}); + assert!(serde_json::from_value::(settings).is_err()); + } + + let hint = |path, data_type| JsonTypeHint { + path, + data_type, + nullable: true, + default_constraint: None, + inverted_index: false, + }; + assert!( + JsonSettings::try_new( + vec![hint( + vec!["nested".to_string(), JSON2_REMAINDER_FIELD_NAME.to_string()], + ConcreteDataType::string_datatype(), + )], + None, + ) + .is_ok() + ); + assert!( + JsonSettings::try_new( + vec![hint( + vec!["nested".to_string(); JSON2_MAX_STRUCTURED_DEPTH + 1], + ConcreteDataType::string_datatype(), + )], + None, + ) + .is_err() + ); + assert!( + JsonSettings::try_new( + vec![hint( + vec!["date".to_string()], + ConcreteDataType::date_datatype(), + )], + None, + ) + .is_err() + ); } #[test] @@ -702,14 +896,15 @@ mod tests { } #[test] - fn test_encode_json_respects_type_hint() { - let settings = JsonSettings::new(vec![JsonTypeHint { + fn test_encode_json_respects_type_hint() -> Result<()> { + let type_hints = vec![JsonTypeHint { path: vec!["age".to_string()], data_type: ConcreteDataType::int64_datatype(), nullable: false, default_constraint: None, inverted_index: false, - }]); + }]; + let settings = JsonSettings::try_new(type_hints, None)?; let result = settings .encode(json!({ @@ -731,17 +926,19 @@ mod tests { })) .unwrap_err(); assert!(err.to_string().contains("does not match JSON2 type hint")); + Ok(()) } #[test] - fn test_encode_json_respects_unsigned_type_hint() { - let settings = JsonSettings::new(vec![JsonTypeHint { + fn test_encode_json_respects_unsigned_type_hint() -> Result<()> { + let type_hints = vec![JsonTypeHint { path: vec!["count".to_string()], data_type: ConcreteDataType::uint64_datatype(), nullable: false, default_constraint: None, inverted_index: false, - }]); + }]; + let settings = JsonSettings::try_new(type_hints, None)?; let result = settings .encode(json!({ @@ -765,17 +962,19 @@ mod tests { })) .unwrap_err(); assert!(err.to_string().contains("does not match JSON2 type hint")); + Ok(()) } #[test] - fn test_encode_json_fills_missing_type_hint_with_default() { - let settings = JsonSettings::new(vec![JsonTypeHint { + fn test_encode_json_fills_missing_type_hint_with_default() -> Result<()> { + let type_hints = vec![JsonTypeHint { path: vec!["user".to_string(), "age".to_string()], data_type: ConcreteDataType::int64_datatype(), nullable: false, default_constraint: Some(ColumnDefaultConstraint::Value(Value::Int64(7))), inverted_index: false, - }]); + }]; + let settings = JsonSettings::try_new(type_hints, None)?; let result = settings .encode(json!({})) @@ -790,17 +989,19 @@ mod tests { panic!("Expected user Struct value"); }; assert_eq!(struct_field_value(user, "age"), &Value::Int64(7)); + Ok(()) } #[test] - fn test_encode_json_fills_missing_nullable_type_hint_with_null() { - let settings = JsonSettings::new(vec![JsonTypeHint { + fn test_encode_json_fills_missing_nullable_type_hint_with_null() -> Result<()> { + let type_hints = vec![JsonTypeHint { path: vec!["user".to_string(), "name".to_string()], data_type: ConcreteDataType::string_datatype(), nullable: true, default_constraint: None, inverted_index: false, - }]); + }]; + let settings = JsonSettings::try_new(type_hints, None)?; let result = settings .encode(json!({ "user": {} })) @@ -815,28 +1016,31 @@ mod tests { panic!("Expected user Struct value"); }; assert_eq!(struct_field_value(user, "name"), &Value::Null); + Ok(()) } #[test] - fn test_encode_json_rejects_missing_non_null_type_hint() { - let settings = JsonSettings::new(vec![JsonTypeHint { + fn test_encode_json_rejects_missing_non_null_type_hint() -> Result<()> { + let type_hints = vec![JsonTypeHint { path: vec!["user".to_string(), "age".to_string()], data_type: ConcreteDataType::int64_datatype(), nullable: false, default_constraint: None, inverted_index: false, - }]); + }]; + let settings = JsonSettings::try_new(type_hints, None)?; let err = settings.encode(json!({})).unwrap_err(); assert!( err.to_string() .contains("missing non-null JSON2 type hint path user.age") ); + Ok(()) } #[test] - fn test_encode_json_merges_missing_type_hint_prefix() { - let settings = JsonSettings::new(vec![ + fn test_encode_json_merges_missing_type_hint_prefix() -> Result<()> { + let type_hints = vec![ JsonTypeHint { path: vec!["user".to_string(), "age".to_string()], data_type: ConcreteDataType::int64_datatype(), @@ -853,13 +1057,10 @@ mod tests { ))), inverted_index: false, }, - ]); + ]; + let settings = JsonSettings::try_new(type_hints, None)?; - let result = settings - .encode(json!({})) - .unwrap() - .into_json_inner() - .unwrap(); + let result = settings.encode(json!({}))?.into_json_inner().unwrap(); let Value::Struct(root) = result else { panic!("Expected Struct value"); @@ -872,6 +1073,7 @@ mod tests { struct_field_value(user, "name"), &Value::String("unknown".into()) ); + Ok(()) } #[test] diff --git a/src/sql/src/error.rs b/src/sql/src/error.rs index cc0b9f19506..dc819bda017 100644 --- a/src/sql/src/error.rs +++ b/src/sql/src/error.rs @@ -339,6 +339,13 @@ pub enum Error { #[snafu(implicit)] location: Location, }, + + #[snafu(transparent)] + Datatypes { + #[snafu(implicit)] + location: Location, + source: datatypes::error::Error, + }, } impl ErrorExt for Error { @@ -384,7 +391,9 @@ impl ErrorExt for Error { #[cfg(feature = "enterprise")] InvalidTriggerWebhookOption { .. } => StatusCode::InvalidArguments, - SerializeColumnDefaultConstraint { source, .. } => source.status_code(), + SerializeColumnDefaultConstraint { source, .. } | Datatypes { source, .. } => { + source.status_code() + } ConvertToGrpcDataType { source, .. } => source.status_code(), SqlCommon { source, .. } => source.status_code(), diff --git a/src/sql/src/parsers/create_parser.rs b/src/sql/src/parsers/create_parser.rs index a257f029e35..be7ed0670dc 100644 --- a/src/sql/src/parsers/create_parser.rs +++ b/src/sql/src/parsers/create_parser.rs @@ -711,8 +711,8 @@ impl<'a> ParserContext<'a> { let mut extensions = ColumnExtensions::default(); let data_type = - if let Some((data_type, type_hints)) = json::parse_json2_type_and_hints(parser)? { - extensions.json_type_hints = type_hints; + if let Some((data_type, options)) = json::parse_json2_type_and_options(parser)? { + extensions.json2_options = options; data_type } else { parser.parse_data_type().context(SyntaxSnafu)? diff --git a/src/sql/src/parsers/create_parser/json.rs b/src/sql/src/parsers/create_parser/json.rs index e2f493719c1..63399f81a17 100644 --- a/src/sql/src/parsers/create_parser/json.rs +++ b/src/sql/src/parsers/create_parser/json.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use datatypes::extension::json::JSON2_REMAINDER_FIELD_NAME; use datatypes::json::JSON2_MAX_STRUCTURED_DEPTH; use snafu::{ResultExt, ensure}; use sqlparser::ast::{DataType, ExactNumberInfo, Expr, ObjectName, UnaryOperator}; @@ -22,14 +23,15 @@ use sqlparser::tokenizer::Token; use crate::ast::Ident; use crate::error::{InvalidSqlSnafu, Result, SyntaxSnafu}; use crate::parsers::create_parser::{INVERTED, SKIPPING}; -use crate::statements::create::JsonTypeHint; +use crate::statements::create::{Json2Options, JsonTypeHint}; use crate::statements::transform::type_alias::get_type_by_alias; const JSON2_TYPE_NAME: &str = "JSON2"; +const MAX_AUTO_EXPANDED_PATHS: &str = "max_auto_expanded_paths"; -pub(super) fn parse_json2_type_and_hints( +pub(super) fn parse_json2_type_and_options( parser: &mut Parser<'_>, -) -> Result)>> { +) -> Result)>> { let token = parser.peek_token(); let Token::Word(word) = &token.token else { return Ok(None); @@ -41,26 +43,62 @@ pub(super) fn parse_json2_type_and_hints( parser.next_token(); let data_type = DataType::Custom(ObjectName::from(vec![Ident::new(JSON2_TYPE_NAME)]), vec![]); - let type_hints = if parser.consume_token(&Token::LParen) { - parse_json2_type_hints(parser)? + let options = if parser.consume_token(&Token::LParen) { + parse_json2_options(parser)? } else { - vec![] + None }; - Ok(Some((data_type, type_hints))) + Ok(Some((data_type, options))) } -fn parse_json2_type_hints(parser: &mut Parser<'_>) -> Result> { - let mut hints = Vec::new(); - +fn parse_json2_options(parser: &mut Parser<'_>) -> Result> { if parser.consume_token(&Token::RParen) { - return Ok(hints); + return Ok(None); } + let mut max_auto_expanded_paths = None; + let mut type_hints = Vec::new(); loop { - let hint = parse_json2_type_hint(parser)?; - ensure_no_path_conflict(&hints, &hint.path)?; - hints.push(hint); + let token = parser.peek_token(); + let is_max_auto_expanded_paths = matches!( + &token.token, + Token::Word(word) + if word.quote_style.is_none() + && word.value.eq_ignore_ascii_case(MAX_AUTO_EXPANDED_PATHS) + ); + if is_max_auto_expanded_paths { + parser.next_token(); + ensure!( + max_auto_expanded_paths.is_none(), + InvalidSqlSnafu { + msg: format!("duplicated JSON2 option '{MAX_AUTO_EXPANDED_PATHS}'") + } + ); + parser.expect_token(&Token::Eq).context(SyntaxSnafu)?; + + let token = parser.next_token(); + let Token::Number(value, _) = token.token else { + return InvalidSqlSnafu { + msg: format!( + "JSON2 option '{MAX_AUTO_EXPANDED_PATHS}' expects a non-negative integer" + ), + } + .fail(); + }; + max_auto_expanded_paths = Some(value.parse::().map_err(|_| { + InvalidSqlSnafu { + msg: format!( + "JSON2 option '{MAX_AUTO_EXPANDED_PATHS}' expects a non-negative integer" + ), + } + .build() + })?); + } else { + let hint = parse_json2_type_hint(parser)?; + ensure_no_path_conflict(&type_hints, &hint.path)?; + type_hints.push(hint); + } if parser.consume_token(&Token::Comma) { if parser.consume_token(&Token::RParen) { @@ -72,11 +110,22 @@ fn parse_json2_type_hints(parser: &mut Parser<'_>) -> Result> } } - Ok(hints) + Ok(Some(Json2Options { + max_auto_expanded_paths, + type_hints, + })) } fn parse_json2_type_hint(parser: &mut Parser<'_>) -> Result { let path = parse_json2_path(parser)?; + ensure!( + path.first().is_none_or(|x| x != JSON2_REMAINDER_FIELD_NAME), + InvalidSqlSnafu { + msg: format!( + "JSON2 type hint path cannot be rooted at reserved field '{JSON2_REMAINDER_FIELD_NAME}'" + ) + } + ); ensure!( path.len() <= JSON2_MAX_STRUCTURED_DEPTH, InvalidSqlSnafu { @@ -309,7 +358,7 @@ CREATE TABLE traces ( column.column_def.data_type, DataType::Custom(_, _) )); - let hints = column.extensions.json_type_hints; + let hints = column.extensions.json2_options.unwrap().type_hints; assert_eq!(hints.len(), 4); assert_eq!(hints[0].path, vec!["service.name"]); @@ -339,6 +388,71 @@ CREATE TABLE traces ( assert!(hints[3].nullable); } + #[test] + fn test_parse_json2_max_auto_expanded_paths() { + let column = parse_json2_column( + r#" +CREATE TABLE traces ( + log_json_data JSON2 ( + http.method STRING, + max_auto_expanded_paths = 0 + ), + ts TIMESTAMP TIME INDEX, +)"#, + ); + + let options = column.extensions.json2_options.unwrap(); + assert_eq!(options.max_auto_expanded_paths, Some(0)); + assert_eq!(options.type_hints.len(), 1); + + let empty = parse_json2_column( + r#" +CREATE TABLE traces ( + log_json_data JSON2 (), + ts TIMESTAMP TIME INDEX, +)"#, + ); + assert!(empty.extensions.json2_options.is_none()); + + let quoted = parse_json2_column( + r#" +CREATE TABLE traces ( + log_json_data JSON2 ( + "max_auto_expanded_paths" STRING, + nested."!__remainder__!" STRING + ), + ts TIMESTAMP TIME INDEX, +)"#, + ); + let options = quoted.extensions.json2_options.unwrap(); + assert_eq!(options.max_auto_expanded_paths, None); + assert_eq!(options.type_hints.len(), 2); + } + + #[test] + fn test_parse_json2_max_auto_expanded_paths_rejects_invalid_options() { + for options in [ + "max_auto_expanded_paths = 0, max_auto_expanded_paths = 1", + "max_auto_expanded_paths = -1", + "max_auto_expanded_paths = 1.5", + "max_auto_expanded_paths = 4294967296", + r#""!__remainder__!".value STRING"#, + ] { + let sql = format!( + "CREATE TABLE traces (log_json_data JSON2 ({options}), ts TIMESTAMP TIME INDEX)" + ); + assert!( + ParserContext::create_with_dialect( + &sql, + &GreptimeDbDialect {}, + ParseOptions::default() + ) + .is_err(), + "{options}" + ); + } + } + #[test] fn test_parse_json2_type_hint_default_nullable() { let column = parse_json2_column( @@ -349,7 +463,7 @@ CREATE TABLE traces ( )"#, ); - let hints = column.extensions.json_type_hints; + let hints = column.extensions.json2_options.unwrap().type_hints; assert_eq!(hints.len(), 1); assert!(hints[0].nullable); } @@ -369,7 +483,7 @@ CREATE TABLE traces ( )"#, ); - let hints = column.extensions.json_type_hints; + let hints = column.extensions.json2_options.unwrap().type_hints; assert_eq!(hints.len(), 4); assert_eq!(hints[0].path, vec!["a", "b"]); assert_eq!(hints[1].path, vec!["x", "y"]); @@ -402,7 +516,7 @@ CREATE TABLE traces ( )"#, ); - let hints = column.extensions.json_type_hints; + let hints = column.extensions.json2_options.unwrap().type_hints; assert_eq!(hints.len(), 14); for hint in hints.iter().take(6) { assert_eq!(hint.data_type, DataType::BigInt(None)); @@ -428,7 +542,7 @@ CREATE TABLE traces ( )"#, ); - let hints = column.extensions.json_type_hints; + let hints = column.extensions.json2_options.unwrap().type_hints; assert_eq!(hints.len(), 2); assert_eq!( hints[0] diff --git a/src/sql/src/statements.rs b/src/sql/src/statements.rs index aafe62d2e6e..4f96b66f87f 100644 --- a/src/sql/src/statements.rs +++ b/src/sql/src/statements.rs @@ -718,11 +718,7 @@ mod tests { "true".to_string(), ), ])), - vector_options: None, - skipping_index_options: None, - inverted_index_options: None, - json_type_hints: vec![], - vector_index_options: None, + ..Default::default() }, }; @@ -749,17 +745,13 @@ mod tests { options: vec![], }, extensions: ColumnExtensions { - fulltext_index_options: None, - vector_options: None, - skipping_index_options: None, - inverted_index_options: None, - json_type_hints: vec![], vector_index_options: Some(OptionMap::from([ ("metric".to_string(), "cosine".to_string()), ("connectivity".to_string(), "32".to_string()), ("expansion_add".to_string(), "200".to_string()), ("expansion_search".to_string(), "100".to_string()), ])), + ..Default::default() }, }; @@ -790,12 +782,8 @@ mod tests { options: vec![], }, extensions: ColumnExtensions { - fulltext_index_options: None, - vector_options: None, - skipping_index_options: None, - inverted_index_options: None, - json_type_hints: vec![], vector_index_options: Some(OptionMap::default()), + ..Default::default() }, }; diff --git a/src/sql/src/statements/create.rs b/src/sql/src/statements/create.rs index dc94c7f8889..f3fbc908362 100644 --- a/src/sql/src/statements/create.rs +++ b/src/sql/src/statements/create.rs @@ -136,7 +136,28 @@ pub struct ColumnExtensions { pub inverted_index_options: Option, /// Vector index options for HNSW-based vector similarity search. pub vector_index_options: Option, - pub json_type_hints: Vec, + /// JSON2-specific column options. + pub json2_options: Option, +} + +/// JSON2-specific options represented in the SQL AST. +#[derive(Debug, PartialEq, Eq, Clone, Visit, VisitMut, Serialize)] +pub struct Json2Options { + /// Maximum number of unhinted JSON2 paths expanded into Arrow fields. + pub(crate) max_auto_expanded_paths: Option, + /// Paths stored as explicitly typed JSON2 fields. + pub(crate) type_hints: Vec, +} + +impl Display for Json2Options { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let mut options = Vec::with_capacity(self.type_hints.len() + 1); + if let Some(max) = self.max_auto_expanded_paths { + options.push(format!("max_auto_expanded_paths = {max}")); + } + options.extend(self.type_hints.iter().map(format_json_type_hint)); + write!(f, "(\n {}\n )", options.iter().join(",\n ")) + } } #[derive(Debug, PartialEq, Eq, Clone, Visit, VisitMut, Serialize)] @@ -180,12 +201,8 @@ impl Display for Column { } write!(f, "{} {}", self.column_def.name, self.column_def.data_type)?; - if !self.extensions.json_type_hints.is_empty() { - write!( - f, - "{}", - format_json_type_hints(&self.extensions.json_type_hints) - )?; + if let Some(options) = &self.extensions.json2_options { + write!(f, "{options}")?; } for option in &self.column_def.options { write!(f, " {option}")?; @@ -335,29 +352,30 @@ impl ColumnExtensions { } pub fn build_json_settings(&self) -> Result> { - if self.json_type_hints.is_empty() { + let Some(options) = &self.json2_options else { return Ok(None); - } + }; - Ok(Some(JsonSettings::new( - self.json_type_hints - .iter() - .map(|hint| { - Ok(datatypes::json::JsonTypeHint { - path: hint.path.clone(), - data_type: json_type_hint_concrete_data_type(&hint.data_type)?, - nullable: hint.nullable, - default_constraint: build_json_type_hint_default_constraint(hint)?, - inverted_index: hint.inverted_index, - }) + let type_hints = options + .type_hints + .iter() + .map(|hint| { + Ok(datatypes::json::JsonTypeHint { + path: hint.path.clone(), + data_type: json_type_hint_concrete_data_type(&hint.data_type)?, + nullable: hint.nullable, + default_constraint: build_json_type_hint_default_constraint(hint)?, + inverted_index: hint.inverted_index, }) - .collect::>>()?, - ))) + }) + .collect::>>()?; + let settings = JsonSettings::try_new(type_hints, options.max_auto_expanded_paths)?; + Ok(Some(settings)) } pub fn set_json_settings(&mut self, settings: JsonSettings) -> Result<()> { - self.json_type_hints = settings - .type_hints + let (type_hints, max_auto_expanded_paths) = settings.into_parts(); + let type_hints = type_hints .into_iter() .map(|hint| { let data_type = json_type_hint_sql_data_type(&hint.data_type)?; @@ -374,6 +392,11 @@ impl ColumnExtensions { }) }) .collect::>>()?; + self.json2_options = (max_auto_expanded_paths.is_some() || !type_hints.is_empty()) + .then_some(Json2Options { + max_auto_expanded_paths, + type_hints, + }); Ok(()) } } @@ -487,13 +510,6 @@ fn format_json_type_hint(hint: &JsonTypeHint) -> String { ) } -fn format_json_type_hints(hints: &[JsonTypeHint]) -> String { - format!( - "(\n {}\n )", - hints.iter().map(format_json_type_hint).join(",\n ") - ) -} - fn format_json_path_segment(segment: &str) -> String { format!("\"{}\"", segment.replace('"', "\"\"")) } @@ -786,6 +802,7 @@ mod tests { use datatypes::schema::ColumnDefaultConstraint; use datatypes::value::Value; + use super::*; use crate::dialect::GreptimeDbDialect; use crate::error::Error; use crate::parser::{ParseOptions, ParserContext}; @@ -1066,7 +1083,7 @@ ENGINE=mito .build_json_settings() .unwrap() .unwrap(); - let hints = settings.type_hints; + let hints = settings.type_hints(); assert_eq!(hints[0].data_type, ConcreteDataType::int64_datatype()); assert_eq!( @@ -1118,10 +1135,10 @@ ENGINE=mito } #[test] - fn test_set_json_settings_normalizes_type_hint_sql_types() { + fn test_set_json_settings_normalizes_type_hint_sql_types() -> Result<()> { let mut extensions = super::ColumnExtensions::default(); - extensions - .set_json_settings(JsonSettings::new(vec![ + let settings = JsonSettings::try_new( + vec![ DatatypeJsonTypeHint { path: vec!["i".to_string()], data_type: ConcreteDataType::int32_datatype(), @@ -1157,36 +1174,51 @@ ENGINE=mito default_constraint: None, inverted_index: false, }, - ])) - .unwrap(); + ], + None, + )?; + extensions.set_json_settings(settings)?; assert_eq!( extensions - .json_type_hints + .json2_options + .unwrap() + .type_hints .iter() .map(|hint| hint.data_type.to_string()) .collect::>(), vec!["BIGINT", "DOUBLE", "BIGINT UNSIGNED", "STRING", "BOOLEAN"] ); + Ok(()) } #[test] - fn test_set_json_settings_rejects_unsupported_type_hint_type() { - let mut extensions = super::ColumnExtensions::default(); - let err = extensions - .set_json_settings(JsonSettings::new(vec![DatatypeJsonTypeHint { + fn test_set_json_settings_rejects_unsupported_type_hint_type() -> Result<()> { + let err = JsonSettings::try_new( + vec![DatatypeJsonTypeHint { path: vec!["u".to_string()], data_type: ConcreteDataType::date_datatype(), nullable: true, default_constraint: None, inverted_index: false, - }])) - .unwrap_err(); + }], + None, + ) + .unwrap_err(); assert!( err.to_string() .contains("unsupported JSON2 type hint data type") ); + Ok(()) + } + + #[test] + fn test_set_empty_json_settings_omits_json2_options() -> Result<()> { + let mut extensions = ColumnExtensions::default(); + extensions.set_json_settings(JsonSettings::default())?; + assert!(extensions.json2_options.is_none()); + Ok(()) } #[test] @@ -1364,15 +1396,11 @@ AS SELECT number FROM numbers_input where number > 10"#, // Test zero connectivity should fail let extensions = ColumnExtensions { - fulltext_index_options: None, - vector_options: None, - skipping_index_options: None, - inverted_index_options: None, - json_type_hints: vec![], vector_index_options: Some(OptionMap::from([( "connectivity".to_string(), "0".to_string(), )])), + ..Default::default() }; let result = extensions.build_vector_index_options(); assert!(result.is_err()); @@ -1385,15 +1413,11 @@ AS SELECT number FROM numbers_input where number > 10"#, // Test zero expansion_add should fail let extensions = ColumnExtensions { - fulltext_index_options: None, - vector_options: None, - skipping_index_options: None, - inverted_index_options: None, - json_type_hints: vec![], vector_index_options: Some(OptionMap::from([( "expansion_add".to_string(), "0".to_string(), )])), + ..Default::default() }; let result = extensions.build_vector_index_options(); assert!(result.is_err()); @@ -1406,15 +1430,11 @@ AS SELECT number FROM numbers_input where number > 10"#, // Test zero expansion_search should fail let extensions = ColumnExtensions { - fulltext_index_options: None, - vector_options: None, - skipping_index_options: None, - inverted_index_options: None, - json_type_hints: vec![], vector_index_options: Some(OptionMap::from([( "expansion_search".to_string(), "0".to_string(), )])), + ..Default::default() }; let result = extensions.build_vector_index_options(); assert!(result.is_err()); @@ -1427,16 +1447,12 @@ AS SELECT number FROM numbers_input where number > 10"#, // Test valid values should succeed let extensions = ColumnExtensions { - fulltext_index_options: None, - vector_options: None, - skipping_index_options: None, - inverted_index_options: None, - json_type_hints: vec![], vector_index_options: Some(OptionMap::from([ ("connectivity".to_string(), "32".to_string()), ("expansion_add".to_string(), "200".to_string()), ("expansion_search".to_string(), "100".to_string()), ])), + ..Default::default() }; let result = extensions.build_vector_index_options(); assert!(result.is_ok()); diff --git a/tests/compatibility/cases/json2_storage_layout_settings/case.toml b/tests/compatibility/cases/json2_storage_layout_settings/case.toml new file mode 100644 index 00000000000..8f1dd3fe64a --- /dev/null +++ b/tests/compatibility/cases/json2_storage_layout_settings/case.toml @@ -0,0 +1,8 @@ +name = "json2_storage_layout_settings" +reason = "Verify JSON2 storage layout settings in Arrow extension metadata survive restarts and upgrades." +introduced_by = "PR #8895" +topologies = ["distributed", "standalone"] +from_range = [">=v1.3.0"] +to_range = [">=v1.3.0"] +features = ["json2", "table", "metadata", "read"] +owner = "query" diff --git a/tests/compatibility/cases/json2_storage_layout_settings/setup.sql b/tests/compatibility/cases/json2_storage_layout_settings/setup.sql new file mode 100644 index 00000000000..2592185ef23 --- /dev/null +++ b/tests/compatibility/cases/json2_storage_layout_settings/setup.sql @@ -0,0 +1,11 @@ +CREATE TABLE t_json2_storage_layout_settings ( + ts TIMESTAMP TIME INDEX, + j JSON2(max_auto_expanded_paths = 0) +) WITH ( + 'append_mode' = 'true' +); + +INSERT INTO t_json2_storage_layout_settings VALUES + ('2026-08-17 00:00:00+0000', '{"a": 1}'); + +ADMIN FLUSH_TABLE('t_json2_storage_layout_settings'); diff --git a/tests/compatibility/cases/json2_storage_layout_settings/verify.result b/tests/compatibility/cases/json2_storage_layout_settings/verify.result new file mode 100644 index 00000000000..452ed7a5516 --- /dev/null +++ b/tests/compatibility/cases/json2_storage_layout_settings/verify.result @@ -0,0 +1,27 @@ +SELECT j.a AS a +FROM t_json2_storage_layout_settings; + ++---+ +| a | ++---+ +| 1 | ++---+ + +SHOW CREATE TABLE t_json2_storage_layout_settings; + ++---------------------------------+----------------------------------------------------------------+ +| Table | Create Table | ++---------------------------------+----------------------------------------------------------------+ +| t_json2_storage_layout_settings | CREATE TABLE IF NOT EXISTS "t_json2_storage_layout_settings" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "j" JSON2( | +| | max_auto_expanded_paths = 0 | +| | ) NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | WITH( | +| | append_mode = 'true' | +| | ) | ++---------------------------------+----------------------------------------------------------------+ diff --git a/tests/compatibility/cases/json2_storage_layout_settings/verify.sql b/tests/compatibility/cases/json2_storage_layout_settings/verify.sql new file mode 100644 index 00000000000..dae3cd8a235 --- /dev/null +++ b/tests/compatibility/cases/json2_storage_layout_settings/verify.sql @@ -0,0 +1,4 @@ +SELECT j.a AS a +FROM t_json2_storage_layout_settings; + +SHOW CREATE TABLE t_json2_storage_layout_settings; From b882e393dfadcf9406c76d1017fe64e0652f0202 Mon Sep 17 00:00:00 2001 From: sun Date: Tue, 18 Aug 2026 03:47:58 +0000 Subject: [PATCH 02/10] feat: update dashboard to v0.13.13 (#8898) --- src/servers/dashboard/VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/servers/dashboard/VERSION b/src/servers/dashboard/VERSION index 3d431cfd5f0..b68b26e6f78 100644 --- a/src/servers/dashboard/VERSION +++ b/src/servers/dashboard/VERSION @@ -1 +1 @@ -v0.13.12 +v0.13.13 From b96ea86a621e7283e77e4e37dd4e3ec8dfc44f73 Mon Sep 17 00:00:00 2001 From: LFC <990479+MichaelScofield@users.noreply.github.com> Date: Wed, 19 Aug 2026 02:27:06 +0000 Subject: [PATCH 03/10] refactor(json2): add JSON2 v2 physical layout primitives (#8901) * refactor(json2): add JSON2 v2 physical layout primitives Signed-off-by: luofucong * resolve PR comments Signed-off-by: luofucong * fix ci Signed-off-by: luofucong --------- Signed-off-by: luofucong --- Cargo.lock | 50 +++++ Cargo.toml | 3 + src/datatypes/Cargo.toml | 3 + src/datatypes/src/extension/json.rs | 202 +++++++++++++++++- src/datatypes/src/vectors/json.rs | 1 + src/datatypes/src/vectors/json/array.rs | 248 ++++++++++++++++++++-- src/datatypes/src/vectors/json/variant.rs | 239 +++++++++++++++++++++ 7 files changed, 728 insertions(+), 18 deletions(-) create mode 100644 src/datatypes/src/vectors/json/variant.rs diff --git a/Cargo.lock b/Cargo.lock index 0f473d4e030..16b4c6b1045 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4554,6 +4554,9 @@ dependencies = [ "num", "num-traits", "ordered-float 4.6.0", + "parquet-variant", + "parquet-variant-compute", + "parquet-variant-json", "paste", "regex", "serde", @@ -10259,6 +10262,53 @@ dependencies = [ "zstd", ] +[[package]] +name = "parquet-variant" +version = "58.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74c8db065291f088a2aad8ab831853eae1871c0d311c8d0b83bbc3b7e735d0fc" +dependencies = [ + "arrow 58.3.0", + "arrow-schema 58.3.0", + "chrono", + "half", + "indexmap 2.13.0", + "num-traits", + "simdutf8", + "uuid", +] + +[[package]] +name = "parquet-variant-compute" +version = "58.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a530e8d5b5e14efcb39c9a6ec55432ad11f6afb7dc4455a79be0dc615fe3cc31" +dependencies = [ + "arrow 58.3.0", + "arrow-schema 58.3.0", + "chrono", + "half", + "indexmap 2.13.0", + "parquet-variant", + "parquet-variant-json", + "serde_json", + "uuid", +] + +[[package]] +name = "parquet-variant-json" +version = "58.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "00ed89908289f67caa2ca078f9ff9aacd6229a313ec92b12bf4f48f613dc2b97" +dependencies = [ + "arrow-schema 58.3.0", + "base64 0.22.1", + "chrono", + "parquet-variant", + "serde_json", + "uuid", +] + [[package]] name = "parse-size" version = "1.1.0" diff --git a/Cargo.toml b/Cargo.toml index daee1bd4090..777d8b302b4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -195,6 +195,9 @@ otel-arrow-rust = { git = "https://github.com/GreptimeTeam/otel-arrow", rev = "5 ] } parking_lot = "0.12" parquet = { version = "58.3", default-features = false, features = ["arrow", "async", "object_store"] } +parquet-variant = "58.3" +parquet-variant-compute = "58.3" +parquet-variant-json = "58.3" paste = "1.0" pin-project = "1.0" pretty_assertions = "1.4.0" diff --git a/src/datatypes/Cargo.toml b/src/datatypes/Cargo.toml index 3bf2a5d8eec..124b1e0ea64 100644 --- a/src/datatypes/Cargo.toml +++ b/src/datatypes/Cargo.toml @@ -27,6 +27,9 @@ jsonb.workspace = true num = "0.4" num-traits = "0.2" ordered-float.workspace = true +parquet-variant.workspace = true +parquet-variant-compute.workspace = true +parquet-variant-json.workspace = true paste.workspace = true regex.workspace = true serde.workspace = true diff --git a/src/datatypes/src/extension/json.rs b/src/datatypes/src/extension/json.rs index 109aa853b21..ab4a138420e 100644 --- a/src/datatypes/src/extension/json.rs +++ b/src/datatypes/src/extension/json.rs @@ -18,31 +18,148 @@ use std::sync::Arc; use arrow_schema::extension::{ EXTENSION_TYPE_METADATA_KEY, EXTENSION_TYPE_NAME_KEY, ExtensionType, }; -use arrow_schema::{ArrowError, DataType, Field}; +use arrow_schema::{ArrowError, DataType, Field, FieldRef}; +use parquet_variant_compute::VariantType; use serde::{Deserialize, Serialize}; -use snafu::ResultExt; +use snafu::{ResultExt, ensure}; +use crate::error::InvalidJson2LayoutSnafu; pub use crate::json::JSON2_REMAINDER_FIELD_NAME; use crate::json::JsonSettings; const LEGACY_JSON_STRUCTURE_SETTINGS_KEY: &str = "json_structure_settings"; +const JSON2_LAYOUT_V1: u8 = 1; +const JSON2_LAYOUT_V2: u8 = 2; -#[derive(Debug, Clone, Serialize, Deserialize, Default)] +/// Parsed physical layout of a JSON2 Arrow root field. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Json2PhysicalLayout { + version: u8, +} + +impl Json2PhysicalLayout { + /// Parses the JSON2 layout version from a root field. + /// + /// The version is read from the extension metadata; missing metadata + /// defaults to V1. + /// + /// Supported versions: + /// - V1 + /// - V2 + /// + /// Errors when the field is not a JSON2 extension or the version is not + /// one of the supported versions. + pub fn try_from_root(field: &Field) -> crate::error::Result { + ensure!( + is_json2_extension_type(field), + InvalidJson2LayoutSnafu { + reason: format!("field '{}' is not a JSON2 extension", field.name()), + } + ); + + let version = match field.extension_type_name() { + Some(Json2ExtensionType::NAME) => field + .metadata() + .get(EXTENSION_TYPE_METADATA_KEY) + .map(|x| parse_version(x)) + .transpose()? + .flatten() + .unwrap_or(JSON2_LAYOUT_V1), + _ => JSON2_LAYOUT_V1, + }; + ensure!( + matches!(version, JSON2_LAYOUT_V1 | JSON2_LAYOUT_V2), + InvalidJson2LayoutSnafu { + reason: format!("unsupported JSON2 layout version: {version}"), + } + ); + Ok(Self { version }) + } + + /// Returns whether this is the JSON2 physical layout version 2. + pub fn is_version_2(&self) -> bool { + self.version == JSON2_LAYOUT_V2 + } +} + +fn parse_version(metadata: &str) -> crate::error::Result> { + serde_json::from_str::(metadata) + .map(|x| x.layout_version) + .map_err(|e| { + InvalidJson2LayoutSnafu { + reason: format!(r#"invalid extension metadata: "{metadata}", error: {e}"#), + } + .build() + }) +} + +/// Returns the remainder field of a JSON2 v2 root. +pub(crate) fn json2_remainder_field(field: &Field) -> crate::error::Result> { + let layout = Json2PhysicalLayout::try_from_root(field)?; + if !layout.is_version_2() { + return Ok(None); + } + + let remainder = if let DataType::Struct(fields) = field.data_type() { + fields + .iter() + .find(|x| x.name() == JSON2_REMAINDER_FIELD_NAME) + } else { + None + }; + + if let Some(remainder) = remainder { + let _ = remainder.try_extension_type::().map_err(|e| { + InvalidJson2LayoutSnafu { + reason: e.to_string(), + } + .build() + })?; + } + Ok(remainder) +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct JsonMetadata { /// JSON2 settings stored in Arrow extension metadata. json_settings: JsonSettings, + /// Physical JSON2 layout used by this Arrow field. + #[serde(default, skip_serializing_if = "Option::is_none")] + layout_version: Option, } impl JsonMetadata { - /// Creates JSON2 extension metadata. + /// Creates metadata for the legacy JSON2 layout. pub fn new(json_settings: JsonSettings) -> Self { - Self { json_settings } + Self { + json_settings, + layout_version: None, + } + } + + /// Creates metadata for the new JSON2 physical v2 layout. + pub fn new_v2(json_settings: JsonSettings) -> Self { + Self { + json_settings, + layout_version: Some(JSON2_LAYOUT_V2), + } } /// Returns the JSON2 settings. pub fn json_settings(&self) -> &JsonSettings { &self.json_settings } + + /// Returns whether this metadata describes JSON2 layout version 2. + pub fn is_version_2(&self) -> bool { + self.layout_version == Some(JSON2_LAYOUT_V2) + } +} + +impl Default for JsonMetadata { + fn default() -> Self { + Self::new(JsonSettings::default()) + } } /// Arrow extension type for legacy JSONB columns. @@ -227,6 +344,7 @@ mod tests { use arrow_schema::{Field, Fields}; use super::*; + use crate::vectors::json::variant::variant_field; #[test] fn test_json2_extension_type_detection() { @@ -282,4 +400,78 @@ mod tests { assert!(JsonExtensionType::try_new(&DataType::Null, ()).is_ok()); assert!(JsonExtensionType::try_new(&DataType::Struct(Fields::empty()), ()).is_err()); } + + #[test] + fn test_json_metadata_layout_version_compatibility() -> serde_json::Result<()> { + let legacy: JsonMetadata = serde_json::from_str(r#"{"json_settings":{}}"#)?; + assert!(!legacy.is_version_2()); + + let metadata = JsonMetadata::new_v2(JsonSettings::default()); + assert!(metadata.is_version_2()); + let serialized = serde_json::to_string(&metadata)?; + let deserialized: JsonMetadata = serde_json::from_str(&serialized)?; + assert!(deserialized.is_version_2()); + assert_eq!(deserialized, metadata); + Ok(()) + } + + #[test] + fn test_parse_json2_physical_layout() -> crate::error::Result<()> { + let legacy = Field::new("data", DataType::Struct(Fields::empty()), true) + .with_extension_type(Json2ExtensionType::default()); + assert!(!Json2PhysicalLayout::try_from_root(&legacy)?.is_version_2()); + + let v2 = Field::new( + "data", + DataType::Struct( + vec![ + Arc::new(variant_field(JSON2_REMAINDER_FIELD_NAME, true)), + Arc::new(Field::new("count", DataType::Int64, true)), + ] + .into(), + ), + true, + ) + .with_extension_type(Json2ExtensionType::new(Arc::new(JsonMetadata::new_v2( + JsonSettings::default(), + )))); + assert!(Json2PhysicalLayout::try_from_root(&v2)?.is_version_2()); + let remainder = json2_remainder_field(&v2)?.unwrap(); + assert_eq!(JSON2_REMAINDER_FIELD_NAME, remainder.name()); + Ok(()) + } + + #[test] + fn test_reject_invalid_json2_physical_layout() { + let field = Field::new("data", DataType::Struct(Fields::empty()), true); + assert!(Json2PhysicalLayout::try_from_root(&field).is_err()); + + let metadata = + Json2ExtensionType::new(Arc::new(JsonMetadata::new_v2(JsonSettings::default()))); + let missing = Field::new("data", DataType::Struct(Fields::empty()), true) + .with_extension_type(metadata.clone()); + assert!(json2_remainder_field(&missing).is_ok_and(|x| x.is_none())); + + let invalid = Field::new( + "data", + DataType::Struct( + vec![Arc::new(Field::new( + JSON2_REMAINDER_FIELD_NAME, + DataType::Binary, + true, + ))] + .into(), + ), + true, + ) + .with_extension_type(metadata); + assert!(json2_remainder_field(&invalid).is_err()); + + let future = Field::new("data", DataType::Struct(Fields::empty()), true) + .with_extension_type(Json2ExtensionType::new(Arc::new(JsonMetadata { + json_settings: JsonSettings::default(), + layout_version: Some(JSON2_LAYOUT_V2 + 1), + }))); + assert!(Json2PhysicalLayout::try_from_root(&future).is_err()); + } } diff --git a/src/datatypes/src/vectors/json.rs b/src/datatypes/src/vectors/json.rs index 3dcb8d7bc07..c68e6cf3d92 100644 --- a/src/datatypes/src/vectors/json.rs +++ b/src/datatypes/src/vectors/json.rs @@ -14,3 +14,4 @@ pub mod array; pub(crate) mod builder; +pub mod variant; diff --git a/src/datatypes/src/vectors/json/array.rs b/src/datatypes/src/vectors/json/array.rs index afe9f553056..0426334c3dd 100644 --- a/src/datatypes/src/vectors/json/array.rs +++ b/src/datatypes/src/vectors/json/array.rs @@ -17,9 +17,12 @@ use std::sync::Arc; use arrow::compute::{can_cast_types, cast}; use arrow_array::cast::AsArray; -use arrow_array::types::{Float64Type, Int64Type, UInt64Type}; +use arrow_array::types::{ + Float32Type, Float64Type, Int8Type, Int16Type, Int32Type, Int64Type, UInt8Type, UInt16Type, + UInt32Type, UInt64Type, +}; use arrow_array::{Array, ArrayRef, GenericListArray, ListArray, StructArray, new_null_array}; -use arrow_schema::{DataType, FieldRef}; +use arrow_schema::{DataType, Field, FieldRef}; use common_telemetry::trace; use serde_json::Value; use snafu::{OptionExt, ResultExt}; @@ -29,9 +32,11 @@ use crate::data_type::ConcreteDataType; use crate::error::{ AlignJsonArraySnafu, ArrowComputeSnafu, InvalidJsonSnafu, InvalidJsonbSnafu, Result, }; +use crate::extension::json::{JSON2_REMAINDER_FIELD_NAME, json2_remainder_field}; use crate::json::value::{decode_json_variant, encode_serde_json_as_jsonb}; use crate::prelude::{DataType as _, Value as GreptimeValue}; use crate::value::{ListValue, StructValue}; +use crate::vectors::json::variant::variant_to_json_values; pub struct JsonArray<'a> { inner: &'a ArrayRef, @@ -48,8 +53,15 @@ impl JsonArray<'_> { let value = match array.data_type() { DataType::Null => Value::Null, DataType::Boolean => Value::Bool(array.as_boolean().value(i)), + DataType::Int8 => Value::from(array.as_primitive::().value(i)), + DataType::Int16 => Value::from(array.as_primitive::().value(i)), + DataType::Int32 => Value::from(array.as_primitive::().value(i)), DataType::Int64 => Value::from(array.as_primitive::().value(i)), + DataType::UInt8 => Value::from(array.as_primitive::().value(i)), + DataType::UInt16 => Value::from(array.as_primitive::().value(i)), + DataType::UInt32 => Value::from(array.as_primitive::().value(i)), DataType::UInt64 => Value::from(array.as_primitive::().value(i)), + DataType::Float32 => Value::from(array.as_primitive::().value(i)), DataType::Float64 => Value::from(array.as_primitive::().value(i)), DataType::Utf8 | DataType::LargeUtf8 | DataType::Utf8View => { Value::String(string_array_value(array, i).to_string()) @@ -92,6 +104,60 @@ impl JsonArray<'_> { Ok(value) } + /// Projects a physical JSON2 array to a logical query type. + /// + /// TODO(LFC) Supersede `project_to_v2` to `project_to`. + pub fn project_to_v2(&self, field: &Field, target: &DataType) -> Result { + if json2_remainder_field(field)?.is_some() { + project_json_values(self.json2_values()?, target) + } else { + self.project_to(target) + } + } + + fn json2_values(&self) -> Result> { + let structs = self.inner.as_struct_opt().context(AlignJsonArraySnafu { + reason: "JSON2 layout v2 root array must be a struct", + })?; + let remainder = structs.column_by_name(JSON2_REMAINDER_FIELD_NAME); + let mut remainders = if let Some(remainder) = remainder { + variant_to_json_values(remainder)? + } else { + vec![None; structs.len()] + }; + let mut values = Vec::with_capacity(structs.len()); + let mut path = Vec::new(); + + for (i, remainder) in remainders.iter_mut().enumerate() { + if structs.is_null(i) { + values.push(Value::Null); + continue; + } + + let mut object = match remainder.take() { + None => serde_json::Map::new(), + Some(Value::Object(object)) => object, + Some(value) => { + return InvalidJsonSnafu { + value: format!("JSON2 layout v2 remainder must be an object, got {value}"), + } + .fail(); + } + }; + + for (child, column) in structs.fields().iter().zip(structs.columns()) { + if child.name() == JSON2_REMAINDER_FIELD_NAME { + continue; + } + let value = JsonArray::from(column).try_get_value(i)?; + merge_explicit_value(&mut object, child.name().clone(), value, &mut path)?; + } + values.push(Value::Object(object)); + } + + Ok(values) + } + /// Normalizes a JSON2 array to the wider `expect` data type without losing /// information. /// @@ -326,6 +392,34 @@ impl JsonArray<'_> { } } +fn merge_explicit_value( + remainder: &mut serde_json::Map, + key: String, + explicit: Value, + path: &mut Vec, +) -> Result<()> { + let Some(existing) = remainder.get_mut(&key) else { + remainder.insert(key, explicit); + return Ok(()); + }; + path.push(key); + + let (Value::Object(remainder), Value::Object(explicit)) = (existing, explicit) else { + return InvalidJsonSnafu { + value: format!( + "cannot merge '{}' in explicit fields and remainder: not both objects", + path.join("."), + ), + } + .fail(); + }; + for (key, value) in explicit { + merge_explicit_value(remainder, key, value, path)?; + } + path.pop(); + Ok(()) +} + /// Returns whether Arrow can cast between the types without JSON-aware projection. /// Binary and nested types require JSONB decoding or recursive projection. fn can_fast_cast_types(from_type: &DataType, to_type: &DataType) -> bool { @@ -453,13 +547,16 @@ impl<'a> From<&'a ArrayRef> for JsonArray<'a> { mod test { use arrow_array::types::Int64Type; use arrow_array::{ - BinaryArray, BooleanArray, Float64Array, Int32Array, Int64Array, ListArray, StringArray, - UInt64Array, + BinaryArray, BooleanArray, Float32Array, Float64Array, Int8Array, Int16Array, Int32Array, + Int64Array, ListArray, StringArray, UInt8Array, UInt16Array, UInt32Array, UInt64Array, }; use arrow_schema::{Field, Fields}; use serde_json::json; use super::*; + use crate::extension::json::{Json2ExtensionType, JsonMetadata}; + use crate::json::JsonSettings; + use crate::vectors::json::variant::{json_values_to_variant, variant_field}; #[test] fn test_try_get_value() -> Result<()> { @@ -474,6 +571,20 @@ mod test { assert_eq!(JsonArray::from(&ints).try_get_value(0)?, json!(-7)); assert_eq!(JsonArray::from(&ints).try_get_value(1)?, Value::Null); + macro_rules! assert_number { + ($array:expr, $expected:expr) => {{ + let array: ArrayRef = Arc::new($array); + assert_eq!(JsonArray::from(&array).try_get_value(0)?, json!($expected)); + }}; + } + assert_number!(Int8Array::from(vec![-8]), -8); + assert_number!(Int16Array::from(vec![-16]), -16); + assert_number!(Int32Array::from(vec![-32]), -32); + assert_number!(UInt8Array::from(vec![8]), 8); + assert_number!(UInt16Array::from(vec![16]), 16); + assert_number!(UInt32Array::from(vec![32]), 32); + assert_number!(Float32Array::from(vec![1.25]), 1.25); + let floats: ArrayRef = Arc::new(Float64Array::from(vec![Some(1.5)])); assert_eq!(JsonArray::from(&floats).try_get_value(0)?, json!(1.5)); @@ -529,15 +640,6 @@ mod test { json!({"flag": null, "items": [2]}) ); - let unsupported: ArrayRef = Arc::new(Int32Array::from(vec![1])); - assert_eq!( - JsonArray::from(&unsupported) - .try_get_value(0) - .unwrap_err() - .to_string(), - "Invalid JSON: unknown JSON type Int32" - ); - Ok(()) } @@ -915,4 +1017,124 @@ mod test { Ok(()) } + + #[test] + fn test_reconstruct_json2_v2_value() -> Result<()> { + let remainders = json_values_to_variant(&[ + Some(json!({"cold": 1, "nested": {"right": true}})), + Some(json!({"!__remainder__!": "user value"})), + ])?; + let remainder = Arc::new(variant_field(JSON2_REMAINDER_FIELD_NAME, true)); + let nested = Arc::new(Field::new_struct( + "nested", + [Arc::new(Field::new("left", DataType::Utf8, true))], + true, + )); + let nested_values: ArrayRef = Arc::new(StructArray::from(vec![( + Arc::new(Field::new("left", DataType::Utf8, true)), + Arc::new(StringArray::from(vec![Some("value"), None])) as ArrayRef, + )])); + let fields = Fields::from(vec![ + remainder, + Arc::new(Field::new("count", DataType::Int64, true)), + nested, + ]); + let array: ArrayRef = Arc::new(StructArray::new( + fields.clone(), + vec![ + remainders, + Arc::new(Int64Array::from(vec![Some(42), None])), + nested_values, + ], + None, + )); + let field = Field::new("data", DataType::Struct(fields), true).with_extension_type( + Json2ExtensionType::new(Arc::new(JsonMetadata::new_v2(JsonSettings::default()))), + ); + + assert_eq!( + json!({ + "cold": 1, + "count": 42, + "nested": {"left": "value", "right": true} + }), + JsonArray::from(&array).json2_values()?[0] + ); + assert_eq!( + json!({ + "!__remainder__!": "user value", + "count": null, + "nested": {"left": null} + }), + JsonArray::from(&array).json2_values()?[1] + ); + + let target = DataType::Struct( + vec![ + Arc::new(Field::new("cold", DataType::UInt64, true)), + Arc::new(Field::new("count", DataType::Int64, true)), + ] + .into(), + ); + let projected = JsonArray::from(&array).project_to_v2(&field, &target)?; + assert_eq!( + json!({"cold": 1, "count": 42}), + JsonArray::from(&projected).try_get_value(0)? + ); + assert_eq!( + json!({"cold": null, "count": null}), + JsonArray::from(&projected).try_get_value(1)? + ); + Ok(()) + } + + #[test] + fn test_project_partial_json2_v2_without_remainder() -> Result<()> { + let fields = Fields::from(vec![Arc::new(Field::new("hot", DataType::Int64, true))]); + let array: ArrayRef = Arc::new(StructArray::new( + fields.clone(), + vec![Arc::new(Int64Array::from(vec![1, 2]))], + None, + )); + let field = Field::new("data", DataType::Struct(fields), true).with_extension_type( + Json2ExtensionType::new(Arc::new(JsonMetadata::new_v2(JsonSettings::default()))), + ); + + let projected = JsonArray::from(&array).project_to_v2(&field, field.data_type())?; + assert!(Arc::ptr_eq(&array, &projected)); + Ok(()) + } + + #[test] + fn test_reject_conflict_json2_v2_path() -> Result<()> { + let remainders = json_values_to_variant(&[Some(json!({"count": 1}))])?; + let fields = Fields::from(vec![ + Arc::new(variant_field(JSON2_REMAINDER_FIELD_NAME, true)), + Arc::new(Field::new("count", DataType::Int64, true)), + ]); + let array: ArrayRef = Arc::new(StructArray::new( + fields, + vec![remainders, Arc::new(Int64Array::from(vec![2]))], + None, + )); + let error = JsonArray::from(&array).json2_values().unwrap_err(); + assert!( + error.to_string().contains( + "cannot merge 'count' in explicit fields and remainder: not both objects" + ) + ); + + let Value::Object(mut remainder) = json!({"nested": {"count": 1}}) else { + unreachable!(); + }; + let error = merge_explicit_value( + &mut remainder, + "nested".to_string(), + json!({"count": 2}), + &mut Vec::new(), + ) + .unwrap_err(); + assert!(error.to_string().contains("cannot merge 'nested.count'")); + Ok(()) + } } diff --git a/src/datatypes/src/vectors/json/variant.rs b/src/datatypes/src/vectors/json/variant.rs new file mode 100644 index 00000000000..1a4b8a861d5 --- /dev/null +++ b/src/datatypes/src/vectors/json/variant.rs @@ -0,0 +1,239 @@ +// Copyright 2023 Greptime Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::sync::Arc; + +use arrow_array::ArrayRef; +#[cfg(test)] +use arrow_schema::ArrowError; +use arrow_schema::{DataType, Field}; +#[cfg(test)] +use parquet_variant::{ObjectFieldBuilder, Variant, VariantBuilderExt, VariantDecimal16}; +#[cfg(test)] +use parquet_variant_compute::VariantArrayBuilder; +use parquet_variant_compute::{VariantArray, VariantType}; +use parquet_variant_json::VariantToJson; +use snafu::ResultExt; + +use crate::error::{ArrowComputeSnafu, Result}; +#[cfg(test)] +use crate::json::value::{JsonNumber, JsonVariant, decode_json_variant}; + +/// Returns the canonical Arrow field for an unshredded Parquet Variant array. +pub fn variant_field(name: impl Into, nullable: bool) -> Field { + Field::new_struct( + name, + [ + Arc::new(Field::new("metadata", DataType::BinaryView, false)), + Arc::new(Field::new("value", DataType::BinaryView, false)), + ], + nullable, + ) + .with_extension_type(VariantType) +} + +#[cfg(test)] +pub(crate) fn json_values_to_variant(values: &[Option]) -> Result { + let mut builder = VariantArrayBuilder::new(values.len()); + for value in values { + match value { + Some(value) => append_json_value(&mut builder, value).context(ArrowComputeSnafu)?, + None => builder.append_null(), + } + } + Ok(ArrayRef::from(builder.build())) +} + +/// Encodes JSON variants as an unshredded Parquet Variant array. +#[cfg(test)] +fn json_variants_to_variant(values: &[Option]) -> Result { + let mut builder = VariantArrayBuilder::new(values.len()); + for value in values { + match value { + Some(value) => append_json_variant(&mut builder, value).context(ArrowComputeSnafu)?, + None => builder.append_null(), + } + } + Ok(ArrayRef::from(builder.build())) +} + +#[cfg(test)] +fn append_json_variant( + builder: &mut impl VariantBuilderExt, + value: &JsonVariant, +) -> std::result::Result<(), ArrowError> { + match value { + JsonVariant::Null => builder.append_value(Variant::Null), + JsonVariant::Bool(value) => builder.append_value(*value), + JsonVariant::Number(JsonNumber::PosInt(value)) => { + if let Ok(value) = i64::try_from(*value) { + builder.append_value(value); + } else { + append_large_u64(builder, *value)?; + } + } + JsonVariant::Number(JsonNumber::NegInt(value)) => builder.append_value(*value), + JsonVariant::Number(JsonNumber::Float(value)) => { + if value.0.is_finite() { + builder.append_value(value.0) + } else { + builder.append_value("NaN") + } + } + JsonVariant::String(value) => builder.append_value(value.as_str()), + JsonVariant::Array(values) => { + let mut list = builder.try_new_list()?; + for value in values { + append_json_variant(&mut list, value)?; + } + list.finish(); + } + JsonVariant::Object(values) => { + let mut object = builder.try_new_object()?; + for (name, value) in values { + append_json_variant(&mut ObjectFieldBuilder::new(name, &mut object), value)?; + } + object.finish(); + } + JsonVariant::Variant(value) => { + let value = decode_json_variant(value) + .map_err(|e| ArrowError::JsonError(format!("Failed to decode JSONB: {e}")))?; + append_json_value(builder, &value)?; + } + } + Ok(()) +} + +#[cfg(test)] +fn append_json_value( + builder: &mut impl VariantBuilderExt, + value: &serde_json::Value, +) -> std::result::Result<(), ArrowError> { + match value { + serde_json::Value::Null => builder.append_value(Variant::Null), + serde_json::Value::Bool(value) => builder.append_value(*value), + serde_json::Value::Number(value) => { + if let Some(value) = value.as_i64() { + builder.append_value(value); + } else if let Some(value) = value.as_u64() { + append_large_u64(builder, value)?; + } else if let Some(value) = value.as_f64() { + builder.append_value(value); + } else { + return Err(ArrowError::InvalidArgumentError(format!( + "Failed to encode JSON number as Variant: {value}" + ))); + } + } + serde_json::Value::String(value) => builder.append_value(value.as_str()), + serde_json::Value::Array(values) => { + let mut list = builder.try_new_list()?; + for value in values { + append_json_value(&mut list, value)?; + } + list.finish(); + } + serde_json::Value::Object(values) => { + let mut object = builder.try_new_object()?; + for (name, value) in values { + append_json_value(&mut ObjectFieldBuilder::new(name, &mut object), value)?; + } + object.finish(); + } + } + Ok(()) +} + +/// Parquet Variant has no unsigned integer primitive. Treat u64 as i64 first, then use Decimal16 +/// to represent large (larger than i64::MAX) u64. +#[cfg(test)] +fn append_large_u64( + builder: &mut impl VariantBuilderExt, + value: u64, +) -> std::result::Result<(), ArrowError> { + let value = VariantDecimal16::try_new(value as i128, 0).map_err(|e| { + ArrowError::InvalidArgumentError(format!( + "Failed to encode JSON large integer as Variant Decimal16: {e}" + )) + })?; + builder.append_value(value); + Ok(()) +} + +/// Decodes an unshredded Parquet Variant array into JSON values. +pub fn variant_to_json_values(array: &ArrayRef) -> Result>> { + let variants = VariantArray::try_new(array.as_ref()).context(ArrowComputeSnafu)?; + (0..variants.len()) + .map(|i| { + if variants.is_null(i) { + Ok(None) + } else { + variants + .try_value(i) + .and_then(|x| x.to_json_value()) + .context(ArrowComputeSnafu) + .map(Some) + } + }) + .collect() +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + + #[test] + fn test_variant_field_matches_canonical_layout() { + let expected = VariantArrayBuilder::new(0) + .build() + .field("remainder") + .with_nullable(true); + assert_eq!(expected, variant_field("remainder", true)); + } + + #[test] + fn test_variant_json_round_trip() -> Result<()> { + let values = vec![ + None, + Some(serde_json::Value::Null), + Some(json!({})), + Some(json!({"nested": {"items": [1, "two", null]}})), + Some(json!({"unicode": "\u{503c}"})), + Some(json!({"max_u64": u64::MAX})), + ]; + + let array = json_values_to_variant(&values)?; + assert_eq!(values, variant_to_json_values(&array)?); + + let variants = values + .clone() + .into_iter() + .map(|x| x.map(JsonVariant::from)) + .collect::>(); + let array = json_variants_to_variant(&variants)?; + assert_eq!(values, variant_to_json_values(&array)?); + + let variants = [Some(JsonVariant::Variant( + jsonb::parse_value(br#"{"nested": true}"#).unwrap().to_vec(), + ))]; + let array = json_variants_to_variant(&variants)?; + assert_eq!( + vec![Some(json!({"nested": true}))], + variant_to_json_values(&array)? + ); + Ok(()) + } +} From 8dbfbee611cc9d3996830ac8aeebffeb755c196f Mon Sep 17 00:00:00 2001 From: Ning Sun Date: Wed, 19 Aug 2026 03:47:44 +0000 Subject: [PATCH 04/10] refactor: remove open metrics parser (#8905) --- Cargo.lock | 18 ------------------ src/frontend/src/instance/prom_store.rs | 10 +--------- src/servers/Cargo.toml | 1 - src/servers/src/http/prom_store.rs | 9 --------- src/servers/src/prom_store.rs | 6 ------ src/servers/src/query_handler.rs | 3 --- src/servers/tests/http/prom_store_test.rs | 6 +----- 7 files changed, 2 insertions(+), 51 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 16b4c6b1045..d7c9241afca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -977,12 +977,6 @@ dependencies = [ "syn 2.0.117", ] -[[package]] -name = "auto_ops" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7460f7dd8e100147b82a63afca1a20eb6c231ee36b90ba7272e14951cb58af59" - [[package]] name = "autocfg" version = "1.5.0" @@ -9802,17 +9796,6 @@ dependencies = [ "url", ] -[[package]] -name = "openmetrics-parser" -version = "0.4.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e40a68c62e09c5dfec2f6472af3bd5e8ddf506fcf14c78ece23794ffbb874eca" -dependencies = [ - "auto_ops", - "pest", - "pest_derive", -] - [[package]] name = "opensrv-mysql" version = "0.8.0" @@ -13461,7 +13444,6 @@ dependencies = [ "notify", "object-pool", "once_cell", - "openmetrics-parser", "opensrv-mysql", "opentelemetry-proto", "operator", diff --git a/src/frontend/src/instance/prom_store.rs b/src/frontend/src/instance/prom_store.rs index 428628df123..e0b17b1a89e 100644 --- a/src/frontend/src/instance/prom_store.rs +++ b/src/frontend/src/instance/prom_store.rs @@ -47,7 +47,7 @@ use servers::http::header::{CONTENT_ENCODING_SNAPPY, CONTENT_TYPE_PROTOBUF, coll use servers::http::prom_store::PHYSICAL_TABLE_PARAM; use servers::interceptor::{PromStoreProtocolInterceptor, PromStoreProtocolInterceptorRef}; use servers::pending_rows_batcher::PendingRowsSchemaAlterer; -use servers::prom_store::{self, Metrics}; +use servers::prom_store; use servers::query_handler::{ PromStoreProtocolHandler, PromStoreProtocolHandlerRef, PromStoreResponse, }; @@ -648,10 +648,6 @@ impl PromStoreProtocolHandler for Instance { .fail(), } } - - async fn ingest_metrics(&self, _metrics: Metrics) -> ServerResult<()> { - todo!(); - } } impl Instance { @@ -791,10 +787,6 @@ impl PromStoreProtocolHandler for ExportMetricHandler { ) -> ServerResult { unreachable!(); } - - async fn ingest_metrics(&self, _metrics: Metrics) -> ServerResult<()> { - unreachable!(); - } } #[cfg(test)] diff --git a/src/servers/Cargo.toml b/src/servers/Cargo.toml index d64a1bd5a5d..59d00b90ad7 100644 --- a/src/servers/Cargo.toml +++ b/src/servers/Cargo.toml @@ -87,7 +87,6 @@ mysql_common = "0.38" notify.workspace = true object-pool = "0.5" once_cell.workspace = true -openmetrics-parser = "0.4" # Wait for https://github.com/databendlabs/opensrv/pull/81 opensrv-mysql = { git = "https://github.com/GreptimeTeam/opensrv", rev = "6c5a451544194b7bb60a8318d155d4f892b49f2c" } opentelemetry-proto.workspace = true diff --git a/src/servers/src/http/prom_store.rs b/src/servers/src/http/prom_store.rs index 1f0d26789eb..cb7b6307c4b 100644 --- a/src/servers/src/http/prom_store.rs +++ b/src/servers/src/http/prom_store.rs @@ -770,7 +770,6 @@ mod tests { use super::*; use crate::prom_remote_write::validation::PromValidationMode; - use crate::prom_store::Metrics; use crate::query_handler::PromStoreProtocolHandler; #[test] @@ -966,10 +965,6 @@ mod tests { ) -> Result { unimplemented!() } - - async fn ingest_metrics(&self, _metrics: Metrics) -> Result<()> { - unimplemented!() - } } #[tokio::test] @@ -1067,9 +1062,5 @@ mod tests { ) -> Result { unimplemented!() } - - async fn ingest_metrics(&self, _metrics: Metrics) -> Result<()> { - unimplemented!() - } } } diff --git a/src/servers/src/prom_store.rs b/src/servers/src/prom_store.rs index c4f2f929cae..980504515a8 100644 --- a/src/servers/src/prom_store.rs +++ b/src/servers/src/prom_store.rs @@ -34,7 +34,6 @@ use datafusion::dataframe::DataFrame; use datafusion::prelude::{Expr, col, lit, regexp_match}; use datafusion_common::ScalarValue; use datafusion_expr::LogicalPlan; -use openmetrics_parser::{MetricsExposition, PrometheusType, PrometheusValue}; use snafu::{OptionExt, ResultExt, ensure}; use snap::raw::{Decoder, Encoder}; @@ -95,11 +94,6 @@ pub fn is_physical_table_selection_label(label: &str) -> bool { label == PHYSICAL_TABLE_LABEL || label == PHYSICAL_TABLE_LABEL_ALT } -/// Metrics for push gateway protocol -pub struct Metrics { - pub exposition: MetricsExposition, -} - /// Get table name from remote query pub fn table_name(q: &Query) -> Result { let mut matchers = q diff --git a/src/servers/src/query_handler.rs b/src/servers/src/query_handler.rs index a6a086e0f23..96f412ac1e1 100644 --- a/src/servers/src/query_handler.rs +++ b/src/servers/src/query_handler.rs @@ -54,7 +54,6 @@ use crate::error::Result; use crate::http::jaeger::QueryTraceParams; use crate::influxdb::InfluxdbRequest; use crate::opentsdb::codec::DataPoint; -use crate::prom_store::Metrics; pub type OpentsdbProtocolHandlerRef = Arc; pub type InfluxdbLineProtocolHandlerRef = Arc; pub type PromStoreProtocolHandlerRef = Arc; @@ -125,8 +124,6 @@ pub trait PromStoreProtocolHandler { /// Handling prometheus remote read requests async fn read(&self, request: ReadRequest, ctx: QueryContextRef) -> Result; - /// Handling push gateway requests - async fn ingest_metrics(&self, metrics: Metrics) -> Result<()>; } #[async_trait] diff --git a/src/servers/tests/http/prom_store_test.rs b/src/servers/tests/http/prom_store_test.rs index 6e8658f663f..8f3e2b0e770 100644 --- a/src/servers/tests/http/prom_store_test.rs +++ b/src/servers/tests/http/prom_store_test.rs @@ -44,7 +44,7 @@ use servers::http::{HttpOptions, HttpServerBuilder}; use servers::prom_remote_write::v2::test_util as remote_write_v2; use servers::prom_remote_write::validation::PromValidationMode; use servers::prom_store; -use servers::prom_store::{Metrics, snappy_compress}; +use servers::prom_store::snappy_compress; use servers::query_handler::sql::SqlQueryHandler; use servers::query_handler::{PromStoreProtocolHandler, PromStoreResponse}; use session::context::QueryContextRef; @@ -146,10 +146,6 @@ impl PromStoreProtocolHandler for DummyInstance { body: response.encode_to_vec(), }) } - - async fn ingest_metrics(&self, _metrics: Metrics) -> Result<()> { - unimplemented!(); - } } #[async_trait] From 3de4feddd5a5c6a282d5b2eae7541937de6bcdf8 Mon Sep 17 00:00:00 2001 From: dennis zhuang Date: Wed, 19 Aug 2026 04:32:23 +0000 Subject: [PATCH 05/10] feat(otlp): report the cause of rejected trace spans (#8897) * feat(otlp): report the cause of rejected trace spans When trace-v1 ingestion cannot coerce an attribute value, it falls back to single-span writes and rejects the bad span. That behavior is correct, but the OTLP partial-success message only carried `Rejected span : (InvalidArguments)`: the column, the source value, the source type and the target type were all dropped, so locating the bad attribute required adding a detailed exporter on the collector side and replaying traffic. Two places lost the information. `prepare_trace_column_rewrites` built a message without the failing value, and the span rejection path kept only the status code from the error. Coercion errors now name the failing value, e.g. failed to coerce trace column 'span_attributes.http.response.body.size' in table 'opentelemetry_traces' from String("") to Int64 and the rejection detail carries that cause. Values are user data, so a string keeps at most 16 characters, is escaped, and binary payloads report only their length; the cause itself is bounded at 256 characters. Both truncations cut on a char boundary. Failure details now deduplicate: repeats of the same (site, cause) collapse into one entry with an occurrence count, keyed on the untruncated cause so two failures that differ past the display limit stay separate. Only four distinct entries are retained and the rest are counted, which keeps the state bounded no matter how many distinct bad values a request carries. A fully rejected request is logged at warn level and a partial success at debug level, since the latter repeats every export interval; the detail goes out as a Debug field so a newline in an attribute key cannot forge log lines. Rejection semantics are unchanged: partial success, same accepted and rejected counts, same HTTP status mapping. Signed-off-by: Dennis Zhuang * refactor(otlp): compare failure keys directly instead of hashing The dedup identity was a DefaultHasher fingerprint of `(label, key)`, which bought a fixed 8 bytes per entry at the cost of an import, four lines, and a collision argument the reader has to make. Entries are capped at four and a cause runs a couple of hundred characters, so the saving is about a kilobyte per in-flight request while the column name it avoids retaining is already held several times over by the request itself. Compare the strings instead, keeping the untruncated cause as the key so failures differing past the display limit still stay apart. Labels are metric label values and always static, so the entry borrows them. Signed-off-by: Dennis Zhuang --------- Signed-off-by: Dennis Zhuang --- src/frontend/src/instance/otlp/README.md | 12 ++ .../src/instance/otlp/trace_ingest.rs | 172 ++++++++++++++++-- .../src/instance/otlp/trace_ingest/tests.rs | 151 ++++++++++++--- src/frontend/src/instance/otlp/trace_types.rs | 153 ++++++++++++++-- tests-integration/tests/http.rs | 9 + 5 files changed, 436 insertions(+), 61 deletions(-) diff --git a/src/frontend/src/instance/otlp/README.md b/src/frontend/src/instance/otlp/README.md index 8db27384183..4489ab0c3f3 100644 --- a/src/frontend/src/instance/otlp/README.md +++ b/src/frontend/src/instance/otlp/README.md @@ -158,6 +158,18 @@ Accounting follows the main-table write: accepted or rejected span counts; and - failure details are bounded before they are folded into `TraceIngestOutcome`. +A rejection detail carries the failing cause, not just a status code, so an +unusable attribute value names its column, source value, and target type. Repeats +of the same `(site, cause)` collapse into one entry with an occurrence count, and +only a fixed number of distinct entries is retained; failures past that limit are +counted but their causes are dropped, which keeps the state bounded regardless of +how many distinct bad values a request carries. + +A request that rejects every span is logged at warn level. A partial success is +logged at debug level only: it repeats every export interval while the sender +keeps emitting the value, and its detail already reaches the sender through the +OTLP partial-success message. + Finally, the HTTP handler returns: - full success when there are no rejected spans or failure details; diff --git a/src/frontend/src/instance/otlp/trace_ingest.rs b/src/frontend/src/instance/otlp/trace_ingest.rs index f6ce60b2d11..671f6c6f7cf 100644 --- a/src/frontend/src/instance/otlp/trace_ingest.rs +++ b/src/frontend/src/instance/otlp/trace_ingest.rs @@ -25,7 +25,7 @@ use client::Output; use common_error::ext::{BoxedError, ErrorExt}; use common_error::status_code::StatusCode; use common_meta::rpc::ddl::TriggerReason; -use common_telemetry::warn; +use common_telemetry::{debug, warn}; use datatypes::prelude::ConcreteDataType; use opentelemetry_proto::tonic::collector::trace::v1::ExportTraceServiceRequest; use pipeline::{GreptimePipelineParams, PipelineWay}; @@ -50,11 +50,15 @@ use crate::instance::otlp::trace_types::{ PendingTraceColumnRewrite, PreparedTraceColumnRewrites, TraceColumnRewriteError, choose_trace_reconcile_decision, enrich_trace_reconcile_error, is_trace_reconcile_candidate_type, prepare_trace_column_rewrites, push_observed_trace_type, + truncate_for_diagnostics, }; use crate::metrics::{OTLP_TRACES_FAILURE_COUNT, OTLP_TRACES_ROWS}; const TRACE_FAILURE_MESSAGE_LIMIT: usize = 4; +/// Maximum characters of a failure cause echoed to the client and the log. +const TRACE_FAILURE_CAUSE_LIMIT: usize = 256; + /// Determines how trace ingestion responds to a failure before a write is dispatched. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum ChunkFailureReaction { @@ -86,7 +90,34 @@ struct TraceChunkIngestContext<'a> { struct TraceIngestState { aux_data: TraceAuxData, outcome: TraceIngestOutcome, - failure_messages: Vec, + failure_messages: TraceFailureMessages, +} + +/// Bounded, deduplicated failure details for one trace request. +/// +/// Occurrences past [`TRACE_FAILURE_MESSAGE_LIMIT`] distinct failures are +/// counted but their keys are dropped: retaining them would let this state grow +/// with the number of distinct bad values in a request. +#[derive(Debug, Default)] +struct TraceFailureMessages { + entries: Vec, + suppressed_occurrences: usize, +} + +#[derive(Debug)] +struct TraceFailureEntry { + label: &'static str, + /// Untruncated: two causes can agree on a truncated prefix and differ + /// exactly where the actionable detail is. + key: String, + message: String, + occurrences: usize, +} + +impl TraceFailureMessages { + fn is_empty(&self) -> bool { + self.entries.is_empty() + } } /// How a v1 chunk should be reconciled when it is written. @@ -863,7 +894,7 @@ impl Instance { let mut ingest_state = TraceIngestState { aux_data: TraceAuxData::default(), outcome: TraceIngestOutcome::default(), - failure_messages: Vec::new(), + failure_messages: TraceFailureMessages::default(), }; let main_result: ServerResult<()> = async { @@ -954,6 +985,7 @@ impl Instance { Self::push_trace_failure_message( &mut ingest_state.failure_messages, "aux_table_update_failed", + err.status_code().as_ref(), format!( "Auxiliary trace tables were not fully updated ({})", err.status_code().as_ref() @@ -968,6 +1000,7 @@ impl Instance { Err(err) => Self::push_trace_failure_message( &mut ingest_state.failure_messages, "aux_table_update_failed", + err.status_code().as_ref(), format!( "Auxiliary trace tables were not fully updated ({})", err.status_code().as_ref() @@ -985,6 +1018,35 @@ impl Instance { ingest_state.failure_messages, ); + if let Some(error_message) = &ingest_state.outcome.error_message { + let accepted_spans = ingest_state.outcome.accepted_spans; + let rejected_spans = ingest_state.outcome.rejected_spans; + // A partial success repeats every export interval while the sender + // keeps emitting the bad value, so only a fully rejected request is + // worth a warning. Either way the detail reaches the sender: as an + // OTLP partial success, or as the status message of a 400. + // + // The detail embeds attribute keys verbatim, so it goes out as a + // Debug field; interpolating it would let a newline in an attribute + // key forge log lines. + if accepted_spans == 0 && rejected_spans > 0 { + warn!( + table_name = ingest_ctx.table_name, + rejected_spans, + error_message = ?error_message, + "OTLP trace ingest rejected every span" + ); + } else { + debug!( + table_name = ingest_ctx.table_name, + accepted_spans, + rejected_spans, + error_message = ?error_message, + "OTLP trace ingest reported failures" + ); + } + } + Ok(ingest_state.outcome) } @@ -1019,6 +1081,7 @@ impl Instance { Self::push_trace_failure_message( &mut ingest_state.failure_messages, ChunkFailureReaction::Propagate.as_metric_label(), + err.status_code().as_ref(), format!( "Propagating chunk write failure ({})", err.status_code().as_ref() @@ -1069,13 +1132,18 @@ impl Instance { // recover valid data. let span_count = chunks.iter().map(|chunk| chunk.rows.len()).sum::(); ingest_state.outcome.rejected_spans += span_count; + let (cause, shown_cause) = Self::trace_failure_cause(&err); Self::push_trace_failure_message( &mut ingest_state.failure_messages, ChunkFailureReaction::DiscardChunk.as_metric_label(), + // Merging discards of different sizes would make the reported + // count times its occurrences wrong. + &format!("{span_count}:{cause}"), format!( - "Discarded {} spans after pre-write request failure ({})", + "Discarded {} spans after pre-write request failure ({}): {}", span_count, - err.status_code().as_ref() + err.status_code().as_ref(), + shown_cause ), ); return Ok(()); @@ -1110,6 +1178,7 @@ impl Instance { Self::push_trace_failure_message( &mut ingest_state.failure_messages, ChunkFailureReaction::RetryPerSpan.as_metric_label(), + "incompatible_binary_and_json", "Chunk fallback triggered by incompatible binary and JSON values".to_string(), ); return self @@ -1136,6 +1205,7 @@ impl Instance { Self::push_trace_failure_message( &mut ingest_state.failure_messages, ChunkFailureReaction::RetryPerSpan.as_metric_label(), + err.status_code().as_ref(), format!("Chunk fallback triggered by {}", err.status_code().as_ref()), ); return self @@ -1144,13 +1214,19 @@ impl Instance { } ChunkFailureReaction::DiscardChunk => { ingest_state.outcome.rejected_spans += span_count; + let (cause, shown_cause) = Self::trace_failure_cause(&err); Self::push_trace_failure_message( &mut ingest_state.failure_messages, ChunkFailureReaction::DiscardChunk.as_metric_label(), + // Chunks discarded for one cause differ in size; merging + // them would make the reported count times its + // occurrences wrong. + &format!("{span_count}:{cause}"), format!( - "Discarded {} spans after pre-write chunk failure ({})", + "Discarded {} spans after pre-write chunk failure ({}): {}", span_count, - err.status_code().as_ref() + err.status_code().as_ref(), + shown_cause ), ); return Ok(()); @@ -1159,6 +1235,7 @@ impl Instance { Self::push_trace_failure_message( &mut ingest_state.failure_messages, ChunkFailureReaction::Propagate.as_metric_label(), + err.status_code().as_ref(), format!( "Propagating pre-write chunk failure ({})", err.status_code().as_ref() @@ -1182,6 +1259,7 @@ impl Instance { Self::push_trace_failure_message( &mut ingest_state.failure_messages, ChunkFailureReaction::Propagate.as_metric_label(), + err.status_code().as_ref(), format!( "Propagating chunk write failure ({})", err.status_code().as_ref() @@ -1221,6 +1299,7 @@ impl Instance { Self::push_trace_failure_message( &mut ingest_state.failure_messages, ChunkFailureReaction::Propagate.as_metric_label(), + err.status_code().as_ref(), format!( "Propagating pre-write span failure for {}:{} ({})", span.trace_id, @@ -1232,14 +1311,19 @@ impl Instance { } ingest_state.outcome.rejected_spans += 1; + // Dedup on the cause, not the span id: one bad column rejects + // every span and would otherwise fill the entry list. + let (cause, shown_cause) = Self::trace_failure_cause(&err); Self::push_trace_failure_message( &mut ingest_state.failure_messages, "span_rejected", + &cause, format!( - "Rejected span {}:{} ({})", + "Rejected span {}:{} ({}): {}", span.trace_id, span.span_id, - err.status_code().as_ref() + err.status_code().as_ref(), + shown_cause ), ); continue; @@ -1256,6 +1340,7 @@ impl Instance { Self::push_trace_failure_message( &mut ingest_state.failure_messages, ChunkFailureReaction::Propagate.as_metric_label(), + err.status_code().as_ref(), format!( "Propagating span write failure for {}:{} ({})", span.trace_id, @@ -1619,24 +1704,52 @@ impl Instance { outcome.write_cost += cost; } - fn push_trace_failure_message(messages: &mut Vec, label: &str, message: String) { + /// Returns the full cause of a pre-write failure and its display form. + /// + /// `output_msg` masks internal errors and unwraps the root cause. Dedup must + /// key on the full text: two causes can agree on a truncated prefix and + /// differ exactly where the actionable detail is. + fn trace_failure_cause(err: &error::Error) -> (String, String) { + let cause = err.output_msg(); + let display = truncate_for_diagnostics(&cause, TRACE_FAILURE_CAUSE_LIMIT); + (cause, display) + } + + /// Records one failure, merging repeats of `(label, key)` into a count. + fn push_trace_failure_message( + messages: &mut TraceFailureMessages, + label: &'static str, + key: &str, + message: String, + ) { OTLP_TRACES_FAILURE_COUNT.with_label_values(&[label]).inc(); - if messages.len() < TRACE_FAILURE_MESSAGE_LIMIT { - messages.push(message); - } else if messages.len() == TRACE_FAILURE_MESSAGE_LIMIT { - tracing::debug!( - label, - limit = TRACE_FAILURE_MESSAGE_LIMIT, - "Trace ingest failure message limit reached; suppressing additional failure details" - ); + if let Some(entry) = messages + .entries + .iter_mut() + .find(|entry| entry.label == label && entry.key == key) + { + entry.occurrences += 1; + return; } + + if messages.entries.len() >= TRACE_FAILURE_MESSAGE_LIMIT { + messages.suppressed_occurrences += 1; + return; + } + + messages.entries.push(TraceFailureEntry { + label, + key: key.to_string(), + message, + occurrences: 1, + }); } fn finish_trace_failure_message( accepted_spans: usize, rejected_spans: usize, - messages: Vec, + messages: TraceFailureMessages, ) -> Option { if rejected_spans == 0 && messages.is_empty() { return None; @@ -1648,8 +1761,27 @@ impl Instance { ); if !messages.is_empty() { + let details = messages + .entries + .into_iter() + .map(|entry| { + if entry.occurrences > 1 { + format!("{} (x{})", entry.message, entry.occurrences) + } else { + entry.message + } + }) + .collect::>() + .join("; "); summary.push_str(": "); - summary.push_str(&messages.join("; ")); + summary.push_str(&details); + } + + if messages.suppressed_occurrences > 0 { + summary.push_str(&format!( + "; {} additional failures suppressed", + messages.suppressed_occurrences + )); } Some(summary) diff --git a/src/frontend/src/instance/otlp/trace_ingest/tests.rs b/src/frontend/src/instance/otlp/trace_ingest/tests.rs index b3a0570db51..ec5bad89754 100644 --- a/src/frontend/src/instance/otlp/trace_ingest/tests.rs +++ b/src/frontend/src/instance/otlp/trace_ingest/tests.rs @@ -34,8 +34,8 @@ use servers::otlp::trace::span::{SpanEvents, SpanLinks, TraceSpan}; use servers::otlp::trace::v1::{TraceBinaryType, TraceRetryColumn}; use super::{ - ChunkFailureReaction, Instance, TraceChunkRetry, TraceChunkSchemaState, TraceRequestSchema, - TraceRequestSchemaPlan, TraceSpanMetadata, TraceTablePreAlter, chunk_owned, + ChunkFailureReaction, Instance, TraceChunkRetry, TraceChunkSchemaState, TraceFailureMessages, + TraceRequestSchema, TraceRequestSchemaPlan, TraceSpanMetadata, TraceTablePreAlter, chunk_owned, wrap_trace_alter_failure, }; use crate::metrics::OTLP_TRACES_FAILURE_COUNT; @@ -102,22 +102,28 @@ fn test_classify_trace_prewrite_failure() { #[test] fn test_finish_trace_failure_message() { - let message = Instance::finish_trace_failure_message( - 3, - 2, - vec!["Rejected span trace:span (InvalidArguments)".to_string()], - ) - .unwrap(); + let mut messages = TraceFailureMessages::default(); + Instance::push_trace_failure_message( + &mut messages, + "span_rejected", + "coercion", + "Rejected span trace:span (InvalidArguments)".to_string(), + ); + + let message = Instance::finish_trace_failure_message(3, 2, messages).unwrap(); assert!(message.contains("Accepted 3 spans, rejected 2 spans")); assert!(message.contains("Rejected span trace:span")); - assert_eq!(Instance::finish_trace_failure_message(2, 0, vec![]), None); + assert_eq!( + Instance::finish_trace_failure_message(2, 0, TraceFailureMessages::default()), + None + ); } #[test] fn test_finish_trace_failure_message_without_detail_messages() { assert_eq!( - Instance::finish_trace_failure_message(0, 2, vec![]), + Instance::finish_trace_failure_message(0, 2, TraceFailureMessages::default()), Some("Accepted 0 spans, rejected 2 spans".to_string()) ); } @@ -126,39 +132,138 @@ fn test_finish_trace_failure_message_without_detail_messages() { fn test_push_trace_failure_message_increments_labeled_counter() { let label = "retry_per_span_counter_test"; let initial = OTLP_TRACES_FAILURE_COUNT.with_label_values(&[label]).get(); - let mut messages = Vec::new(); + let mut messages = TraceFailureMessages::default(); Instance::push_trace_failure_message( &mut messages, label, + "InvalidArguments", "Chunk fallback triggered by InvalidArguments".to_string(), ); - assert_eq!(messages.len(), 1); + assert_eq!(messages.entries.len(), 1); assert_eq!( OTLP_TRACES_FAILURE_COUNT.with_label_values(&[label]).get(), initial + 1 ); } +/// One bad column rejects every span, so repeats must collapse instead of +/// filling the bounded entry list with the same cause. +#[test] +fn test_push_trace_failure_message_collapses_repeated_cause() { + let label = "span_rejected_dedup_test"; + let initial = OTLP_TRACES_FAILURE_COUNT.with_label_values(&[label]).get(); + let mut messages = TraceFailureMessages::default(); + + for idx in 0..3 { + Instance::push_trace_failure_message( + &mut messages, + label, + "failed to coerce", + format!("Rejected span trace:span-{idx} (InvalidArguments): failed to coerce"), + ); + } + + assert_eq!(messages.entries.len(), 1); + assert_eq!(messages.entries[0].occurrences, 3); + assert_eq!(messages.suppressed_occurrences, 0); + // Every occurrence is still metered even though the details collapse. + assert_eq!( + OTLP_TRACES_FAILURE_COUNT.with_label_values(&[label]).get(), + initial + 3 + ); + + let summary = Instance::finish_trace_failure_message(0, 3, messages).unwrap(); + assert!( + summary.contains("Rejected span trace:span-0 (InvalidArguments): failed to coerce (x3)"), + "unexpected summary: {summary}" + ); +} + +/// Dedup identity comes from the full cause, so two failures that only differ +/// past the display limit must stay separate instead of one silently vanishing. +#[test] +fn test_trace_failure_cause_keeps_identity_beyond_the_display_limit() { + let shared_prefix = "x".repeat(400); + let (first_cause, first_shown) = Instance::trace_failure_cause( + &servers::error::InvalidParameterSnafu { + reason: format!("{shared_prefix}-int64"), + } + .build(), + ); + let (second_cause, second_shown) = Instance::trace_failure_cause( + &servers::error::InvalidParameterSnafu { + reason: format!("{shared_prefix}-float64"), + } + .build(), + ); + + assert_eq!(first_shown, second_shown); + assert_ne!(first_cause, second_cause); + + let mut messages = TraceFailureMessages::default(); + Instance::push_trace_failure_message(&mut messages, "span_rejected", &first_cause, first_shown); + Instance::push_trace_failure_message( + &mut messages, + "span_rejected", + &second_cause, + second_shown, + ); + + assert_eq!(messages.entries.len(), 2); +} + +/// The same cause reported by two different sites must stay distinguishable. +#[test] +fn test_push_trace_failure_message_separates_labels_sharing_a_key() { + let mut messages = TraceFailureMessages::default(); + + Instance::push_trace_failure_message( + &mut messages, + "span_rejected", + "failed to coerce", + "Rejected span trace:span (InvalidArguments): failed to coerce".to_string(), + ); + Instance::push_trace_failure_message( + &mut messages, + "discard_chunk", + "failed to coerce", + "Discarded 7 spans after pre-write chunk failure (InvalidArguments): failed to coerce" + .to_string(), + ); + + assert_eq!(messages.entries.len(), 2); +} + #[test] fn test_push_trace_failure_message_caps_recorded_messages() { let label = "retry_per_span_limit_test"; - let mut messages = Vec::new(); + let mut messages = TraceFailureMessages::default(); - for idx in 0..=4 { - Instance::push_trace_failure_message(&mut messages, label, format!("failure-{idx}")); + for idx in 0..=5 { + Instance::push_trace_failure_message( + &mut messages, + label, + &format!("cause-{idx}"), + format!("failure-{idx}"), + ); } - assert_eq!(messages.len(), 4); assert_eq!( - messages, - vec![ - "failure-0".to_string(), - "failure-1".to_string(), - "failure-2".to_string(), - "failure-3".to_string() - ] + messages + .entries + .iter() + .map(|entry| entry.message.as_str()) + .collect::>(), + vec!["failure-0", "failure-1", "failure-2", "failure-3"] + ); + assert_eq!(messages.suppressed_occurrences, 2); + + let summary = Instance::finish_trace_failure_message(0, 6, messages).unwrap(); + assert!( + summary.ends_with("; 2 additional failures suppressed"), + "unexpected summary: {summary}" ); } diff --git a/src/frontend/src/instance/otlp/trace_types.rs b/src/frontend/src/instance/otlp/trace_types.rs index 0e2df25053e..603cc66c0d1 100644 --- a/src/frontend/src/instance/otlp/trace_types.rs +++ b/src/frontend/src/instance/otlp/trace_types.rs @@ -22,6 +22,39 @@ use servers::otlp::coerce::{ use crate::instance::otlp::trace_semconv::trace_semconv_fixed_type; +/// Attribute values are user data echoed back in the OTLP partial-success +/// message and the server log, so diagnostics keep at most this many characters. +const TRACE_VALUE_DIAGNOSTIC_LIMIT: usize = 16; + +/// Truncates to `limit` characters, marking the cut with `...`. +/// +/// Diagnostics carry user-controlled text; slicing by byte offset would panic in +/// the middle of a multi-byte sequence. +pub(super) fn truncate_for_diagnostics(text: &str, limit: usize) -> String { + match text.char_indices().nth(limit) { + Some((offset, _)) => format!("{}...", &text[..offset]), + None => text.to_string(), + } +} + +/// Renders a failing trace value as `Type(value)`, e.g. `String("")`. +fn describe_trace_value(value: &ValueData, request_type: ColumnDataType) -> String { + let payload = match value { + ValueData::StringValue(string_value) => format!( + "{:?}", + truncate_for_diagnostics(string_value, TRACE_VALUE_DIAGNOSTIC_LIMIT) + ), + ValueData::BoolValue(bool_value) => bool_value.to_string(), + ValueData::I64Value(int_value) => int_value.to_string(), + ValueData::F64Value(float_value) => float_value.to_string(), + ValueData::BinaryValue(bytes) => format!("{} bytes", bytes.len()), + // Other value kinds never reach trace coercion, so report the type alone. + _ => return format!("{request_type:?}"), + }; + + format!("{request_type:?}({payload})") +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(super) enum TraceReconcileDecision { UseExisting(ColumnDataType), @@ -191,8 +224,10 @@ pub(super) fn prepare_trace_column_rewrites( let Some(value) = row.values.get(pending_rewrite.col_idx) else { continue; }; - let Some(request_type) = value.value_data.as_ref().and_then(trace_value_datatype) - else { + let Some(request_value) = value.value_data.as_ref() else { + continue; + }; + let Some(request_type) = trace_value_datatype(request_value) else { continue; }; if request_type == pending_rewrite.target_type { @@ -201,20 +236,18 @@ pub(super) fn prepare_trace_column_rewrites( let value_data = coerce_value_data(&value.value_data, pending_rewrite.target_type, request_type) - .map_err(|_| { - TraceColumnRewriteError { - error: error::InvalidParameterSnafu { - reason: format!( - "failed to coerce trace column '{}' in table '{}' from {:?} to {:?}", - pending_rewrite.column_name, - table_name, - request_type, - pending_rewrite.target_type - ), - } - .build(), - column_name: pending_rewrite.column_name.clone(), - } + .map_err(|_| TraceColumnRewriteError { + error: error::InvalidParameterSnafu { + reason: format!( + "failed to coerce trace column '{}' in table '{}' from {} to {:?}", + pending_rewrite.column_name, + table_name, + describe_trace_value(request_value, request_type), + pending_rewrite.target_type + ), + } + .build(), + column_name: pending_rewrite.column_name.clone(), })?; values.push(PreparedTraceValueRewrite { row_idx, @@ -301,8 +334,8 @@ mod tests { use super::{ PendingTraceColumnRewrite, TraceReconcileDecision, choose_trace_reconcile_decision, - enrich_trace_reconcile_error, is_trace_reconcile_candidate_type, - prepare_trace_column_rewrites, push_observed_trace_type, + describe_trace_value, enrich_trace_reconcile_error, is_trace_reconcile_candidate_type, + prepare_trace_column_rewrites, push_observed_trace_type, truncate_for_diagnostics, }; #[test] @@ -458,6 +491,90 @@ mod tests { .unwrap_err(); assert_eq!(err.error.status_code(), StatusCode::InvalidArguments); assert_eq!(err.column_name, "span_attributes.attr_int"); + assert!( + err.error.to_string().contains( + "failed to coerce trace column 'span_attributes.attr_int' in table \ + 'trace_type_atomicity' from String(\"not_a_number\") to Int64" + ), + "unexpected error message: {}", + err.error + ); + } + + /// The PHP instrumentation case: an empty string must be distinguishable + /// from any other unparsable value in the reported diagnostics. + #[test] + fn test_prepare_trace_column_rewrites_reports_empty_string_value() { + let rows = vec![Row { + values: vec![Value { + value_data: Some(ValueData::StringValue(String::new())), + }], + }]; + let pending_rewrites = vec![PendingTraceColumnRewrite { + col_idx: 0, + target_type: ColumnDataType::Int64, + column_name: "span_attributes.http.response.body.size".to_string(), + }]; + + let err = prepare_trace_column_rewrites(&rows, pending_rewrites, "opentelemetry_traces") + .unwrap_err(); + assert!( + err.error.to_string().contains( + "'span_attributes.http.response.body.size' in table 'opentelemetry_traces' \ + from String(\"\") to Int64" + ), + "unexpected error message: {}", + err.error + ); + } + + #[test] + fn test_describe_trace_value_bounds_and_escapes_strings() { + assert_eq!( + describe_trace_value( + &ValueData::StringValue(String::new()), + ColumnDataType::String + ), + r#"String("")"# + ); + assert_eq!( + describe_trace_value( + &ValueData::StringValue("a\tb\"c".to_string()), + ColumnDataType::String + ), + r#"String("a\tb\"c")"# + ); + assert_eq!( + describe_trace_value( + &ValueData::StringValue("0123456789abcdefghij".to_string()), + ColumnDataType::String + ), + r#"String("0123456789abcdef...")"# + ); + } + + #[test] + fn test_describe_trace_value_omits_binary_content() { + assert_eq!( + describe_trace_value( + &ValueData::BinaryValue(vec![1_u8, 2, 3]), + ColumnDataType::Binary + ), + "Binary(3 bytes)" + ); + assert_eq!( + describe_trace_value(&ValueData::F64Value(1.5), ColumnDataType::Float64), + "Float64(1.5)" + ); + } + + /// Truncation runs over user-supplied text, so it must not split a + /// multi-byte character. + #[test] + fn test_truncate_for_diagnostics_cuts_on_char_boundary() { + assert_eq!(truncate_for_diagnostics("日本語テキスト", 3), "日本語..."); + assert_eq!(truncate_for_diagnostics("short", 16), "short"); + assert_eq!(truncate_for_diagnostics("exact", 5), "exact"); } #[test] diff --git a/tests-integration/tests/http.rs b/tests-integration/tests/http.rs index 079439bf78c..90cbb8c93fe 100644 --- a/tests-integration/tests/http.rs +++ b/tests-integration/tests/http.rs @@ -7782,6 +7782,15 @@ pub async fn test_otlp_traces_v1(store_type: StorageType) { ), "unexpected partial success body: {body:?}" ); + // The rejection must name the column, the failing value and the target type, + // otherwise locating the bad attribute needs a collector-side capture. + assert!( + partial_success.error_message.contains( + "failed to coerce trace column 'span_attributes.attr_int' in table \ + 'trace_type_abort' from String(\"not_a_number\") to Int64" + ), + "unexpected partial success body: {body:?}" + ); validate_data( "otlp_traces_v1_type_abort_rows", From 2182dccd9b76efc87e2e3ad7e7d355e62d9b1d03 Mon Sep 17 00:00:00 2001 From: "Lei, HUANG" <6406592+v0y4g3r@users.noreply.github.com> Date: Wed, 19 Aug 2026 10:04:50 +0000 Subject: [PATCH 06/10] fix: cap default runtime sizes to a minimum of 2 threads (#8908) * fix: cap default runtime sizes to a minimum of 2 threads RuntimeOptions derived its default sizes directly from num_cpus. On single-core machines every runtime (global, compact, query, ingest) ended up with one worker thread, which can easily deadlock async code (e.g. block_on combined with spawn). Clamp all CPU-derived runtime sizes to at least 2 threads. Signed-off-by: Lei, HUANG * fix: init logging before runtimes so runtime options are logged The global runtimes were initialized before the global logging subscriber, so the "Creating runtime ..." info logs that carry the runtime sizes were silently dropped. Initialize logging first in all node start paths; common-telemetry has no dependency on common-runtime, so the reorder is safe. Signed-off-by: Lei, HUANG --------- Signed-off-by: Lei, HUANG --- config/config.md | 10 ++--- config/datanode.example.toml | 4 +- config/frontend.example.toml | 2 +- config/metasrv.example.toml | 2 +- config/standalone.example.toml | 2 +- src/cmd/src/datanode/builder.rs | 6 +-- src/cmd/src/flownode.rs | 4 +- src/cmd/src/frontend.rs | 4 +- src/cmd/src/metasrv.rs | 4 +- src/cmd/src/standalone.rs | 4 +- src/common/runtime/src/global.rs | 67 +++++++++++++++++++++++++++----- 11 files changed, 78 insertions(+), 31 deletions(-) diff --git a/config/config.md b/config/config.md index 123eaf90a20..1b18565cc08 100644 --- a/config/config.md +++ b/config/config.md @@ -25,7 +25,7 @@ | `runtime` | -- | -- | The runtime options. | | `runtime.global_rt_size` | Integer | `8` | The number of threads to execute the runtime for global read operations. | | `runtime.compact_rt_size` | Integer | `4` | The number of threads to execute compact operations. | -| `runtime.compact_rt_max_blocking_threads` | Integer | `4` | The maximum number of blocking threads for compact operations.
Defaults to max(num_cpus / 2, 1). | +| `runtime.compact_rt_max_blocking_threads` | Integer | `4` | The maximum number of blocking threads for compact operations.
Defaults to max(num_cpus / 2, 2). | | `http` | -- | -- | The HTTP server options. | | `http.addr` | String | `127.0.0.1:4000` | The address to bind the HTTP server. | | `http.timeout` | String | `0s` | HTTP request timeout. Set to 0 to disable timeout.
When Prometheus pending-row batching is enabled, a nonzero timeout less than or equal to the
`prom_store.pending_rows_flush_interval` plus 1 second is adjusted to that value. | @@ -255,7 +255,7 @@ | `runtime` | -- | -- | The runtime options. | | `runtime.global_rt_size` | Integer | `8` | The number of threads to execute the runtime for global read operations. | | `runtime.compact_rt_size` | Integer | `4` | The number of threads to execute compact operations. | -| `runtime.compact_rt_max_blocking_threads` | Integer | `4` | The maximum number of blocking threads for compact operations.
Defaults to max(num_cpus / 2, 1). | +| `runtime.compact_rt_max_blocking_threads` | Integer | `4` | The maximum number of blocking threads for compact operations.
Defaults to max(num_cpus / 2, 2). | | `http` | -- | -- | The HTTP server options. | | `http.addr` | String | `127.0.0.1:4000` | The address to bind the HTTP server. | | `http.timeout` | String | `0s` | HTTP request timeout. Set to 0 to disable timeout.
When Prometheus pending-row batching is enabled, a nonzero timeout less than or equal to the
`prom_store.pending_rows_flush_interval` plus 1 second is adjusted to that value. | @@ -398,7 +398,7 @@ | `runtime` | -- | -- | The runtime options. | | `runtime.global_rt_size` | Integer | `8` | The number of threads to execute the runtime for global read operations. | | `runtime.compact_rt_size` | Integer | `4` | The number of threads to execute compact operations. | -| `runtime.compact_rt_max_blocking_threads` | Integer | `4` | The maximum number of blocking threads for compact operations.
Defaults to max(num_cpus / 2, 1). | +| `runtime.compact_rt_max_blocking_threads` | Integer | `4` | The maximum number of blocking threads for compact operations.
Defaults to max(num_cpus / 2, 2). | | `backend_tls` | -- | -- | TLS configuration for kv store backend (applicable for etcd, PostgreSQL, and MySQL backends)
When using etcd, PostgreSQL, or MySQL as metadata store, you can configure TLS here

Note: if TLS is configured in both this section and the `store_addrs` connection string, the
settings here will override the TLS settings in `store_addrs`. | | `backend_tls.mode` | String | `prefer` | TLS mode, refer to https://www.postgresql.org/docs/current/libpq-ssl.html
- "disable" - No TLS
- "prefer" (default) - Try TLS, fallback to plain
- "require" - Require TLS
- "verify_ca" - Require TLS and verify CA
- "verify_full" - Require TLS and verify hostname | | `backend_tls.cert_path` | String | `""` | Path to client certificate file (for client authentication)
Like "/path/to/client.crt" | @@ -509,8 +509,8 @@ | `runtime` | -- | -- | The runtime options. | | `runtime.global_rt_size` | Integer | `8` | The number of threads to execute the runtime for global read operations. | | `runtime.compact_rt_size` | Integer | `4` | The number of threads to execute compact operations. | -| `runtime.compact_rt_max_blocking_threads` | Integer | `4` | The maximum number of blocking threads for compact operations.
Defaults to max(num_cpus / 2, 1). | -| `runtime.query_rt_size` | Integer | `7` | The number of threads to execute datanode query operations.
Defaults to max(num_cpus - 1, 1). | +| `runtime.compact_rt_max_blocking_threads` | Integer | `4` | The maximum number of blocking threads for compact operations.
Defaults to max(num_cpus / 2, 2). | +| `runtime.query_rt_size` | Integer | `7` | The number of threads to execute datanode query operations.
Defaults to max(num_cpus - 1, 2). | | `runtime.ingest_rt_size` | Integer | `8` | The number of threads to execute datanode ingestion operations. | | `meta_client` | -- | -- | The metasrv client options. | | `meta_client.metasrv_addrs` | Array | -- | The addresses of the metasrv. | diff --git a/config/datanode.example.toml b/config/datanode.example.toml index 1f1377d91cc..5966405452a 100644 --- a/config/datanode.example.toml +++ b/config/datanode.example.toml @@ -84,10 +84,10 @@ watch = false ## The number of threads to execute compact operations. #+ compact_rt_size = 4 ## The maximum number of blocking threads for compact operations. -## Defaults to max(num_cpus / 2, 1). +## Defaults to max(num_cpus / 2, 2). #+ compact_rt_max_blocking_threads = 4 ## The number of threads to execute datanode query operations. -## Defaults to max(num_cpus - 1, 1). +## Defaults to max(num_cpus - 1, 2). #+ query_rt_size = 7 ## The number of threads to execute datanode ingestion operations. #+ ingest_rt_size = 8 diff --git a/config/frontend.example.toml b/config/frontend.example.toml index 260984a4ab3..ec1f9c89b42 100644 --- a/config/frontend.example.toml +++ b/config/frontend.example.toml @@ -44,7 +44,7 @@ default_column_prefix = "greptime" ## The number of threads to execute compact operations. #+ compact_rt_size = 4 ## The maximum number of blocking threads for compact operations. -## Defaults to max(num_cpus / 2, 1). +## Defaults to max(num_cpus / 2, 2). #+ compact_rt_max_blocking_threads = 4 ## The HTTP server options. diff --git a/config/metasrv.example.toml b/config/metasrv.example.toml index f607211d636..6017bfcc49a 100644 --- a/config/metasrv.example.toml +++ b/config/metasrv.example.toml @@ -92,7 +92,7 @@ node_max_idle_time = "24hours" ## The number of threads to execute compact operations. #+ compact_rt_size = 4 ## The maximum number of blocking threads for compact operations. -## Defaults to max(num_cpus / 2, 1). +## Defaults to max(num_cpus / 2, 2). #+ compact_rt_max_blocking_threads = 4 ## TLS configuration for kv store backend (applicable for etcd, PostgreSQL, and MySQL backends) diff --git a/config/standalone.example.toml b/config/standalone.example.toml index 068025e2a28..5ada605b92b 100644 --- a/config/standalone.example.toml +++ b/config/standalone.example.toml @@ -57,7 +57,7 @@ max_concurrent_queries = 0 ## The number of threads to execute compact operations. #+ compact_rt_size = 4 ## The maximum number of blocking threads for compact operations. -## Defaults to max(num_cpus / 2, 1). +## Defaults to max(num_cpus / 2, 2). #+ compact_rt_max_blocking_threads = 4 ## The HTTP server options. diff --git a/src/cmd/src/datanode/builder.rs b/src/cmd/src/datanode/builder.rs index afabcb85f10..3dfa03e52e5 100644 --- a/src/cmd/src/datanode/builder.rs +++ b/src/cmd/src/datanode/builder.rs @@ -61,9 +61,6 @@ impl InstanceBuilder { } async fn init(opts: &mut DatanodeOptions, plugins: &mut Plugins) -> Result> { - common_runtime::init_global_runtimes(&opts.runtime); - common_runtime::init_datanode_runtimes(&opts.runtime); - let dn_opts = &mut opts.component; let guard = common_telemetry::init_global_logging( APP_NAME, @@ -73,6 +70,9 @@ impl InstanceBuilder { None, ); + common_runtime::init_global_runtimes(&opts.runtime); + common_runtime::init_datanode_runtimes(&opts.runtime); + crate::options::flush_dropped_plugin_warnings(); log_versions(verbose_version(), short_version(), APP_NAME); maybe_activate_heap_profile(&dn_opts.memory); diff --git a/src/cmd/src/flownode.rs b/src/cmd/src/flownode.rs index ee071875c3a..eccfaa63b07 100644 --- a/src/cmd/src/flownode.rs +++ b/src/cmd/src/flownode.rs @@ -248,8 +248,6 @@ impl StartCommand { } async fn build(&self, opts: FlownodeOptions) -> Result { - common_runtime::init_global_runtimes(&opts.runtime); - let guard = common_telemetry::init_global_logging( APP_NAME, &opts.component.logging, @@ -258,6 +256,8 @@ impl StartCommand { None, ); + common_runtime::init_global_runtimes(&opts.runtime); + crate::options::flush_dropped_plugin_warnings(); log_versions(verbose_version(), short_version(), APP_NAME); maybe_activate_heap_profile(&opts.component.memory); diff --git a/src/cmd/src/frontend.rs b/src/cmd/src/frontend.rs index 9d5d0bab1ee..e3737088e8d 100644 --- a/src/cmd/src/frontend.rs +++ b/src/cmd/src/frontend.rs @@ -330,8 +330,6 @@ impl StartCommand { } async fn build(&self, opts: FrontendOptions) -> Result { - common_runtime::init_global_runtimes(&opts.runtime); - let guard = common_telemetry::init_global_logging( APP_NAME, &opts.component.logging, @@ -340,6 +338,8 @@ impl StartCommand { Some(&opts.component.slow_query), ); + common_runtime::init_global_runtimes(&opts.runtime); + crate::options::flush_dropped_plugin_warnings(); log_versions(verbose_version(), short_version(), APP_NAME); maybe_activate_heap_profile(&opts.component.memory); diff --git a/src/cmd/src/metasrv.rs b/src/cmd/src/metasrv.rs index 791efeb6e86..369f5c428d1 100644 --- a/src/cmd/src/metasrv.rs +++ b/src/cmd/src/metasrv.rs @@ -318,8 +318,6 @@ impl StartCommand { } pub async fn build(&self, opts: MetasrvOptions) -> Result { - common_runtime::init_global_runtimes(&opts.runtime); - let guard = common_telemetry::init_global_logging( APP_NAME, &opts.component.logging, @@ -328,6 +326,8 @@ impl StartCommand { None, ); + common_runtime::init_global_runtimes(&opts.runtime); + crate::options::flush_dropped_plugin_warnings(); log_versions(verbose_version(), short_version(), APP_NAME); maybe_activate_heap_profile(&opts.component.memory); diff --git a/src/cmd/src/standalone.rs b/src/cmd/src/standalone.rs index 9f8155b9cc2..2ffcc5e729c 100644 --- a/src/cmd/src/standalone.rs +++ b/src/cmd/src/standalone.rs @@ -395,8 +395,6 @@ impl StartCommand { #[allow(clippy::diverging_sub_expression)] /// Build GreptimeDB instance with the loaded options. pub async fn build(&self, opts: GreptimeOptions) -> Result { - common_runtime::init_global_runtimes(&opts.runtime); - let guard = common_telemetry::init_global_logging( APP_NAME, &opts.component.logging, @@ -405,6 +403,8 @@ impl StartCommand { Some(&opts.component.slow_query), ); + common_runtime::init_global_runtimes(&opts.runtime); + crate::options::flush_dropped_plugin_warnings(); log_versions(verbose_version(), short_version(), APP_NAME); maybe_activate_heap_profile(&opts.component.memory); diff --git a/src/common/runtime/src/global.rs b/src/common/runtime/src/global.rs index e28eaf61d15..827832fca22 100644 --- a/src/common/runtime/src/global.rs +++ b/src/common/runtime/src/global.rs @@ -27,6 +27,9 @@ use crate::{Builder, JoinHandle, Runtime}; const GLOBAL_WORKERS: usize = 8; const COMPACT_WORKERS: usize = 4; const HB_WORKERS: usize = 2; +/// The minimum number of worker threads for runtimes sized by CPU count. +/// A single-threaded runtime can easily deadlock in async code. +const MIN_RUNTIME_THREADS: usize = 2; /// The options for the global runtimes. #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] @@ -44,19 +47,25 @@ pub struct RuntimeOptions { pub ingest_rt_size: usize, } -impl Default for RuntimeOptions { - fn default() -> Self { - let cpus = num_cpus::get(); +impl RuntimeOptions { + fn with_num_cpus(cpus: usize) -> Self { + let cpus = usize::max(cpus, MIN_RUNTIME_THREADS); Self { global_rt_size: cpus, - compact_rt_size: usize::max(cpus / 2, 1), - compact_rt_max_blocking_threads: usize::max(cpus / 2, 1), - query_rt_size: usize::max(cpus.saturating_sub(1), 1), + compact_rt_size: usize::max(cpus / 2, MIN_RUNTIME_THREADS), + compact_rt_max_blocking_threads: usize::max(cpus / 2, MIN_RUNTIME_THREADS), + query_rt_size: usize::max(cpus.saturating_sub(1), MIN_RUNTIME_THREADS), ingest_rt_size: cpus, } } } +impl Default for RuntimeOptions { + fn default() -> Self { + Self::with_num_cpus(num_cpus::get()) + } +} + pub fn create_runtime(runtime_name: &str, thread_name: &str, worker_threads: usize) -> Runtime { info!( "Creating runtime with runtime_name: {runtime_name}, thread_name: {thread_name}, work_threads: {worker_threads}." @@ -287,18 +296,56 @@ mod tests { #[test] fn test_datanode_runtime_options_default() { let options = RuntimeOptions::default(); - let cpus = num_cpus::get(); + let cpus = usize::max(num_cpus::get(), MIN_RUNTIME_THREADS); assert_eq!(cpus, options.global_rt_size); - assert_eq!(usize::max(cpus / 2, 1), options.compact_rt_size); assert_eq!( - usize::max(cpus / 2, 1), + usize::max(cpus / 2, MIN_RUNTIME_THREADS), + options.compact_rt_size + ); + assert_eq!( + usize::max(cpus / 2, MIN_RUNTIME_THREADS), options.compact_rt_max_blocking_threads ); - assert_eq!(usize::max(cpus.saturating_sub(1), 1), options.query_rt_size); + assert_eq!( + usize::max(cpus.saturating_sub(1), MIN_RUNTIME_THREADS), + options.query_rt_size + ); assert_eq!(cpus, options.ingest_rt_size); } + #[test] + fn test_runtime_options_min_threads() { + for cpus in [0, 1, 2] { + let options = RuntimeOptions::with_num_cpus(cpus); + assert!( + options.global_rt_size >= MIN_RUNTIME_THREADS, + "global_rt_size {} < {MIN_RUNTIME_THREADS} with {cpus} cpus", + options.global_rt_size + ); + assert!( + options.compact_rt_size >= MIN_RUNTIME_THREADS, + "compact_rt_size {} < {MIN_RUNTIME_THREADS} with {cpus} cpus", + options.compact_rt_size + ); + assert!( + options.compact_rt_max_blocking_threads >= MIN_RUNTIME_THREADS, + "compact_rt_max_blocking_threads {} < {MIN_RUNTIME_THREADS} with {cpus} cpus", + options.compact_rt_max_blocking_threads + ); + assert!( + options.query_rt_size >= MIN_RUNTIME_THREADS, + "query_rt_size {} < {MIN_RUNTIME_THREADS} with {cpus} cpus", + options.query_rt_size + ); + assert!( + options.ingest_rt_size >= MIN_RUNTIME_THREADS, + "ingest_rt_size {} < {MIN_RUNTIME_THREADS} with {cpus} cpus", + options.ingest_rt_size + ); + } + } + #[test] fn test_datanode_runtimes_fallback_to_global_runtime() { let runtimes = GlobalRuntimes::new( From 8fac712870d5f95aef6ccdec8e9ae45f872f4afb Mon Sep 17 00:00:00 2001 From: Weny Xu Date: Wed, 19 Aug 2026 12:43:42 +0000 Subject: [PATCH 07/10] fix(meta): avoid blocking runtime on stats cache lock (#8910) Signed-off-by: WenyXu --- .../src/handler/collect_stats_handler.rs | 293 +++++++++++++++++- 1 file changed, 289 insertions(+), 4 deletions(-) diff --git a/src/meta-srv/src/handler/collect_stats_handler.rs b/src/meta-srv/src/handler/collect_stats_handler.rs index d444498798f..e3ae7ebe915 100644 --- a/src/meta-srv/src/handler/collect_stats_handler.rs +++ b/src/meta-srv/src/handler/collect_stats_handler.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::cmp::Ordering; +use std::sync::Arc; use api::v1::meta::{HeartbeatRequest, Role}; use common_meta::datanode::{DatanodeStatKey, DatanodeStatValue, Stat}; @@ -24,6 +25,7 @@ use common_meta::rpc::store::PutRequest; use common_telemetry::{error, info, warn}; use dashmap::DashMap; use snafu::ResultExt; +use tokio::sync::Mutex; use crate::error::{self, Result}; use crate::handler::{HandleControl, HeartbeatAccumulator, HeartbeatHandler}; @@ -70,7 +72,7 @@ impl EpochStats { const DEFAULT_FLUSH_STATS_FACTOR: usize = 3; pub struct CollectStatsHandler { - stats_cache: DashMap, + stats_cache: DashMap>>, flush_stats_factor: usize, } @@ -106,10 +108,16 @@ impl HeartbeatHandler for CollectStatsHandler { }; let key = current_stat.stat_key(); - let mut entry = self.stats_cache.entry(key).or_default(); + let state = { + let entry = self + .stats_cache + .entry(key) + .or_insert_with(|| Arc::new(Mutex::new(EpochStats::default()))); + Arc::clone(entry.value()) + }; + let mut epoch_stats = state.lock().await; let key: Vec = key.into(); - let epoch_stats = entry.value_mut(); let refresh = if let Some(epoch) = epoch_stats.epoch() { match current_stat.node_epoch.cmp(&epoch) { @@ -170,7 +178,7 @@ impl HeartbeatHandler for CollectStatsHandler { } } -async fn rewrite_node_address(ctx: &mut Context, stat: &Stat) { +async fn rewrite_node_address(ctx: &Context, stat: &Stat) { let peer = Peer { id: stat.id, addr: stat.addr.clone(), @@ -217,10 +225,188 @@ async fn rewrite_node_address(ctx: &mut Context, stat: &Stat) { #[cfg(test)] mod tests { + use std::any::Any; + use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering as AtomicOrdering}; + use std::sync::{Arc, Mutex as StdMutex, mpsc}; + use std::thread; + use std::time::Duration; + use common_meta::datanode::DatanodeStatKey; + use common_meta::error::{Error as MetaError, Result as MetaResult}; + use common_meta::kv_backend::{KvBackend, KvBackendRef, ResettableKvBackend, TxnService}; + use common_meta::rpc::store::{ + BatchDeleteRequest, BatchDeleteResponse, BatchGetRequest, BatchGetResponse, + BatchPutRequest, BatchPutResponse, DeleteRangeRequest, DeleteRangeResponse, PutResponse, + RangeRequest, RangeResponse, + }; + use tokio::sync::Semaphore; + use tokio::time::{sleep, timeout}; use super::*; use crate::handler::test_utils::TestEnv; + use crate::service::store::cached_kv::LeaderCachedKvBackend; + + struct ControlledKvBackend { + recorded_puts: StdMutex>, + put_entered: Semaphore, + put_release: Semaphore, + block_next_put: AtomicBool, + delay_puts: AtomicBool, + active_puts: AtomicUsize, + max_active_puts: AtomicUsize, + } + + impl ControlledKvBackend { + fn new() -> Self { + Self { + recorded_puts: StdMutex::new(Vec::new()), + put_entered: Semaphore::new(0), + put_release: Semaphore::new(0), + block_next_put: AtomicBool::new(false), + delay_puts: AtomicBool::new(false), + active_puts: AtomicUsize::new(0), + max_active_puts: AtomicUsize::new(0), + } + } + + fn block_next_put(&self) { + self.block_next_put.store(true, AtomicOrdering::Relaxed); + } + + async fn wait_for_blocked_put(&self) { + self.put_entered.acquire().await.unwrap().forget(); + } + + fn release_one_put(&self) { + self.put_release.add_permits(1); + } + + fn set_delay_puts(&self, delay: bool) { + self.delay_puts.store(delay, AtomicOrdering::Relaxed); + } + + fn clear_recorded_puts(&self) { + self.recorded_puts.lock().unwrap().clear(); + } + + fn recorded_puts(&self) -> Vec { + self.recorded_puts.lock().unwrap().clone() + } + + fn max_active_puts(&self) -> usize { + self.max_active_puts.load(AtomicOrdering::Relaxed) + } + + fn start_put(&self, req: &PutRequest) -> PutGuard<'_> { + self.recorded_puts.lock().unwrap().push(req.clone()); + let active = self.active_puts.fetch_add(1, AtomicOrdering::Relaxed) + 1; + self.max_active_puts + .fetch_max(active, AtomicOrdering::Relaxed); + PutGuard { backend: self } + } + } + + struct PutGuard<'a> { + backend: &'a ControlledKvBackend, + } + + impl Drop for PutGuard<'_> { + fn drop(&mut self) { + self.backend + .active_puts + .fetch_sub(1, AtomicOrdering::Relaxed); + } + } + + #[async_trait::async_trait] + impl TxnService for ControlledKvBackend { + type Error = MetaError; + } + + #[async_trait::async_trait] + impl KvBackend for ControlledKvBackend { + fn name(&self) -> &str { + "controlled" + } + + fn as_any(&self) -> &dyn Any { + self + } + + async fn range(&self, _req: RangeRequest) -> MetaResult { + unimplemented!() + } + + async fn put(&self, req: PutRequest) -> MetaResult { + let _guard = self.start_put(&req); + if self.block_next_put.swap(false, AtomicOrdering::Relaxed) { + self.put_entered.add_permits(1); + self.put_release.acquire().await.unwrap().forget(); + } + if self.delay_puts.load(AtomicOrdering::Relaxed) { + sleep(Duration::from_millis(10)).await; + } + Ok(PutResponse::default()) + } + + async fn batch_put(&self, _req: BatchPutRequest) -> MetaResult { + unimplemented!() + } + + async fn batch_get(&self, _req: BatchGetRequest) -> MetaResult { + unimplemented!() + } + + async fn delete_range(&self, _req: DeleteRangeRequest) -> MetaResult { + unimplemented!() + } + + async fn batch_delete(&self, _req: BatchDeleteRequest) -> MetaResult { + unimplemented!() + } + } + + impl ResettableKvBackend for ControlledKvBackend { + fn reset(&self) { + self.clear_recorded_puts(); + } + + fn as_kv_backend_ref(self: Arc) -> KvBackendRef { + self + } + } + + fn stat(node_id: u64, epoch: u64, marker: u64, addr: &str) -> Stat { + Stat { + timestamp_millis: marker as i64, + id: node_id, + addr: addr.to_string(), + region_num: marker, + node_epoch: epoch, + ..Default::default() + } + } + + async fn handle_stat( + handler: Arc, + mut ctx: Context, + stat: Stat, + ) -> Result { + let mut acc = HeartbeatAccumulator { + stat: Some(stat), + ..Default::default() + }; + handler + .handle(&HeartbeatRequest::default(), &mut ctx, &mut acc) + .await + } + + fn use_controlled_address_backend(ctx: &mut Context) -> Arc { + let backend = Arc::new(ControlledKvBackend::new()); + ctx.leader_cached_kv_backend = + Arc::new(LeaderCachedKvBackend::with_always_leader(backend.clone())); + backend + } #[tokio::test] async fn test_handle_datanode_stats() { @@ -250,6 +436,105 @@ mod tests { assert_eq!(handler.flush_stats_factor, val.stats.len()); } + #[test] + fn test_same_datanode_wait_keeps_current_thread_runtime_responsive() { + let (backend_tx, backend_rx) = mpsc::sync_channel(1); + let (timer_tx, timer_rx) = mpsc::sync_channel(1); + let (done_tx, done_rx) = mpsc::sync_channel(1); + + let worker = thread::spawn(move || { + tokio::runtime::Builder::new_current_thread() + .enable_time() + .build() + .unwrap() + .block_on(async move { + let env = TestEnv::new(); + let mut ctx = env.ctx(); + let address_backend = use_controlled_address_backend(&mut ctx); + address_backend.block_next_put(); + backend_tx.send(address_backend.clone()).unwrap(); + + let handler = Arc::new(CollectStatsHandler::default()); + let first = tokio::spawn(handle_stat( + handler.clone(), + ctx.clone(), + stat(101, 1, 1, "dn-101-v1"), + )); + address_backend.wait_for_blocked_put().await; + + let second = + tokio::spawn(handle_stat(handler, ctx, stat(101, 1, 2, "dn-101-v1"))); + tokio::spawn(async move { + sleep(Duration::from_millis(10)).await; + timer_tx.send(()).unwrap(); + }); + tokio::task::yield_now().await; + + first.await.unwrap().unwrap(); + second.await.unwrap().unwrap(); + done_tx.send(()).unwrap(); + }); + }); + + let address_backend = backend_rx.recv_timeout(Duration::from_secs(1)).unwrap(); + let timer_result = timer_rx.recv_timeout(Duration::from_secs(1)); + address_backend.release_one_put(); + timer_result.expect("the current-thread runtime must remain responsive"); + done_rx + .recv_timeout(Duration::from_secs(1)) + .expect("both heartbeat handlers must complete after releasing the write"); + worker.join().unwrap(); + } + + #[tokio::test] + async fn test_concurrent_flush_persists_every_stat_once() { + let env = TestEnv::new(); + let mut ctx = env.ctx(); + let stats_backend = Arc::new(ControlledKvBackend::new()); + ctx.in_memory = stats_backend.clone(); + stats_backend.set_delay_puts(true); + + let flush_stats_factor = 3; + let handler = Arc::new(CollectStatsHandler::new(Some(flush_stats_factor))); + handle_stat(handler.clone(), ctx.clone(), stat(101, 1, 0, "dn-101")) + .await + .unwrap(); + stats_backend.clear_recorded_puts(); + + let mut tasks = Vec::with_capacity(2 * flush_stats_factor); + for marker in 1..=(2 * flush_stats_factor) { + tasks.push(tokio::spawn(handle_stat( + handler.clone(), + ctx.clone(), + stat(101, 1, marker as u64, "dn-101"), + ))); + } + for task in tasks { + timeout(Duration::from_secs(1), task) + .await + .unwrap() + .unwrap() + .unwrap(); + } + + let puts = stats_backend.recorded_puts(); + assert_eq!(2, puts.len()); + let mut markers = puts + .into_iter() + .flat_map(|put| { + let value: DatanodeStatValue = put.value.try_into().unwrap(); + value + .stats + .into_iter() + .map(|stat| stat.region_num) + .collect::>() + }) + .collect::>(); + markers.sort_unstable(); + assert_eq!((1..=6).collect::>(), markers); + assert_eq!(1, stats_backend.max_active_puts()); + } + async fn handle_request_many_times( mut ctx: Context, handler: &CollectStatsHandler, From 76f08d2b3f9f5905c9169244dbed8b8bd2178973 Mon Sep 17 00:00:00 2001 From: LFC <990479+MichaelScofield@users.noreply.github.com> Date: Fri, 21 Aug 2026 03:28:39 +0000 Subject: [PATCH 08/10] refactor(json2): add bounded auto-expansion to the JSON2 vector builder (#8909) * refactor(json2): add bounded auto-expansion to the JSON2 vector builder Signed-off-by: luofucong * resolve PR comments Signed-off-by: luofucong * fix ci Signed-off-by: luofucong --------- Signed-off-by: luofucong --- src/datatypes/src/vectors/json/builder.rs | 841 ++++++++++++++++++++-- src/datatypes/src/vectors/json/variant.rs | 11 +- 2 files changed, 774 insertions(+), 78 deletions(-) diff --git a/src/datatypes/src/vectors/json/builder.rs b/src/datatypes/src/vectors/json/builder.rs index e6ceb6d6a5f..125dbdad58a 100644 --- a/src/datatypes/src/vectors/json/builder.rs +++ b/src/datatypes/src/vectors/json/builder.rs @@ -13,72 +13,488 @@ // limitations under the License. use std::any::Any; +use std::collections::{BTreeMap, HashMap}; use std::sync::Arc; +use arrow_array::cast::AsArray; +use arrow_array::{Array, ArrayRef, StructArray}; use arrow_schema::DataType; +use parquet_variant_compute::VariantArrayBuilder; +use snafu::{ResultExt, ensure}; use crate::data_type::ConcreteDataType; -use crate::error::{Result, TryFromValueSnafu, UnexpectedSnafu, UnsupportedOperationSnafu}; +use crate::error::{ + ArrowComputeSnafu, Result, TryFromValueSnafu, UnexpectedSnafu, UnsupportedOperationSnafu, +}; +use crate::extension::json::JSON2_REMAINDER_FIELD_NAME; use crate::json::value::{JsonNumber, JsonVariant, encode_json_variant}; +use crate::json::{JSON2_MAX_STRUCTURED_DEPTH, JsonSettings}; use crate::prelude::{ValueRef, Vector, VectorRef}; use crate::types::StructType; use crate::types::json_type::{JsonNativeType, is_include}; use crate::value::{ListValue, StructValue, StructValueRef, Value}; -use crate::vectors::{MutableVector, StructVectorBuilder}; +use crate::vectors::json::variant::{append_json_variant, variant_field}; +use crate::vectors::{Helper, MutableVector, StructVectorBuilder}; +type JsonObjectValue = BTreeMap; + +/// Builds JSON2 vectors from object values. +/// +/// Legacy mode merges all observed paths into the explicit Struct schema. +/// Auto-expanding mode always materializes type-hinted paths, selects up to +/// `max_auto_expanded_paths` compatible unhinted leaf paths by frequency, and stores +/// conflicting or unselected paths in the Variant remainder field. #[derive(Clone)] pub(crate) struct JsonVectorBuilder { - merged_type: JsonNativeType, - values: Vec, + state: JsonVectorBuilderState, +} + +#[derive(Clone)] +enum JsonVectorBuilderState { + Legacy { + merged_type: JsonNativeType, + values: Vec, + }, + AutoExpanding { + /// Paths declared by type hints and always stored as dedicated Struct fields. + explicit_type: JsonNativeType, + /// Maximum number of additional paths selected from buffered values. + max_auto_expanded_paths: u32, + /// Buffered values used to infer auto-expanded paths before building the vector. + values: Vec, + }, +} + +impl JsonVectorBuilderState { + fn native_type(&self) -> JsonNativeType { + match self { + Self::Legacy { merged_type, .. } => merged_type.clone(), + Self::AutoExpanding { + explicit_type, + max_auto_expanded_paths, + values, + } => infer_expanded_type(explicit_type, *max_auto_expanded_paths, values), + } + } + + fn len(&self) -> usize { + match self { + Self::Legacy { values, .. } | Self::AutoExpanding { values, .. } => values.len(), + } + } + + fn try_build(&mut self) -> Result { + match self { + Self::Legacy { + merged_type, + values, + } => build_legacy(values, merged_type), + Self::AutoExpanding { + explicit_type, + max_auto_expanded_paths, + values, + } => { + let expanded_type = + infer_expanded_type(explicit_type, *max_auto_expanded_paths, values); + build_with_remainder(values, &expanded_type) + } + } + } + + fn try_push_value_ref(&mut self, value: &ValueRef) -> Result<()> { + let ValueRef::Json(value) = value else { + return TryFromValueSnafu { + reason: format!("expected JSON value, got {value:?}"), + } + .fail(); + }; + ensure!( + value.is_object() || value.is_null(), + TryFromValueSnafu { + reason: format!("expected JSON object value, got {value:?}"), + } + ); + match self { + Self::Legacy { + merged_type, + values, + } => { + let json_type = value.json_type(); + if !is_include(merged_type, json_type.as_ref()) { + merged_type.merge(json_type.as_ref()); + } + values.push(JsonVariant::from(value.variant())); + } + Self::AutoExpanding { values, .. } => { + values.push(JsonVariant::from(value.variant())); + } + } + Ok(()) + } + + fn push_null(&mut self) { + match self { + Self::Legacy { values, .. } | Self::AutoExpanding { values, .. } => { + values.push(JsonVariant::Null) + } + } + } } impl JsonVectorBuilder { + /// Creates a builder that merges all observed paths into the explicit schema. pub(crate) fn new(initial_native_type: JsonNativeType, capacity: usize) -> Self { debug_assert!(matches!( initial_native_type, JsonNativeType::Object(_) | JsonNativeType::Null )); Self { - merged_type: initial_native_type, - values: Vec::with_capacity(capacity), + state: JsonVectorBuilderState::Legacy { + merged_type: initial_native_type, + values: Vec::with_capacity(capacity), + }, + } + } + + /// Creates a builder bounded by the JSON settings and their type hints. + #[cfg_attr(not(test), expect(dead_code))] + pub(crate) fn with_settings(settings: &JsonSettings, capacity: usize) -> Self { + let mut explicit_type = JsonNativeType::Object(Default::default()); + for hint in settings.type_hints() { + insert_dynamic_type(&mut explicit_type, &hint.path, (&hint.data_type).into()); + } + Self { + state: JsonVectorBuilderState::AutoExpanding { + explicit_type, + max_auto_expanded_paths: settings.max_auto_expanded_paths().unwrap_or(u32::MAX), + values: Vec::with_capacity(capacity), + }, } } fn try_build(&mut self) -> Result { - let DataType::Struct(fields) = self.merged_type.as_arrow_type() else { - return UnexpectedSnafu { - reason: "merged JSON2 type must map to Arrow Struct in JsonVectorBuilder", - } - .fail(); - }; - // TODO(LFC): Direct use Arrow's Struct datatype here. - let struct_type = StructType::from(&fields); - - let mut builder = - StructVectorBuilder::with_type_and_capacity(struct_type.clone(), self.values.len()); - for value in std::mem::take(&mut self.values) { - if matches!(&value, JsonVariant::Null) { - builder.push_null(); - continue; - } - let value = json_variant_into_struct_value(value, struct_type.clone())?; - builder.push_struct_value_ref(StructValueRef::Ref(&value))?; - } - Ok(builder.to_vector()) + self.state.try_build() } } -fn json_variant_into_struct_value( - value: JsonVariant, - struct_type: StructType, -) -> Result { - let JsonVariant::Object(object) = value else { - return TryFromValueSnafu { - reason: format!("expected json object value, got {value:?}"), +fn build_legacy(values: &mut Vec, merged_type: &JsonNativeType) -> Result { + build_explicit(values, merged_type, false, |value| match value { + JsonVariant::Null => Ok(None), + JsonVariant::Object(value) => Ok(Some(value)), + _ => TryFromValueSnafu { + reason: "expected json object value".to_string(), + } + .fail(), + }) +} + +fn build_with_remainder( + values: &mut Vec, + expanded_type: &JsonNativeType, +) -> Result { + let mut remainder = VariantArrayBuilder::new(values.len()); + let explicit = build_explicit(values, expanded_type, true, |value| { + if matches!(value, JsonVariant::Null) { + remainder.append_null(); + return Ok(None); + } + let (value, rest) = split_to_explicit(value, expanded_type)?; + append_json_variant(&mut remainder, &JsonVariant::Object(rest)) + .context(ArrowComputeSnafu)?; + Ok(Some(value)) + })?; + finish_vector(explicit, ArrayRef::from(remainder.build())) +} + +fn build_explicit( + values: &mut Vec, + explicit_type: &JsonNativeType, + // Temporary compatibility switch for the legacy storage layout. Once JSON2 fully switches to + // the v2 storage layout, empty objects should always be preserved instead of treated as null. + preserve_empty_structs: bool, + mut project: impl FnMut(JsonVariant) -> Result>, +) -> Result { + let DataType::Struct(fields) = explicit_type.as_arrow_type() else { + return UnexpectedSnafu { + reason: "merged JSON2 type must map to Arrow Struct in JsonVectorBuilder", } .fail(); }; + // TODO(LFC): Direct use Arrow's Struct datatype here. + let struct_type = StructType::from(&fields); + let mut builder = + StructVectorBuilder::with_type_and_capacity(struct_type.clone(), values.len()); + for value in std::mem::take(values) { + let Some(value) = project(value)? else { + builder.push_null(); + continue; + }; + let value = + json_variant_into_struct_value(value, struct_type.clone(), preserve_empty_structs)?; + builder.push_struct_value_ref(StructValueRef::Ref(&value))?; + } + Ok(builder.to_vector()) +} + +fn finish_vector(explicit: VectorRef, remainder: ArrayRef) -> Result { + let explicit = explicit.to_arrow_array(); + let explicit = explicit.as_struct(); + let mut children = explicit + .fields() + .iter() + .cloned() + .zip(explicit.columns().iter().cloned()) + .chain(std::iter::once(( + Arc::new(variant_field(JSON2_REMAINDER_FIELD_NAME, true)), + remainder, + ))) + .collect::>(); + children.sort_unstable_by(|(x, _), (y, _)| x.name().cmp(y.name())); + let (fields, columns): (Vec<_>, Vec) = children.into_iter().unzip(); + let array: ArrayRef = Arc::new(StructArray::new( + fields.into(), + columns, + explicit.nulls().cloned(), + )); + Helper::try_into_vector(array) +} + +fn infer_expanded_type( + explicit_type: &JsonNativeType, + max_auto_expanded_paths: u32, + values: &[JsonVariant], +) -> JsonNativeType { + if max_auto_expanded_paths == 0 { + return explicit_type.clone(); + } + + let mut stats = HashMap::new(); + let mut path = Vec::new(); + init_explicit_path_stats(explicit_type, &mut path, &mut stats); + for value in values { + count_dynamic_paths(value, &mut path, &mut stats); + } + let mut candidates = stats + .iter() + // Explicit paths are already in the output schema and do not consume the dynamic + // expansion budget. + .filter(|(_, stat)| !stat.is_explicit && stat.is_leaf) + // A leaf is eligible only when both itself and every object prefix have one stable + // role and type across all observed values. + .filter(|(path, _)| { + !(1..=path.len()) + .any(|len| stats.get(&path[..len]).is_some_and(|stats| stats.conflicts)) + }) + .collect::>(); + candidates.sort_unstable_by(|(x_path, x), (y_path, y)| { + y.seen_count + .cmp(&x.seen_count) + .then_with(|| x_path.cmp(y_path)) + }); + + let mut expanded_type = explicit_type.clone(); + for (path, candidate) in candidates + .into_iter() + .take(max_auto_expanded_paths as usize) + { + insert_dynamic_type( + &mut expanded_type, + path, + candidate.expected_leaf_type.clone(), + ); + } + expanded_type +} + +/// Aggregated observations for one JSON path. +/// +/// Schema inference first seeds the map with explicit paths, then walks all input values once. +/// Objects, including empty objects, are non-leaf paths; every other non-null value is a leaf. +/// The first dynamic observation fixes the path role and exact leaf type. A later role or type +/// mismatch sets [`PathStats::conflicts`] permanently. Missing paths and null values do not affect +/// the statistics. +/// +/// After collection, dynamic leaves are ranked by [`PathStats::seen_count`]. A candidate is +/// rejected when it or any parent path conflicts, so candidate selection never rescans the input +/// values. +struct PathStats { + /// Whether the path came from a type hint and is already part of the output schema. + is_explicit: bool, + /// Whether the path is a non-object value rather than an object prefix. + is_leaf: bool, + /// Exact type required for a leaf; unused non-leaf paths keep [`JsonNativeType::Null`]. + expected_leaf_type: JsonNativeType, + /// Number of compatible observations used to rank dynamic leaves. + seen_count: usize, + /// Whether the path has ever had inconsistent roles or leaf types. + conflicts: bool, +} + +/// Seeds path statistics from the configured explicit JSON shape. +fn init_explicit_path_stats<'a>( + explicit_type: &'a JsonNativeType, + path: &mut Vec<&'a str>, + stats: &mut HashMap, PathStats>, +) { + let JsonNativeType::Object(fields) = explicit_type else { + return; + }; + for (name, data_type) in fields { + path.push(name); + let is_leaf = !matches!(data_type, JsonNativeType::Object(_)); + let expected_leaf_type = if is_leaf { + data_type.clone() + } else { + JsonNativeType::default() + }; + stats.insert( + path.clone(), + PathStats { + is_explicit: true, + is_leaf, + expected_leaf_type, + seen_count: 0, + conflicts: false, + }, + ); + init_explicit_path_stats(data_type, path, stats); + path.pop(); + } +} + +/// Collects dynamic path statistics while traversing each input value once. +fn count_dynamic_paths<'a>( + value: &'a JsonVariant, + path: &mut Vec<&'a str>, + stats: &mut HashMap, PathStats>, +) { + if matches!(value, JsonVariant::Null) || path.len() > JSON2_MAX_STRUCTURED_DEPTH { + return; + } + + if !path.is_empty() { + let is_leaf = !matches!(value, JsonVariant::Object(_)); + let conflicts = if let Some(stats) = stats.get_mut(path.as_slice()) { + if !stats.conflicts { + let role_conflict = stats.is_leaf != is_leaf; + let type_conflict = || match (&stats.expected_leaf_type, value) { + // If both objects, they are compatible. + (JsonNativeType::Null | JsonNativeType::Object(_), JsonVariant::Object(_)) => { + false + } + _ => stats.expected_leaf_type != value.native_type(), + }; + if role_conflict || type_conflict() { + stats.conflicts = true; + } else { + stats.seen_count += 1; + } + } + stats.conflicts + } else { + let expected_leaf_type = if is_leaf { + value.native_type() + } else { + JsonNativeType::default() + }; + stats.insert( + path.clone(), + PathStats { + is_explicit: false, + is_leaf, + expected_leaf_type, + seen_count: 1, + conflicts: false, + }, + ); + false + }; + if conflicts { + return; + } + } + + if let JsonVariant::Object(object) = value + && !object.is_empty() + { + for (name, value) in object { + path.push(name); + count_dynamic_paths(value, path, stats); + path.pop(); + } + } +} + +fn insert_dynamic_type>( + explicit_type: &mut JsonNativeType, + path: &[S], + data_type: JsonNativeType, +) { + let JsonNativeType::Object(fields) = explicit_type else { + return; + }; + let Some((name, path)) = path.split_first() else { + return; + }; + let name = name.as_ref().to_string(); + if path.is_empty() { + fields.insert(name, data_type); + return; + } + insert_dynamic_type( + fields + .entry(name) + .or_insert_with(|| JsonNativeType::Object(Default::default())), + path, + data_type, + ) +} + +fn split_to_explicit( + value: JsonVariant, + explicit_type: &JsonNativeType, +) -> Result<(JsonObjectValue, JsonObjectValue)> { + let JsonVariant::Object(mut remainder) = value else { + return TryFromValueSnafu { + reason: "expected json object value".to_string(), + } + .fail(); + }; + let JsonNativeType::Object(fields) = explicit_type else { + return UnexpectedSnafu { + reason: "JSON2 explicit type must be an object", + } + .fail(); + }; + let mut explicit = JsonObjectValue::new(); + + for (name, data_type) in fields { + let Some(value) = remainder.remove(name) else { + continue; + }; + if matches!(data_type, JsonNativeType::Object(_)) { + if matches!(value, JsonVariant::Null) { + explicit.insert(name.clone(), value); + continue; + } + let (child_explicit, child_remainder) = split_to_explicit(value, data_type)?; + explicit.insert(name.clone(), JsonVariant::Object(child_explicit)); + if !child_remainder.is_empty() { + remainder.insert(name.clone(), JsonVariant::Object(child_remainder)); + } + } else { + explicit.insert(name.clone(), value); + } + } + Ok((explicit, remainder)) +} + +fn json_variant_into_struct_value( + object: JsonObjectValue, + struct_type: StructType, + preserve_empty_structs: bool, +) -> Result { let mut entries = object.into_iter(); let mut entry = entries.next(); let mut values = Vec::with_capacity(struct_type.fields().len()); @@ -86,7 +502,7 @@ fn json_variant_into_struct_value( let value = match entry.take() { Some((name, value)) if name == field.name() => { entry = entries.next(); - json_variant_into_value(value, field.data_type())? + json_variant_into_value(value, field.data_type(), preserve_empty_structs)? } Some((name, _)) if name.as_str() < field.name() => { return TryFromValueSnafu { @@ -111,10 +527,19 @@ fn json_variant_into_struct_value( Ok(StructValue::new(values, struct_type)) } -fn json_variant_into_value(value: JsonVariant, expected_type: &ConcreteDataType) -> Result { +fn json_variant_into_value( + value: JsonVariant, + expected_type: &ConcreteDataType, + preserve_empty_structs: bool, +) -> Result { let value = match (value, expected_type) { (JsonVariant::Null, _) | (_, ConcreteDataType::Null(_)) => Value::Null, - (JsonVariant::Object(object), _) if object.is_empty() => Value::Null, + (JsonVariant::Object(object), _) if object.is_empty() && !preserve_empty_structs => { + Value::Null + } + (JsonVariant::Object(object), ConcreteDataType::Struct(struct_type)) => Value::Struct( + json_variant_into_struct_value(object, struct_type.clone(), preserve_empty_structs)?, + ), (JsonVariant::Bool(x), ConcreteDataType::Boolean(_)) => Value::Boolean(x), (JsonVariant::Number(x), ConcreteDataType::UInt64(_)) => { let Some(x) = x.as_u64() else { @@ -153,13 +578,10 @@ fn json_variant_into_value(value: JsonVariant, expected_type: &ConcreteDataType) let item_type = list_type.item_type().clone(); let values = array .into_iter() - .map(|v| json_variant_into_value(v, &item_type)) + .map(|v| json_variant_into_value(v, &item_type, preserve_empty_structs)) .collect::>>()?; Value::List(ListValue::new(values, Arc::new(item_type))) } - (value @ JsonVariant::Object(_), ConcreteDataType::Struct(struct_type)) => { - Value::Struct(json_variant_into_struct_value(value, struct_type.clone())?) - } (value, ConcreteDataType::Binary(_)) => Value::from(encode_json_variant(value)?), (value, expected_type) => { return TryFromValueSnafu { @@ -173,11 +595,11 @@ fn json_variant_into_value(value: JsonVariant, expected_type: &ConcreteDataType) impl MutableVector for JsonVectorBuilder { fn data_type(&self) -> ConcreteDataType { - ConcreteDataType::json2(self.merged_type.clone()) + ConcreteDataType::json2(self.state.native_type()) } fn len(&self) -> usize { - self.values.len() + self.state.len() } fn as_any(&self) -> &dyn Any { @@ -197,30 +619,11 @@ impl MutableVector for JsonVectorBuilder { } fn try_push_value_ref(&mut self, value: &ValueRef) -> Result<()> { - let ValueRef::Json(value) = value else { - return TryFromValueSnafu { - reason: format!("expected json value, got {value:?}"), - } - .fail(); - }; - let json_type = value.json_type(); - let json_type = json_type.as_ref(); - if !matches!(json_type, JsonNativeType::Object(_) | JsonNativeType::Null) { - return TryFromValueSnafu { - reason: format!("expected json object value, got {value:?}"), - } - .fail(); - } - if !is_include(&self.merged_type, json_type) { - self.merged_type.merge(json_type); - } - - self.values.push(JsonVariant::from(value.variant())); - Ok(()) + self.state.try_push_value_ref(value) } fn push_null(&mut self) { - self.values.push(JsonVariant::Null) + self.state.push_null() } fn extend_slice_of(&mut self, _: &dyn Vector, _: usize, _: usize) -> Result<()> { @@ -236,13 +639,21 @@ impl MutableVector for JsonVectorBuilder { mod tests { use std::sync::Arc; + use arrow_array::cast::AsArray; + use arrow_schema::Field; use common_base::bytes::Bytes; + use serde_json::json; use super::*; use crate::data_type::ConcreteDataType; + use crate::extension::json::{Json2ExtensionType, JsonMetadata}; + use crate::json::JsonTypeHint; + use crate::json::value::decode_json_variant; use crate::types::StructField; use crate::types::json_type::JsonObjectType; use crate::value::{ListValue, StructValue, Value, ValueRef}; + use crate::vectors::json::array::JsonArray; + use crate::vectors::json::variant::variant_to_json_values; #[test] fn test_json_vector_builder() -> Result<()> { @@ -346,7 +757,7 @@ mod tests { let err = object_builder .try_push_value_ref(&boolean.as_value_ref()) .unwrap_err(); - assert!(err.to_string().contains("expected json object value")); + 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. @@ -355,20 +766,311 @@ mod tests { let err = invalid_builder .try_push_value_ref(&ValueRef::Boolean(true)) .unwrap_err(); - assert!(err.to_string().contains("expected json value")); + assert!(err.to_string().contains("expected JSON value")); + + Ok(()) + } + + #[test] + fn test_zero_budget_builder_uses_fixed_schema_and_remainder() -> Result<()> { + let settings = JsonSettings::try_new( + vec![ + JsonTypeHint { + path: vec!["kind".to_string()], + data_type: ConcreteDataType::string_datatype(), + nullable: true, + default_constraint: None, + inverted_index: false, + }, + JsonTypeHint { + path: vec!["commit".to_string(), "operation".to_string()], + data_type: ConcreteDataType::string_datatype(), + nullable: true, + default_constraint: None, + inverted_index: false, + }, + JsonTypeHint { + path: vec!["time_us".to_string()], + data_type: ConcreteDataType::int64_datatype(), + nullable: true, + default_constraint: None, + inverted_index: false, + }, + ], + Some(0), + )?; + let mut builder = JsonVectorBuilder::with_settings(&settings, 2); + let values = [ + json!({ + "kind": "record", + "commit": {"operation": "create", "collection": "post"}, + "extra": 1, + "time_us": 1 + }), + json!({"kind": "other", "dynamic": true, "time_us": 2}), + ]; + for value in values.clone() { + let value = settings.encode(value)?; + builder.try_push_value_ref(&value.as_value_ref())?; + } + let array = builder.to_vector().to_arrow_array(); + assert_eq!(0, builder.len()); + let structs = array.as_struct(); + assert_eq!( + vec![JSON2_REMAINDER_FIELD_NAME, "commit", "kind", "time_us"], + structs + .fields() + .iter() + .map(|x| x.name().as_str()) + .collect::>() + ); + assert_eq!( + vec![ + Some(json!({"commit": {"collection": "post"}, "extra": 1})), + Some(json!({"dynamic": true})), + ], + variant_to_json_values(structs.column_by_name(JSON2_REMAINDER_FIELD_NAME).unwrap())? + ); + + let field = Field::new("data", array.data_type().clone(), true).with_extension_type( + Json2ExtensionType::new(Arc::new(JsonMetadata::new_v2(settings))), + ); + let reconstructed = JsonArray::from(&array).project_to_v2(&field, &DataType::Binary)?; + let reconstructed = reconstructed.as_binary::(); + assert_eq!( + values[0], + decode_json_variant(reconstructed.value(0)).unwrap() + ); + assert_eq!( + json!({ + "kind": "other", + "commit": {"operation": null}, + "dynamic": true, + "time_us": 2 + }), + decode_json_variant(reconstructed.value(1)).unwrap() + ); + + let settings = JsonSettings::try_new(vec![], Some(0))?; + let mut builder = JsonVectorBuilder::with_settings(&settings, 3); + for value in [json!({}), json!({"x": 1})] { + let value = settings.encode(value)?; + builder.try_push_value_ref(&value.as_value_ref())?; + } + builder.push_null(); + let array = builder.to_vector().to_arrow_array(); + let structs = array.as_struct(); + assert_eq!(1, structs.num_columns()); + assert_eq!( + vec![Some(json!({})), Some(json!({"x": 1})), None], + variant_to_json_values(structs.column(0))? + ); + + Ok(()) + } + + #[test] + fn test_finite_budget_selects_dynamic_paths() -> Result<()> { + let settings = JsonSettings::try_new( + vec![JsonTypeHint { + path: vec!["hint".to_string()], + data_type: ConcreteDataType::string_datatype(), + nullable: true, + default_constraint: None, + inverted_index: false, + }], + Some(2), + )?; + let values = [ + json!({ + "hint": "first", + "conflict": 1, + "popular": {"nested": 1}, + "tie_a": "a", + "tie_b": true, + "rare": 1 + }), + json!({ + "hint": "second", + "conflict": "string", + "popular": {"nested": 2}, + "tie_a": "b", + "tie_b": false + }), + json!({"hint": "third", "popular": "scalar"}), + ]; + let mut builder = JsonVectorBuilder::with_settings(&settings, values.len()); + for value in values.clone() { + let value = settings.encode(value)?; + builder.try_push_value_ref(&value.as_value_ref())?; + } + + let array = builder.to_vector().to_arrow_array(); + let structs = array.as_struct(); + assert_eq!( + vec![JSON2_REMAINDER_FIELD_NAME, "hint", "tie_a", "tie_b"], + structs + .fields() + .iter() + .map(|x| x.name().as_str()) + .collect::>() + ); + assert!(structs.column_by_name("popular").is_none()); + assert_eq!( + vec![ + Some(json!({"conflict": 1, "popular": {"nested": 1}, "rare": 1})), + Some(json!({"conflict": "string", "popular": {"nested": 2}})), + Some(json!({"popular": "scalar"})), + ], + variant_to_json_values(structs.column_by_name(JSON2_REMAINDER_FIELD_NAME).unwrap())? + ); + + let field = Field::new("data", array.data_type().clone(), true).with_extension_type( + Json2ExtensionType::new(Arc::new(JsonMetadata::new_v2(settings))), + ); + let reconstructed = JsonArray::from(&array).project_to_v2(&field, &DataType::Binary)?; + let reconstructed = reconstructed.as_binary::(); + assert_eq!( + values[0], + decode_json_variant(reconstructed.value(0)).unwrap() + ); + assert_eq!( + values[1], + decode_json_variant(reconstructed.value(1)).unwrap() + ); + assert_eq!( + json!({ + "hint": "third", + "popular": "scalar", + "tie_a": null, + "tie_b": null + }), + decode_json_variant(reconstructed.value(2)).unwrap() + ); + + Ok(()) + } + + #[test] + fn test_reconstruct_nested_remainder_only_value() -> Result<()> { + let settings = JsonSettings::try_new(vec![], Some(1))?; + let values = [ + json!({"a": {"hot": 1}}), + json!({"a": {"hot": 2}}), + json!({"a": {"cold": 3}}), + ]; + let mut builder = JsonVectorBuilder::with_settings(&settings, values.len()); + for value in values.clone() { + let value = settings.encode(value)?; + builder.try_push_value_ref(&value.as_value_ref())?; + } + + let array = builder.to_vector().to_arrow_array(); + let field = Field::new("data", array.data_type().clone(), true).with_extension_type( + Json2ExtensionType::new(Arc::new(JsonMetadata::new_v2(settings))), + ); + let reconstructed = JsonArray::from(&array).project_to_v2(&field, &DataType::Binary)?; + let reconstructed = reconstructed.as_binary::(); + assert_eq!( + vec![ + values[0].clone(), + values[1].clone(), + json!({"a": {"cold": 3, "hot": null}}), + ], + (0..reconstructed.len()) + .map(|i| decode_json_variant(reconstructed.value(i)).unwrap()) + .collect::>() + ); + + Ok(()) + } + + #[test] + fn test_dynamic_paths_require_the_same_leaf_type() -> Result<()> { + let settings = JsonSettings::try_new( + vec![JsonTypeHint { + path: vec!["nested".to_string(), "hinted".to_string()], + data_type: ConcreteDataType::string_datatype(), + nullable: true, + default_constraint: None, + inverted_index: false, + }], + Some(8), + )?; + let values = [ + json!({ + "branch": {}, + "different": [1], + "empty": {}, + "nested": {}, + "reverse": "scalar", + "same": [1] + }), + json!({ + "branch": {"leaf": 1}, + "different": ["x"], + "empty": {}, + "nested": {"hinted": "x", "leaf": 1}, + "same": [2] + }), + json!({ + "branch": {}, + "different": [2], + "empty": {}, + "nested": {}, + "reverse": {"leaf": 1}, + "same": [3] + }), + ]; + let mut builder = JsonVectorBuilder::with_settings(&settings, values.len()); + for value in values { + let value = settings.encode(value)?; + builder.try_push_value_ref(&value.as_value_ref())?; + } + + let JsonNativeType::Object(fields) = builder.state.native_type() else { + unreachable!(); + }; + assert!(!fields.contains_key("different")); + assert!(!fields.contains_key("empty")); + assert!(!fields.contains_key("reverse")); + assert!(matches!(fields.get("same"), Some(JsonNativeType::Array(_)))); + assert!(matches!( + fields.get("branch"), + Some(JsonNativeType::Object(fields)) if fields.contains_key("leaf") + )); + assert!(matches!( + fields.get("nested"), + Some(JsonNativeType::Object(fields)) + if fields.contains_key("hinted") && fields.contains_key("leaf") + )); Ok(()) } #[test] fn test_json_variant_into_struct_value() -> Result<()> { + let struct_type = StructType::new(Arc::new(vec![StructField::new( + "value".to_string(), + ConcreteDataType::string_datatype(), + true, + )])); assert_eq!( json_variant_into_value( JsonVariant::Object(Default::default()), - &ConcreteDataType::string_datatype(), + &ConcreteDataType::struct_datatype(struct_type.clone()), + false, )?, Value::Null ); + assert_eq!( + json_variant_into_value( + JsonVariant::Object(Default::default()), + &ConcreteDataType::struct_datatype(struct_type.clone()), + true, + )?, + Value::Struct(StructValue::new(vec![Value::Null], struct_type)) + ); let item_type = ConcreteDataType::struct_datatype(StructType::new(Arc::new(vec![StructField::new( @@ -394,22 +1096,23 @@ mod tests { true, ), ])); - let variant = JsonVariant::from([ + let variant = JsonObjectValue::from([ ( - "items", + "items".to_string(), JsonVariant::Array(vec![ JsonVariant::from([("id", JsonVariant::from(1i64))]), JsonVariant::from([("id", JsonVariant::from(2i64))]), ]), ), ( - "meta", + "meta".to_string(), JsonVariant::from([("name", JsonVariant::from("foo"))]), ), ]); let value = Value::Struct(json_variant_into_struct_value( variant, struct_type.clone(), + true, )?); assert_eq!( diff --git a/src/datatypes/src/vectors/json/variant.rs b/src/datatypes/src/vectors/json/variant.rs index 1a4b8a861d5..75d87c99861 100644 --- a/src/datatypes/src/vectors/json/variant.rs +++ b/src/datatypes/src/vectors/json/variant.rs @@ -15,10 +15,7 @@ use std::sync::Arc; use arrow_array::ArrayRef; -#[cfg(test)] -use arrow_schema::ArrowError; -use arrow_schema::{DataType, Field}; -#[cfg(test)] +use arrow_schema::{ArrowError, DataType, Field}; use parquet_variant::{ObjectFieldBuilder, Variant, VariantBuilderExt, VariantDecimal16}; #[cfg(test)] use parquet_variant_compute::VariantArrayBuilder; @@ -27,7 +24,6 @@ use parquet_variant_json::VariantToJson; use snafu::ResultExt; use crate::error::{ArrowComputeSnafu, Result}; -#[cfg(test)] use crate::json::value::{JsonNumber, JsonVariant, decode_json_variant}; /// Returns the canonical Arrow field for an unshredded Parquet Variant array. @@ -68,8 +64,7 @@ fn json_variants_to_variant(values: &[Option]) -> Result Ok(ArrayRef::from(builder.build())) } -#[cfg(test)] -fn append_json_variant( +pub(super) fn append_json_variant( builder: &mut impl VariantBuilderExt, value: &JsonVariant, ) -> std::result::Result<(), ArrowError> { @@ -115,7 +110,6 @@ fn append_json_variant( Ok(()) } -#[cfg(test)] fn append_json_value( builder: &mut impl VariantBuilderExt, value: &serde_json::Value, @@ -157,7 +151,6 @@ fn append_json_value( /// Parquet Variant has no unsigned integer primitive. Treat u64 as i64 first, then use Decimal16 /// to represent large (larger than i64::MAX) u64. -#[cfg(test)] fn append_large_u64( builder: &mut impl VariantBuilderExt, value: u64, From 8d887ddd00d67699d0fb873cda11eb50b5771555 Mon Sep 17 00:00:00 2001 From: discord9 Date: Fri, 21 Aug 2026 03:50:30 +0000 Subject: [PATCH 09/10] fix(query): restore columnar group-by for dictionary-encoded tags (#8902) * fix(query): restore columnar group-by for dictionary-encoded tags Bump the DataFusion fork to be93ffd85 (feat/dict-group-column-53), which backports apache/datafusion #23187: DictionaryGroupValuesColumn lets dictionary-encoded group keys use the columnar GroupValuesColumn fast path (hash distinct dictionary values once per batch, resolve rows by key index) instead of falling back to row-based GroupValuesRows. This fixes the TSBS double-groupby regression introduced by #8541 (preserve dictionary-encoded query labels): v1.2.0-beta.1 scan output changed tag columns to Dictionary(UInt32, Utf8), which DataFusion 53.1.0 did not support in GroupValuesColumn's supported_type allow-list, so GROUP BY queries silently dropped to the ~60% slower row path (time_calculating_group_ids +57%, peak_mem +50%, end-to-end +38%). Adds an end-to-end integration test (dict_groupby_sst) that flushes a flat-format SST with dictionary-encoded hostname, runs the tsbs-style double-groupby query, and asserts correct results with no CastExec inserted before the aggregate. Signed-off-by: discord9 * chore(deps): pin merged dictionary group-by support Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --------- Signed-off-by: discord9 Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> Co-authored-by: discord9 --- Cargo.lock | 68 +++++----- Cargo.toml | 30 ++--- tests-integration/tests/dict_groupby_sst.rs | 139 ++++++++++++++++++++ tests-integration/tests/main.rs | 1 + 4 files changed, 189 insertions(+), 49 deletions(-) create mode 100644 tests-integration/tests/dict_groupby_sst.rs diff --git a/Cargo.lock b/Cargo.lock index d7c9241afca..a389391ea90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3738,7 +3738,7 @@ checksum = "a4ae5f15dda3c708c0ade84bfee31ccab44a3da4f88015ed22f63732abe300c8" [[package]] name = "datafusion" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "arrow-schema 58.3.0", @@ -3792,7 +3792,7 @@ dependencies = [ [[package]] name = "datafusion-catalog" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "async-trait", @@ -3816,7 +3816,7 @@ dependencies = [ [[package]] name = "datafusion-catalog-listing" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "async-trait", @@ -3838,7 +3838,7 @@ dependencies = [ [[package]] name = "datafusion-common" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "ahash 0.8.12", "arrow 58.3.0", @@ -3862,7 +3862,7 @@ dependencies = [ [[package]] name = "datafusion-common-runtime" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "futures", "log", @@ -3872,7 +3872,7 @@ dependencies = [ [[package]] name = "datafusion-datasource" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "async-compression", @@ -3906,7 +3906,7 @@ dependencies = [ [[package]] name = "datafusion-datasource-arrow" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "arrow-ipc 58.3.0", @@ -3929,7 +3929,7 @@ dependencies = [ [[package]] name = "datafusion-datasource-csv" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "async-trait", @@ -3951,7 +3951,7 @@ dependencies = [ [[package]] name = "datafusion-datasource-json" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "async-trait", @@ -3974,7 +3974,7 @@ dependencies = [ [[package]] name = "datafusion-datasource-parquet" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "async-trait", @@ -4003,12 +4003,12 @@ dependencies = [ [[package]] name = "datafusion-doc" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" [[package]] name = "datafusion-execution" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "arrow-buffer 58.3.0", @@ -4030,7 +4030,7 @@ dependencies = [ [[package]] name = "datafusion-expr" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "async-trait", @@ -4052,7 +4052,7 @@ dependencies = [ [[package]] name = "datafusion-expr-common" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "datafusion-common", @@ -4064,7 +4064,7 @@ dependencies = [ [[package]] name = "datafusion-functions" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "arrow-buffer 58.3.0", @@ -4095,7 +4095,7 @@ dependencies = [ [[package]] name = "datafusion-functions-aggregate" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "ahash 0.8.12", "arrow 58.3.0", @@ -4116,7 +4116,7 @@ dependencies = [ [[package]] name = "datafusion-functions-aggregate-common" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "ahash 0.8.12", "arrow 58.3.0", @@ -4128,7 +4128,7 @@ dependencies = [ [[package]] name = "datafusion-functions-nested" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "arrow-ord 58.3.0", @@ -4152,7 +4152,7 @@ dependencies = [ [[package]] name = "datafusion-functions-table" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "async-trait", @@ -4167,7 +4167,7 @@ dependencies = [ [[package]] name = "datafusion-functions-window" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "datafusion-common", @@ -4184,7 +4184,7 @@ dependencies = [ [[package]] name = "datafusion-functions-window-common" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "datafusion-common", "datafusion-physical-expr-common", @@ -4193,7 +4193,7 @@ dependencies = [ [[package]] name = "datafusion-macros" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "datafusion-doc", "quote", @@ -4203,7 +4203,7 @@ dependencies = [ [[package]] name = "datafusion-optimizer" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "chrono", @@ -4252,7 +4252,7 @@ dependencies = [ [[package]] name = "datafusion-physical-expr" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "ahash 0.8.12", "arrow 58.3.0", @@ -4275,7 +4275,7 @@ dependencies = [ [[package]] name = "datafusion-physical-expr-adapter" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "datafusion-common", @@ -4289,7 +4289,7 @@ dependencies = [ [[package]] name = "datafusion-physical-expr-common" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "ahash 0.8.12", "arrow 58.3.0", @@ -4305,7 +4305,7 @@ dependencies = [ [[package]] name = "datafusion-physical-optimizer" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "datafusion-common", @@ -4323,7 +4323,7 @@ dependencies = [ [[package]] name = "datafusion-physical-plan" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "ahash 0.8.12", "arrow 58.3.0", @@ -4354,7 +4354,7 @@ dependencies = [ [[package]] name = "datafusion-proto" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "chrono", @@ -4381,7 +4381,7 @@ dependencies = [ [[package]] name = "datafusion-proto-common" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "datafusion-common", @@ -4391,7 +4391,7 @@ dependencies = [ [[package]] name = "datafusion-pruning" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "datafusion-common", @@ -4407,7 +4407,7 @@ dependencies = [ [[package]] name = "datafusion-session" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "async-trait", "datafusion-common", @@ -4420,7 +4420,7 @@ dependencies = [ [[package]] name = "datafusion-sql" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "arrow 58.3.0", "bigdecimal 0.4.8", @@ -4438,7 +4438,7 @@ dependencies = [ [[package]] name = "datafusion-substrait" version = "53.1.0" -source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=d705d4433d2fcbe2bead54e3f1d6cc59727857e1#d705d4433d2fcbe2bead54e3f1d6cc59727857e1" +source = "git+https://github.com/GreptimeTeam/datafusion.git?rev=4c8a6bf28347b346348136247eb67dfb933f1147#4c8a6bf28347b346348136247eb67dfb933f1147" dependencies = [ "async-recursion", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 777d8b302b4..e6b849a6cd0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -351,21 +351,21 @@ git = "https://github.com/GreptimeTeam/greptime-meter.git" rev = "5618e779cf2bb4755b499c630fba4c35e91898cb" [patch.crates-io] -datafusion = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "d705d4433d2fcbe2bead54e3f1d6cc59727857e1" } -datafusion-common = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "d705d4433d2fcbe2bead54e3f1d6cc59727857e1" } -datafusion-datasource = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "d705d4433d2fcbe2bead54e3f1d6cc59727857e1" } -datafusion-expr = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "d705d4433d2fcbe2bead54e3f1d6cc59727857e1" } -datafusion-expr-common = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "d705d4433d2fcbe2bead54e3f1d6cc59727857e1" } -datafusion-functions = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "d705d4433d2fcbe2bead54e3f1d6cc59727857e1" } -datafusion-functions-aggregate-common = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "d705d4433d2fcbe2bead54e3f1d6cc59727857e1" } -datafusion-functions-window-common = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "d705d4433d2fcbe2bead54e3f1d6cc59727857e1" } -datafusion-optimizer = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "d705d4433d2fcbe2bead54e3f1d6cc59727857e1" } -datafusion-physical-expr = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "d705d4433d2fcbe2bead54e3f1d6cc59727857e1" } -datafusion-physical-expr-common = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "d705d4433d2fcbe2bead54e3f1d6cc59727857e1" } -datafusion-physical-plan = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "d705d4433d2fcbe2bead54e3f1d6cc59727857e1" } -datafusion-proto = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "d705d4433d2fcbe2bead54e3f1d6cc59727857e1" } -datafusion-sql = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "d705d4433d2fcbe2bead54e3f1d6cc59727857e1" } -datafusion-substrait = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "d705d4433d2fcbe2bead54e3f1d6cc59727857e1" } +datafusion = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "4c8a6bf28347b346348136247eb67dfb933f1147" } +datafusion-common = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "4c8a6bf28347b346348136247eb67dfb933f1147" } +datafusion-datasource = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "4c8a6bf28347b346348136247eb67dfb933f1147" } +datafusion-expr = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "4c8a6bf28347b346348136247eb67dfb933f1147" } +datafusion-expr-common = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "4c8a6bf28347b346348136247eb67dfb933f1147" } +datafusion-functions = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "4c8a6bf28347b346348136247eb67dfb933f1147" } +datafusion-functions-aggregate-common = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "4c8a6bf28347b346348136247eb67dfb933f1147" } +datafusion-functions-window-common = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "4c8a6bf28347b346348136247eb67dfb933f1147" } +datafusion-optimizer = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "4c8a6bf28347b346348136247eb67dfb933f1147" } +datafusion-physical-expr = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "4c8a6bf28347b346348136247eb67dfb933f1147" } +datafusion-physical-expr-common = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "4c8a6bf28347b346348136247eb67dfb933f1147" } +datafusion-physical-plan = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "4c8a6bf28347b346348136247eb67dfb933f1147" } +datafusion-proto = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "4c8a6bf28347b346348136247eb67dfb933f1147" } +datafusion-sql = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "4c8a6bf28347b346348136247eb67dfb933f1147" } +datafusion-substrait = { git = "https://github.com/GreptimeTeam/datafusion.git", rev = "4c8a6bf28347b346348136247eb67dfb933f1147" } sqlparser = { git = "https://github.com/GreptimeTeam/sqlparser-rs.git", rev = "2aefa08a8d69c96eec2d6d6703598a009bba6e4c" } # on branch v0.61.x [profile.release] diff --git a/tests-integration/tests/dict_groupby_sst.rs b/tests-integration/tests/dict_groupby_sst.rs new file mode 100644 index 00000000000..862b77ef108 --- /dev/null +++ b/tests-integration/tests/dict_groupby_sst.rs @@ -0,0 +1,139 @@ +// Copyright 2023 Greptime Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use common_query::Output; +use frontend::instance::Instance; +use servers::query_handler::sql::SqlQueryHandler; +use session::context::QueryContext; +use tests_integration::standalone::GreptimeDbStandaloneBuilder; + +async fn execute_sql(instance: &Instance, sql: &str) -> Output { + SqlQueryHandler::do_query(instance, sql, QueryContext::arc()) + .await + .remove(0) + .unwrap() +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_dict_groupby_sst() { + common_telemetry::init_default_ut_logging(); + + let standalone = GreptimeDbStandaloneBuilder::new("dict_groupby_sst") + .build() + .await; + let frontend = standalone.fe_instance(); + + execute_sql( + frontend, + r#" + CREATE TABLE cpu ( + hostname STRING PRIMARY KEY, + usage_user DOUBLE, + usage_system DOUBLE, + usage_idle DOUBLE, + greptime_timestamp TIMESTAMP TIME INDEX + ) WITH (append_mode = 'true', sst_format = 'flat') + "#, + ) + .await; + + let mut rows = String::new(); + for index in 0..1200 { + if index != 0 { + rows.push(','); + } + let hostname = match index % 3 { + 0 => "host_a", + 1 => "host_b", + _ => "host_c", + }; + let usage_user = match index % 3 { + 0 => 10.0, + 1 => 20.0, + _ => 30.0, + }; + let hour = 11 + index / 600; + let minute = (index % 600) / 60; + let second = index % 60; + rows.push_str(&format!( + "('{hostname}', {usage_user}, 1.0, 0.0, '2023-06-12T{hour:02}:{minute:02}:{second:02}Z')" + )); + } + execute_sql(frontend, &format!("INSERT INTO cpu VALUES {rows}")).await; + + execute_sql(frontend, "ADMIN FLUSH_TABLE('cpu')").await; + + let count = execute_sql(frontend, "SELECT count(*) FROM cpu") + .await + .data + .pretty_print() + .await; + assert!( + count.contains("| 1200 |"), + "unexpected row count:\n{count}" + ); + + let query = r#" + SELECT hostname, avg(usage_user), date_trunc('hour', greptime_timestamp) + FROM cpu + WHERE greptime_timestamp >= '2023-06-12T00:00:00Z' + AND greptime_timestamp < '2023-06-14T00:00:00Z' + GROUP BY date_trunc('hour', greptime_timestamp), hostname + ORDER BY hostname, date_trunc('hour', greptime_timestamp) + "#; + let groups = execute_sql(frontend, query).await.data.pretty_print().await; + assert_eq!( + 6, + groups + .lines() + .filter(|line| line.starts_with("| host_")) + .count(), + "expected one group per hostname and hour:\n{groups}" + ); + for (hostname, average) in [("host_a", "10.0"), ("host_b", "20.0"), ("host_c", "30.0")] { + assert_eq!( + 2, + groups.matches(&format!("| {hostname} ")).count(), + "expected two hourly groups for {hostname}:\n{groups}" + ); + assert!( + groups.contains(average), + "expected average {average} for {hostname}:\n{groups}" + ); + } + + let explain = execute_sql( + frontend, + r#" + EXPLAIN ANALYZE VERBOSE + SELECT avg(usage_user), date_trunc('hour', greptime_timestamp) + FROM cpu + WHERE greptime_timestamp >= '2023-06-12T00:00:00Z' + AND greptime_timestamp < '2023-06-14T00:00:00Z' + GROUP BY date_trunc('hour', greptime_timestamp), hostname + "#, + ) + .await + .data + .pretty_print() + .await; + assert!( + explain.contains("AggregateExec"), + "expected aggregate in physical plan:\n{explain}" + ); + assert!( + !explain.contains("CastExec") && !explain.contains("CAST(hostname"), + "dictionary hostname must not be cast before aggregation:\n{explain}" + ); +} diff --git a/tests-integration/tests/main.rs b/tests-integration/tests/main.rs index 3112cb72c32..965a3a67462 100644 --- a/tests-integration/tests/main.rs +++ b/tests-integration/tests/main.rs @@ -16,6 +16,7 @@ mod admin_function_event; mod database_ddl_event; +mod dict_groupby_sst; mod event_recorder_test_util; #[macro_use] mod grpc; From 7fd0a7bb9877b218943429296ad442efcf309f7c Mon Sep 17 00:00:00 2001 From: LFC <990479+MichaelScofield@users.noreply.github.com> Date: Fri, 21 Aug 2026 12:14:47 +0000 Subject: [PATCH 10/10] refactor(json2): optimize JSON2 building without auto-expanded paths (#8928) * refactor(json2): optimize JSON2 building without auto-expanded paths Signed-off-by: luofucong * resolve PR comments Signed-off-by: luofucong * avoid panicking memtable write Signed-off-by: luofucong --------- Signed-off-by: luofucong --- src/datatypes/src/error.rs | 8 + src/datatypes/src/json.rs | 25 +++ src/datatypes/src/vectors/json/builder.rs | 254 ++++++++++++++++++++-- src/datatypes/src/vectors/json/variant.rs | 48 +++- 4 files changed, 321 insertions(+), 14 deletions(-) diff --git a/src/datatypes/src/error.rs b/src/datatypes/src/error.rs index b34a5ffa9bf..1def0d6be8d 100644 --- a/src/datatypes/src/error.rs +++ b/src/datatypes/src/error.rs @@ -77,6 +77,13 @@ pub enum Error { location: Location, }, + #[snafu(display("Unimplemented: {feat}"))] + Unimplemented { + feat: String, + #[snafu(implicit)] + location: Location, + }, + #[snafu(display("Failed to parse version in schema meta, value: {}", value))] ParseSchemaVersion { value: String, @@ -312,6 +319,7 @@ impl ErrorExt for Error { use Error::*; match self { UnsupportedOperation { .. } + | Unimplemented { .. } | UnsupportedArrowType { .. } | UnsupportedJsonType { .. } | UnsupportedDefaultExpr { .. } => StatusCode::Unsupported, diff --git a/src/datatypes/src/json.rs b/src/datatypes/src/json.rs index ec9ca4ac789..79fc10b12e7 100644 --- a/src/datatypes/src/json.rs +++ b/src/datatypes/src/json.rs @@ -140,6 +140,16 @@ impl JsonSettings { /// Encode a serde_json::Value into a Value::Json using current settings. pub fn encode(&self, json: Json) -> Result { + if let Json::Object(object) = &json + && object.contains_key(JSON2_REMAINDER_FIELD_NAME) + { + return error::InvalidJsonSnafu { + value: format!( + "root object cannot contain reserved field '{JSON2_REMAINDER_FIELD_NAME}'" + ), + } + .fail(); + } let mut context = JsonContext { path: Vec::new(), settings: self, @@ -777,6 +787,21 @@ mod tests { } } + #[test] + fn test_encode_rejects_reserved_remainder_field() -> Result<()> { + let settings = JsonSettings::default(); + let err = settings + .encode(json!({"!__remainder__!": "user-value"})) + .unwrap_err(); + assert!( + err.to_string() + .contains("root object cannot contain reserved field") + ); + + settings.encode(json!({"nested": {"!__remainder__!": "user-value"}}))?; + Ok(()) + } + #[test] fn test_encode_json_object() { let json = json!({ diff --git a/src/datatypes/src/vectors/json/builder.rs b/src/datatypes/src/vectors/json/builder.rs index 125dbdad58a..ab3fd6d0436 100644 --- a/src/datatypes/src/vectors/json/builder.rs +++ b/src/datatypes/src/vectors/json/builder.rs @@ -24,17 +24,18 @@ use snafu::{ResultExt, ensure}; use crate::data_type::ConcreteDataType; use crate::error::{ - ArrowComputeSnafu, Result, TryFromValueSnafu, UnexpectedSnafu, UnsupportedOperationSnafu, + ArrowComputeSnafu, Result, TryFromValueSnafu, UnexpectedSnafu, UnimplementedSnafu, + UnsupportedOperationSnafu, }; use crate::extension::json::JSON2_REMAINDER_FIELD_NAME; -use crate::json::value::{JsonNumber, JsonVariant, encode_json_variant}; +use crate::json::value::{JsonNumber, JsonVariant, JsonVariantRef, encode_json_variant}; use crate::json::{JSON2_MAX_STRUCTURED_DEPTH, JsonSettings}; use crate::prelude::{ValueRef, Vector, VectorRef}; use crate::types::StructType; use crate::types::json_type::{JsonNativeType, is_include}; use crate::value::{ListValue, StructValue, StructValueRef, Value}; -use crate::vectors::json::variant::{append_json_variant, variant_field}; -use crate::vectors::{Helper, MutableVector, StructVectorBuilder}; +use crate::vectors::json::variant::{append_json_variant, append_json_variant_ref, variant_field}; +use crate::vectors::{Helper, MutableVector, NullVector, StructVectorBuilder}; type JsonObjectValue = BTreeMap; @@ -44,17 +45,25 @@ type JsonObjectValue = BTreeMap; /// Auto-expanding mode always materializes type-hinted paths, selects up to /// `max_auto_expanded_paths` compatible unhinted leaf paths by frequency, and stores /// conflicting or unselected paths in the Variant remainder field. -#[derive(Clone)] pub(crate) struct JsonVectorBuilder { state: JsonVectorBuilderState, } -#[derive(Clone)] enum JsonVectorBuilderState { Legacy { merged_type: JsonNativeType, values: Vec, }, + ExplicitOnly { + /// Paths declared by type hints and stored as dedicated Struct fields. + explicit_type: JsonNativeType, + /// Concrete Struct type used to append explicit values without buffering rows. + struct_type: StructType, + /// Builder for values selected by the explicit type hints. + explicit: StructVectorBuilder, + /// Builder for all values outside the explicit type hints. + remainder: VariantArrayBuilder, + }, AutoExpanding { /// Paths declared by type hints and always stored as dedicated Struct fields. explicit_type: JsonNativeType, @@ -69,6 +78,7 @@ impl JsonVectorBuilderState { fn native_type(&self) -> JsonNativeType { match self { Self::Legacy { merged_type, .. } => merged_type.clone(), + Self::ExplicitOnly { explicit_type, .. } => explicit_type.clone(), Self::AutoExpanding { explicit_type, max_auto_expanded_paths, @@ -80,6 +90,7 @@ impl JsonVectorBuilderState { fn len(&self) -> usize { match self { Self::Legacy { values, .. } | Self::AutoExpanding { values, .. } => values.len(), + Self::ExplicitOnly { explicit, .. } => explicit.len(), } } @@ -89,6 +100,14 @@ impl JsonVectorBuilderState { merged_type, values, } => build_legacy(values, merged_type), + Self::ExplicitOnly { + explicit, + remainder, + .. + } => { + let remainder = std::mem::replace(remainder, VariantArrayBuilder::new(0)).build(); + finish_vector(explicit.to_vector(), ArrayRef::from(remainder)) + } Self::AutoExpanding { explicit_type, max_auto_expanded_paths, @@ -101,6 +120,38 @@ impl JsonVectorBuilderState { } } + fn try_build_cloned(&self) -> Result { + let mut state = match self { + Self::Legacy { + merged_type, + values, + } => Self::Legacy { + merged_type: merged_type.clone(), + values: values.clone(), + }, + Self::AutoExpanding { + explicit_type, + max_auto_expanded_paths, + values, + } => Self::AutoExpanding { + explicit_type: explicit_type.clone(), + max_auto_expanded_paths: *max_auto_expanded_paths, + values: values.clone(), + }, + // Only TimeSeriesMemtable requires a non-consuming snapshot, while JSON2 targets + // BulkMemtable. We've tried our best to support it above, but if this match arm does + // not, it's OK. The only reason it doesn't is because of `VariantArrayBuilder`. We'll + // track the upstream and see. + Self::ExplicitOnly { .. } => { + return UnimplementedSnafu { + feat: "no auto expanded JSON2 array builder", + } + .fail(); + } + }; + state.try_build() + } + fn try_push_value_ref(&mut self, value: &ValueRef) -> Result<()> { let ValueRef::Json(value) = value else { return TryFromValueSnafu { @@ -125,6 +176,22 @@ impl JsonVectorBuilderState { } values.push(JsonVariant::from(value.variant())); } + Self::ExplicitOnly { + explicit_type, + struct_type, + explicit, + remainder, + } => { + if value.is_null() { + explicit.push_null(); + remainder.append_null(); + } else { + let (value, rest) = + split_to_explicit_ref(value.variant(), explicit_type, struct_type)?; + explicit.push_struct_value_ref(value)?; + append_json_variant_ref(remainder, &rest).context(ArrowComputeSnafu)?; + } + } Self::AutoExpanding { values, .. } => { values.push(JsonVariant::from(value.variant())); } @@ -137,6 +204,14 @@ impl JsonVectorBuilderState { Self::Legacy { values, .. } | Self::AutoExpanding { values, .. } => { values.push(JsonVariant::Null) } + Self::ExplicitOnly { + explicit, + remainder, + .. + } => { + explicit.push_null(); + remainder.append_null(); + } } } } @@ -163,13 +238,28 @@ impl JsonVectorBuilder { for hint in settings.type_hints() { insert_dynamic_type(&mut explicit_type, &hint.path, (&hint.data_type).into()); } - Self { - state: JsonVectorBuilderState::AutoExpanding { + let state = if settings.max_auto_expanded_paths() == Some(0) { + let DataType::Struct(fields) = explicit_type.as_arrow_type() else { + unreachable!("JSON2 explicit type must map to Arrow Struct") + }; + let struct_type = StructType::from(&fields); + JsonVectorBuilderState::ExplicitOnly { + explicit_type, + explicit: StructVectorBuilder::with_type_and_capacity( + struct_type.clone(), + capacity, + ), + struct_type, + remainder: VariantArrayBuilder::new(capacity), + } + } else { + JsonVectorBuilderState::AutoExpanding { explicit_type, max_auto_expanded_paths: settings.max_auto_expanded_paths().unwrap_or(u32::MAX), values: Vec::with_capacity(capacity), - }, - } + } + }; + Self { state } } fn try_build(&mut self) -> Result { @@ -451,6 +541,132 @@ fn insert_dynamic_type>( ) } +fn split_to_explicit_ref<'a>( + value: &JsonVariantRef<'a>, + explicit_type: &JsonNativeType, + struct_type: &StructType, +) -> Result<(StructValueRef<'a>, JsonVariantRef<'a>)> { + let JsonVariantRef::Object(object) = value else { + return TryFromValueSnafu { + reason: "expected json object value".to_string(), + } + .fail(); + }; + let explicit = json_object_ref_into_struct_value_ref(object, struct_type)?; + let remainder = remainder_ref(object, explicit_type)?; + Ok((explicit, JsonVariantRef::Object(remainder))) +} + +fn json_object_ref_into_struct_value_ref<'a>( + object: &BTreeMap<&'a str, JsonVariantRef<'a>>, + struct_type: &StructType, +) -> Result> { + let mut values = Vec::with_capacity(struct_type.fields().len()); + for field in struct_type.fields().iter() { + let value = match object.get(field.name()) { + Some(value) => json_variant_ref_into_value_ref(value, field.data_type())?, + None => ValueRef::Null, + }; + values.push(value); + } + Ok(StructValueRef::RefList { + val: values, + fields: struct_type.clone(), + }) +} + +fn json_variant_ref_into_value_ref<'a>( + value: &JsonVariantRef<'a>, + expected_type: &ConcreteDataType, +) -> Result> { + let value = match (value, expected_type) { + (JsonVariantRef::Null, _) | (_, ConcreteDataType::Null(_)) => ValueRef::Null, + (JsonVariantRef::Object(object), ConcreteDataType::Struct(struct_type)) => { + ValueRef::Struct(json_object_ref_into_struct_value_ref(object, struct_type)?) + } + (JsonVariantRef::Bool(x), ConcreteDataType::Boolean(_)) => ValueRef::Boolean(*x), + (JsonVariantRef::Number(x), ConcreteDataType::UInt64(_)) => { + let Some(x) = x.as_u64() else { + return TryFromValueSnafu { + reason: format!("unable to convert {x:?} to UInt64"), + } + .fail(); + }; + ValueRef::UInt64(x) + } + (JsonVariantRef::Number(x), ConcreteDataType::Int64(_)) => { + let x = match x { + JsonNumber::PosInt(x) => i64::try_from(*x).ok(), + JsonNumber::NegInt(x) => Some(*x), + JsonNumber::Float(_) => None, + }; + let Some(x) = x else { + return TryFromValueSnafu { + reason: format!("unable to convert {x:?} to Int64"), + } + .fail(); + }; + ValueRef::Int64(x) + } + (JsonVariantRef::Number(JsonNumber::PosInt(x)), ConcreteDataType::Float64(_)) => { + ValueRef::Float64((*x as f64).into()) + } + (JsonVariantRef::Number(JsonNumber::NegInt(x)), ConcreteDataType::Float64(_)) => { + ValueRef::Float64((*x as f64).into()) + } + (JsonVariantRef::Number(JsonNumber::Float(x)), ConcreteDataType::Float64(_)) => { + ValueRef::Float64(*x) + } + (JsonVariantRef::String(x), ConcreteDataType::String(_)) => ValueRef::String(x), + (value, expected_type) => { + return TryFromValueSnafu { + reason: format!("unable to convert json value {value:?} to {expected_type}"), + } + .fail(); + } + }; + Ok(value) +} + +fn remainder_ref<'a>( + object: &BTreeMap<&'a str, JsonVariantRef<'a>>, + explicit_type: &JsonNativeType, +) -> Result>> { + let JsonNativeType::Object(fields) = explicit_type else { + return UnexpectedSnafu { + reason: "JSON2 explicit type must be an object", + } + .fail(); + }; + let mut remainder = BTreeMap::new(); + for (&name, value) in object { + match fields.get(name) { + Some(data_type @ JsonNativeType::Object(_)) => match value { + JsonVariantRef::Null => {} + JsonVariantRef::Object(object) => { + let child = remainder_ref(object, data_type)?; + if !child.is_empty() { + remainder.insert(name, JsonVariantRef::Object(child)); + } + } + _ => { + return TryFromValueSnafu { + reason: "expected json object value".to_string(), + } + .fail(); + } + }, + // A non-object entry in the explicit type tree is an explicit leaf and is + // already written to the Struct builder. So here does nothing. + Some(_) => {} + None => { + remainder.insert(name, value.clone()); + } + } + } + Ok(remainder) +} + fn split_to_explicit( value: JsonVariant, explicit_type: &JsonNativeType, @@ -611,11 +827,19 @@ impl MutableVector for JsonVectorBuilder { } fn to_vector(&mut self) -> VectorRef { - self.try_build().unwrap_or_else(|e| panic!("{:?}", e)) + self.try_build().unwrap_or_else(|e| { + // Just try to avoid panicking here. + common_telemetry::error!(e; "Unable to build JSON2 vector"); + Arc::new(NullVector::new(self.len())) + }) } fn to_vector_cloned(&self) -> VectorRef { - self.clone().to_vector() + self.state.try_build_cloned().unwrap_or_else(|e| { + // Just try to avoid panicking here. + common_telemetry::error!(e; "Unable to build JSON2 vector"); + Arc::new(NullVector::new(self.len())) + }) } fn try_push_value_ref(&mut self, value: &ValueRef) -> Result<()> { @@ -772,7 +996,7 @@ mod tests { } #[test] - fn test_zero_budget_builder_uses_fixed_schema_and_remainder() -> Result<()> { + fn test_zero_budget_builder_uses_explicit_only_schema_and_remainder() -> Result<()> { let settings = JsonSettings::try_new( vec![ JsonTypeHint { @@ -800,6 +1024,10 @@ mod tests { Some(0), )?; let mut builder = JsonVectorBuilder::with_settings(&settings, 2); + assert!(matches!( + &builder.state, + JsonVectorBuilderState::ExplicitOnly { .. } + )); let values = [ json!({ "kind": "record", diff --git a/src/datatypes/src/vectors/json/variant.rs b/src/datatypes/src/vectors/json/variant.rs index 75d87c99861..9f90f0aaacc 100644 --- a/src/datatypes/src/vectors/json/variant.rs +++ b/src/datatypes/src/vectors/json/variant.rs @@ -24,7 +24,7 @@ use parquet_variant_json::VariantToJson; use snafu::ResultExt; use crate::error::{ArrowComputeSnafu, Result}; -use crate::json::value::{JsonNumber, JsonVariant, decode_json_variant}; +use crate::json::value::{JsonNumber, JsonVariant, JsonVariantRef, decode_json_variant}; /// Returns the canonical Arrow field for an unshredded Parquet Variant array. pub fn variant_field(name: impl Into, nullable: bool) -> Field { @@ -110,6 +110,52 @@ pub(super) fn append_json_variant( Ok(()) } +pub(super) fn append_json_variant_ref( + builder: &mut impl VariantBuilderExt, + value: &JsonVariantRef<'_>, +) -> std::result::Result<(), ArrowError> { + match value { + JsonVariantRef::Null => builder.append_value(Variant::Null), + JsonVariantRef::Bool(value) => builder.append_value(*value), + JsonVariantRef::Number(JsonNumber::PosInt(value)) => { + if let Ok(value) = i64::try_from(*value) { + builder.append_value(value); + } else { + append_large_u64(builder, *value)?; + } + } + JsonVariantRef::Number(JsonNumber::NegInt(value)) => builder.append_value(*value), + JsonVariantRef::Number(JsonNumber::Float(value)) => { + if value.0.is_finite() { + builder.append_value(value.0) + } else { + builder.append_value("NaN") + } + } + JsonVariantRef::String(value) => builder.append_value(*value), + JsonVariantRef::Array(values) => { + let mut list = builder.try_new_list()?; + for value in values { + append_json_variant_ref(&mut list, value)?; + } + list.finish(); + } + JsonVariantRef::Object(values) => { + let mut object = builder.try_new_object()?; + for (name, value) in values { + append_json_variant_ref(&mut ObjectFieldBuilder::new(name, &mut object), value)?; + } + object.finish(); + } + JsonVariantRef::Variant(value) => { + let value = decode_json_variant(value) + .map_err(|e| ArrowError::JsonError(format!("Failed to decode JSONB: {e}")))?; + append_json_value(builder, &value)?; + } + } + Ok(()) +} + fn append_json_value( builder: &mut impl VariantBuilderExt, value: &serde_json::Value,