diff --git a/src/operator/src/expr_helper.rs b/src/operator/src/expr_helper.rs index 02c1af46706..d69277a1c90 100644 --- a/src/operator/src/expr_helper.rs +++ b/src/operator/src/expr_helper.rs @@ -68,8 +68,8 @@ use sql::statements::create::{ CreateView, TableConstraint, }; use sql::statements::{ - OptionMap, column_to_schema, concrete_data_type_to_sql_data_type, - sql_column_def_to_grpc_column_def, sql_data_type_to_concrete_data_type, value_to_sql_value, + OptionMap, column_to_schema, concrete_data_type_to_sql_data_type, sql_col_to_grpc_col_def, + sql_data_type_to_concrete_data_type, value_to_sql_value, }; use sql::util::extract_tables_from_query; use store_api::mito_engine_options::{COMPACTION_OVERRIDE, COMPACTION_TYPE}; @@ -828,12 +828,10 @@ pub(crate) fn to_alter_table_expr( add_columns: add_columns .into_iter() .map(|add_column| { - let column_def = sql_column_def_to_grpc_column_def( - &add_column.column_def, - Some(&query_ctx.timezone()), - ) - .map_err(BoxedError::new) - .context(ExternalSnafu)?; + let column_def = + sql_col_to_grpc_col_def(&add_column.column, Some(&query_ctx.timezone())) + .map_err(BoxedError::new) + .context(ExternalSnafu)?; if is_interval_type(&column_def.data_type()) { return NotSupportedSnafu { feat: "Add column with interval type", @@ -1225,6 +1223,32 @@ mod tests { assert!(is_json2_extension_type(&schema.arrow_schema().fields()[0])); } + #[test] + fn test_to_alter_add_json2_column_preserves_settings() { + let sql = "ALTER TABLE monitor ADD COLUMN payload JSON2(max_auto_expanded_paths = 0, service STRING, nested.value BIGINT)"; + let stmt = + ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default()) + .unwrap() + .pop() + .unwrap(); + let Statement::AlterTable(alter_table) = stmt else { + unreachable!() + }; + let expr = to_alter_table_expr(alter_table, &QueryContext::arc()).unwrap(); + let AlterTableKind::AddColumns(AddColumns { add_columns, .. }) = expr.kind.unwrap() else { + unreachable!() + }; + let column_schema = + try_as_column_schema(add_columns[0].column_def.as_ref().unwrap()).unwrap(); + let metadata: datatypes::extension::json::JsonMetadata = + serde_json::from_str(&column_schema.metadata()["ARROW:extension:metadata"]).unwrap(); + let settings = metadata.json_settings(); + assert_eq!(Some(0), settings.max_auto_expanded_paths()); + assert_eq!(2, settings.type_hints().len()); + assert_eq!(vec!["service"], settings.type_hints()[0].path); + assert_eq!(vec!["nested", "value"], settings.type_hints()[1].path); + } + #[test] fn test_create_flow_tql_expr() { let sql = r#" diff --git a/src/sql/src/parsers/alter_parser.rs b/src/sql/src/parsers/alter_parser.rs index 3e5c27fda73..62c417d2537 100644 --- a/src/sql/src/parsers/alter_parser.rs +++ b/src/sql/src/parsers/alter_parser.rs @@ -26,7 +26,7 @@ use sqlparser::parser::IsOptional::Mandatory; use sqlparser::parser::{Parser, ParserError}; use sqlparser::tokenizer::{Token, TokenWithSpan}; -use crate::ast::ObjectNamePartExt; +use crate::ast::{ColumnOption, ObjectNamePartExt}; use crate::error::{self, InvalidColumnOptionSnafu, Result, SetFulltextOptionSnafu}; use crate::parser::ParserContext; use crate::parsers::create_parser::{INVERTED, parse_json2_type_and_options}; @@ -379,14 +379,67 @@ impl ParserContext<'_> { Ok(AlterTableOperation::AddConstraint(constraint)) } else { self.parser.prev_token(); - let add_columns = self - .parser - .parse_comma_separated(parse_add_columns) - .context(error::SyntaxSnafu)?; + let mut add_columns = vec![self.parse_add_column()?]; + while self.parser.consume_token(&Token::Comma) { + if matches!( + self.parser.peek_token().token, + Token::SemiColon | Token::EOF + ) || self.matches_keyword(Keyword::WITH) + { + break; + } + add_columns.push(self.parse_add_column()?); + } Ok(AlterTableOperation::AddColumns { add_columns }) } } + fn parse_add_column(&mut self) -> Result { + self.parser + .expect_keyword(Keyword::ADD) + .context(error::SyntaxSnafu)?; + let _ = self.parser.parse_keyword(Keyword::COLUMN); + let add_if_not_exists = + self.parser + .parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]); + let column = self.parse_column_def(true)?; + + // These attributes need ALTER-specific execution support. + ensure!( + column.extensions.fulltext_index_options.is_none() + && column.extensions.skipping_index_options.is_none() + && column.extensions.inverted_index_options.is_none() + && !column + .options() + .iter() + .any(|option| matches!(option.option, ColumnOption::DialectSpecific(_))), + InvalidColumnOptionSnafu { + name: column.name().to_string(), + msg: "ADD COLUMN does not support inline indexes or TIME INDEX", + } + ); + + let location = if self.parser.parse_keyword(Keyword::FIRST) { + Some(AddColumnLocation::First) + } else if matches!(self.parser.peek_token().token, Token::Word(word) if word.value.eq_ignore_ascii_case("AFTER")) + { + self.parser.next_token(); + let name = Self::canonicalize_identifier( + self.parser.parse_identifier().context(error::SyntaxSnafu)?, + ); + Some(AddColumnLocation::After { + column_name: name.value, + }) + } else { + None + }; + Ok(AddColumn { + column, + location, + add_if_not_exists, + }) + } + fn parse_alter_table_drop_default( &mut self, column_name: Ident, @@ -704,34 +757,6 @@ fn parse_string_options(parser: &mut Parser) -> std::result::Result<(String, Str Ok((name, value)) } -fn parse_add_columns(parser: &mut Parser) -> std::result::Result { - parser.expect_keyword(Keyword::ADD)?; - let _ = parser.parse_keyword(Keyword::COLUMN); - let add_if_not_exists = parser.parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]); - let mut column_def = parser.parse_column_def()?; - column_def.name = ParserContext::canonicalize_identifier(column_def.name); - let location = if parser.parse_keyword(Keyword::FIRST) { - Some(AddColumnLocation::First) - } else if let Token::Word(word) = parser.peek_token().token { - if word.value.eq_ignore_ascii_case("AFTER") { - let _ = parser.next_token(); - let name = ParserContext::canonicalize_identifier(parser.parse_identifier()?); - Some(AddColumnLocation::After { - column_name: name.value, - }) - } else { - None - } - } else { - None - }; - Ok(AddColumn { - column_def, - location, - add_if_not_exists, - }) -} - /// Parses a comma separated list of string literals. fn parse_string_option_names(parser: &mut Parser) -> std::result::Result { parser.parse_literal_string() @@ -750,6 +775,75 @@ mod tests { use crate::dialect::GreptimeDbDialect; use crate::parser::ParseOptions; use crate::statements::alter::AlterDatabaseOperation; + use crate::statements::create::{Column, ColumnExtensions}; + + #[test] + fn test_add_column_matches_create_table() { + for (definition, location) in [ + ( + "payload JSON2(max_auto_expanded_paths = 100, service STRING) NOT NULL", + "AFTER ts", + ), + ("payload JSON2(service STRING) NULL", "FIRST"), + ("embedding VECTOR(3) NOT NULL", "AFTER ts"), + ("reading INT DEFAULT 42 COMMENT 'value'", ""), + ("\"select\" STRING NULL", ""), + ] { + let parse = |sql: &str| { + ParserContext::create_with_dialect( + sql, + &GreptimeDbDialect {}, + ParseOptions::default(), + ) + .unwrap() + .pop() + .unwrap() + }; + let Statement::CreateTable(create) = parse(&format!( + "CREATE TABLE t ({definition}, n INT, ts TIMESTAMP TIME INDEX)" + )) else { + unreachable!() + }; + let Statement::AlterTable(alter) = parse(&format!( + "ALTER TABLE t ADD COLUMN {definition} {location}, ADD COLUMN n INT" + )) else { + unreachable!() + }; + let AlterTableOperation::AddColumns { add_columns } = alter.alter_operation() else { + unreachable!() + }; + assert_eq!(add_columns.len(), 2); + for (expected, actual) in create.columns.iter().zip(add_columns) { + assert_eq!(expected, &actual.column); + } + let Statement::AlterTable(roundtrip) = parse(&alter.to_string()) else { + unreachable!() + }; + assert_eq!(alter, roundtrip); + } + } + + #[test] + fn test_add_column_rejects_unsupported_attributes() { + for definition in [ + "payload STRING FULLTEXT INDEX", + "payload STRING SKIPPING INDEX", + "payload STRING INVERTED INDEX", + "ts TIMESTAMP TIME INDEX", + ] { + let err = ParserContext::create_with_dialect( + &format!("ALTER TABLE t ADD COLUMN {definition}"), + &GreptimeDbDialect {}, + ParseOptions::default(), + ) + .unwrap_err(); + assert!( + err.to_string() + .contains("ADD COLUMN does not support inline indexes or TIME INDEX"), + "{err}" + ); + } + } #[test] fn test_parse_alter_database() { @@ -830,10 +924,14 @@ mod tests { match alter_operation { AlterTableOperation::AddColumns { add_columns } => { assert_eq!(add_columns.len(), 1); - assert_eq!("tagk_i", add_columns[0].column_def.name.value); - assert_eq!(DataType::String(None), add_columns[0].column_def.data_type); + assert_eq!("tagk_i", add_columns[0].column.column_def.name.value); + assert_eq!( + DataType::String(None), + add_columns[0].column.column_def.data_type + ); assert!( add_columns[0] + .column .column_def .options .iter() @@ -866,10 +964,14 @@ mod tests { assert_matches!(alter_operation, AlterTableOperation::AddColumns { .. }); match alter_operation { AlterTableOperation::AddColumns { add_columns } => { - assert_eq!("tagk_i", add_columns[0].column_def.name.value); - assert_eq!(DataType::String(None), add_columns[0].column_def.data_type); + assert_eq!("tagk_i", add_columns[0].column.column_def.name.value); + assert_eq!( + DataType::String(None), + add_columns[0].column.column_def.data_type + ); assert!( add_columns[0] + .column .column_def .options .iter() @@ -931,7 +1033,7 @@ mod tests { .zip(expecteds) .collect::, ColumnDef))>>() { - assert_eq!(add_column.column_def, expected.1); + assert_eq!(add_column.column.column_def, expected.1); assert_eq!(&expected.0, &add_column.location); } } @@ -962,28 +1064,37 @@ mod tests { AlterTableOperation::AddColumns { add_columns } => { let expected = [ AddColumn { - column_def: ColumnDef { - name: Ident::new("a"), - data_type: DataType::Integer(None), - options: vec![], + column: Column { + column_def: ColumnDef { + name: Ident::new("a"), + data_type: DataType::Integer(None), + options: vec![], + }, + extensions: ColumnExtensions::default(), }, location: None, add_if_not_exists: true, }, AddColumn { - column_def: ColumnDef { - name: Ident::new("b"), - data_type: DataType::String(None), - options: vec![], + column: Column { + column_def: ColumnDef { + name: Ident::new("b"), + data_type: DataType::String(None), + options: vec![], + }, + extensions: ColumnExtensions::default(), }, location: None, add_if_not_exists: false, }, AddColumn { - column_def: ColumnDef { - name: Ident::new("c"), - data_type: DataType::Int(None), - options: vec![], + column: Column { + column_def: ColumnDef { + name: Ident::new("c"), + data_type: DataType::Int(None), + options: vec![], + }, + extensions: ColumnExtensions::default(), }, location: None, add_if_not_exists: true, diff --git a/src/sql/src/parsers/create_parser.rs b/src/sql/src/parsers/create_parser.rs index 32606679a1c..c87d0274382 100644 --- a/src/sql/src/parsers/create_parser.rs +++ b/src/sql/src/parsers/create_parser.rs @@ -41,8 +41,8 @@ use table::requests::{validate_database_option, validate_database_option_value}; use crate::ast::{ColumnDef, Ident, ObjectNamePartExt}; use crate::error::{ self, InvalidColumnOptionSnafu, InvalidDatabaseOptionSnafu, InvalidDatabaseOptionValueSnafu, - InvalidFlowQuerySnafu, InvalidIntervalSnafu, InvalidSqlSnafu, InvalidTimeIndexSnafu, - MissingTimeIndexSnafu, Result, SyntaxSnafu, UnexpectedSnafu, UnsupportedSnafu, + InvalidFlowQuerySnafu, InvalidIntervalSnafu, InvalidTimeIndexSnafu, MissingTimeIndexSnafu, + Result, SyntaxSnafu, UnexpectedSnafu, UnsupportedSnafu, }; use crate::parser::{FLOW, ParserContext}; use crate::parsers::tql_parser; @@ -685,7 +685,7 @@ impl<'a> ParserContext<'a> { columns: &mut Vec, constraints: &mut Vec, ) -> Result<()> { - let mut column = self.parse_column_def()?; + let mut column = self.parse_column_def(false)?; let mut time_index_opt_idx = None; for (index, opt) in column.options().iter().enumerate() { @@ -775,22 +775,16 @@ impl<'a> ParserContext<'a> { Ok(name) } - pub fn parse_column_def(&mut self) -> Result { - let name = self.parse_column_name().context(SyntaxSnafu)?; + /// Parses a column definition, optionally allowing unquoted keyword names + /// for compatibility with ALTER TABLE ADD COLUMN. + pub fn parse_column_def(&mut self, allow_keyword_name: bool) -> Result { + let name = if allow_keyword_name { + self.parser.parse_identifier().context(SyntaxSnafu)? + } else { + self.parse_column_name().context(SyntaxSnafu)? + }; let parser = &mut self.parser; - ensure!( - !(name.quote_style.is_none() && - // "ALL_KEYWORDS" are sorted. - ALL_KEYWORDS.binary_search(&name.value.to_uppercase().as_str()).is_ok()), - InvalidSqlSnafu { - msg: format!( - "Cannot use keyword '{}' as column name. Hint: add quotes to the name.", - name.value - ), - } - ); - let mut extensions = ColumnExtensions::default(); let data_type = @@ -2924,11 +2918,16 @@ ENGINE=mito"; #[test] fn test_invalid_column_name() { - let sql = "create table foo(user string, i timestamp time index)"; - let result = - ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default()); - let err = result.unwrap_err().output_msg(); - assert!(err.contains("Cannot use keyword 'user' as column name")); + for name in ["user", "at", "value", "select"] { + let sql = format!("create table foo({name} string, i timestamp time index)"); + let result = ParserContext::create_with_dialect( + &sql, + &GreptimeDbDialect {}, + ParseOptions::default(), + ); + let err = result.unwrap_err().output_msg(); + assert!(err.contains(&format!("Cannot use keyword '{name}' as column name"))); + } // If column name is quoted, it's valid even same with keyword. let sql = r#" diff --git a/src/sql/src/statements.rs b/src/sql/src/statements.rs index 8a001510045..c2c97c34567 100644 --- a/src/sql/src/statements.rs +++ b/src/sql/src/statements.rs @@ -173,11 +173,12 @@ fn set_json2_extension(column_schema: &mut ColumnSchema, column: &Column) -> Res Ok(()) } -/// Convert `ColumnDef` in sqlparser to `ColumnDef` in gRPC proto. -pub fn sql_column_def_to_grpc_column_def( - col: &ColumnDef, +/// Converts a SQL [`Column`] to a `ColumnDef` in gRPC proto. +pub fn sql_col_to_grpc_col_def( + column: &Column, timezone: Option<&Timezone>, ) -> Result { + let col = &column.column_def; let name = col.name.value.clone(); let data_type = sql_data_type_to_concrete_data_type(&col.data_type)?; @@ -208,9 +209,11 @@ pub fn sql_column_def_to_grpc_column_def( SemanticType::Field }; - // TODO(fys): Extend the ALTER TABLE ADD COLUMN parser to support JSON2 - // type hints and pass the parsed JsonSettings through this conversion. - let options = json2_extension(&col.data_type, JsonSettings::new_v2()).map(|extension| { + let settings = column + .extensions + .build_json_settings()? + .unwrap_or_else(JsonSettings::new_v2); + let options = json2_extension(&col.data_type, settings).map(|extension| { let mut options = ColumnOptions::default(); options.options.insert( EXTENSION_TYPE_NAME_KEY.to_string(), @@ -489,15 +492,18 @@ mod tests { } #[test] - pub fn test_sql_column_def_to_grpc_column_def() { + pub fn test_sql_col_to_grpc_col_def() { // test basic - let column_def = ColumnDef { - name: "col".into(), - data_type: SqlDataType::Double(ExactNumberInfo::None), - options: vec![], + let column = Column { + column_def: ColumnDef { + name: "col".into(), + data_type: SqlDataType::Double(ExactNumberInfo::None), + options: vec![], + }, + extensions: ColumnExtensions::default(), }; - let grpc_column_def = sql_column_def_to_grpc_column_def(&column_def, None).unwrap(); + let grpc_column_def = sql_col_to_grpc_col_def(&column, None).unwrap(); assert_eq!("col", grpc_column_def.name); assert!(grpc_column_def.is_nullable); // nullable when options are empty @@ -506,56 +512,65 @@ mod tests { assert_eq!(grpc_column_def.semantic_type, SemanticType::Field as i32); // test not null - let column_def = ColumnDef { - name: "col".into(), - data_type: SqlDataType::Double(ExactNumberInfo::None), - options: vec![ColumnOptionDef { - name: None, - option: ColumnOption::NotNull, - }], + let column = Column { + column_def: ColumnDef { + name: "col".into(), + data_type: SqlDataType::Double(ExactNumberInfo::None), + options: vec![ColumnOptionDef { + name: None, + option: ColumnOption::NotNull, + }], + }, + extensions: ColumnExtensions::default(), }; - let grpc_column_def = sql_column_def_to_grpc_column_def(&column_def, None).unwrap(); + let grpc_column_def = sql_col_to_grpc_col_def(&column, None).unwrap(); assert!(!grpc_column_def.is_nullable); // test primary key - let column_def = ColumnDef { - name: "col".into(), - data_type: SqlDataType::Double(ExactNumberInfo::None), - options: vec![ColumnOptionDef { - name: None, - option: ColumnOption::PrimaryKey(PrimaryKeyConstraint { + let column = Column { + column_def: ColumnDef { + name: "col".into(), + data_type: SqlDataType::Double(ExactNumberInfo::None), + options: vec![ColumnOptionDef { name: None, - index_name: None, - index_type: None, - columns: vec![], - index_options: vec![], - characteristics: None, - }), - }], + option: ColumnOption::PrimaryKey(PrimaryKeyConstraint { + name: None, + index_name: None, + index_type: None, + columns: vec![], + index_options: vec![], + characteristics: None, + }), + }], + }, + extensions: ColumnExtensions::default(), }; - let grpc_column_def = sql_column_def_to_grpc_column_def(&column_def, None).unwrap(); + let grpc_column_def = sql_col_to_grpc_col_def(&column, None).unwrap(); assert_eq!(grpc_column_def.semantic_type, SemanticType::Tag as i32); } #[test] - pub fn test_sql_column_def_to_grpc_column_def_with_timezone() { - let column_def = ColumnDef { - name: "col".into(), - // MILLISECOND - data_type: SqlDataType::Timestamp(Some(3), TimezoneInfo::None), - options: vec![ColumnOptionDef { - name: None, - option: ColumnOption::Default(Expr::Value( - SqlValue::SingleQuotedString("2024-01-30T00:01:01".to_string()).into(), - )), - }], + pub fn test_sql_col_to_grpc_col_def_with_timezone() { + let column = Column { + column_def: ColumnDef { + name: "col".into(), + // MILLISECOND + data_type: SqlDataType::Timestamp(Some(3), TimezoneInfo::None), + options: vec![ColumnOptionDef { + name: None, + option: ColumnOption::Default(Expr::Value( + SqlValue::SingleQuotedString("2024-01-30T00:01:01".to_string()).into(), + )), + }], + }, + extensions: ColumnExtensions::default(), }; // with timezone "Asia/Shanghai" - let grpc_column_def = sql_column_def_to_grpc_column_def( - &column_def, + let grpc_column_def = sql_col_to_grpc_col_def( + &column, Some(&Timezone::from_tz_string("Asia/Shanghai").unwrap()), ) .unwrap(); @@ -575,7 +590,7 @@ mod tests { ); // without timezone - let grpc_column_def = sql_column_def_to_grpc_column_def(&column_def, None).unwrap(); + let grpc_column_def = sql_col_to_grpc_col_def(&column, None).unwrap(); assert_eq!("col", grpc_column_def.name); assert!(grpc_column_def.is_nullable); // nullable when options are empty assert_eq!( diff --git a/src/sql/src/statements/alter.rs b/src/sql/src/statements/alter.rs index 4fc45c2e69b..781c65ab528 100644 --- a/src/sql/src/statements/alter.rs +++ b/src/sql/src/statements/alter.rs @@ -22,11 +22,11 @@ use common_query::AddColumnLocation; use datatypes::schema::{FulltextOptions, SkippingIndexOptions}; use itertools::Itertools; use serde::Serialize; -use sqlparser::ast::{ColumnDef, DataType, Expr, Ident, ObjectName, TableConstraint}; +use sqlparser::ast::{DataType, Expr, Ident, ObjectName, TableConstraint}; use sqlparser_derive::{Visit, VisitMut}; use crate::statements::OptionMap; -use crate::statements::create::{Json2Options, Partitions}; +use crate::statements::create::{Column, Json2Options, Partitions}; #[derive(Debug, Clone, PartialEq, Eq, Visit, VisitMut, Serialize)] pub struct AlterTable { @@ -229,18 +229,18 @@ pub enum UnsetIndexOperation { #[derive(Debug, Clone, PartialEq, Eq, Visit, VisitMut, Serialize)] pub struct AddColumn { - pub column_def: ColumnDef, + pub column: Column, pub location: Option, pub add_if_not_exists: bool, } impl Display for AddColumn { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.column)?; if let Some(location) = &self.location { - write!(f, "{} {location}", self.column_def) - } else { - write!(f, "{}", self.column_def) + write!(f, " {location}")?; } + Ok(()) } } diff --git a/src/sql/src/statements/create.rs b/src/sql/src/statements/create.rs index 9bd6c341a65..919c1cd31f9 100644 --- a/src/sql/src/statements/create.rs +++ b/src/sql/src/statements/create.rs @@ -137,7 +137,7 @@ pub struct CreateTable { pub partitions: Option, } -/// Column definition in `CREATE TABLE` statement. +/// Column definition in `CREATE TABLE` and `ALTER TABLE ADD COLUMN`. #[derive(Debug, PartialEq, Eq, Clone, Visit, VisitMut, Serialize)] pub struct Column { /// `ColumnDef` from `sqlparser::ast` @@ -267,10 +267,11 @@ impl Column { && let Some(vector_options) = &self.extensions.vector_options && let Some(dim) = vector_options.get(VECTOR_OPT_DIM) { - return write!(f, "{} VECTOR({})", self.column_def.name, dim); + write!(f, "{} VECTOR({})", self.column_def.name, dim)?; + } else { + write!(f, "{} ", self.column_def.name)?; + write_data_type(f, &self.column_def.data_type)?; } - write!(f, "{} ", self.column_def.name)?; - write_data_type(f, &self.column_def.data_type)?; if let Some(options) = &self.extensions.json2_options { write!(f, "{options}")?; } @@ -771,7 +772,7 @@ mod tests { name: None, option: sqlparser::ast::ColumnOption::Comment("private-value".repeat(8192)), }); - assert!(!vector.to_string().contains("private-value")); + assert!(vector.to_string().contains("private-value")); assert!(!vector.consume_sql_size_budget(&mut 4096)); vector.columns[1].column_def.options.clear(); assert!(vector.consume_sql_size_budget(&mut 4096)); diff --git a/src/sql/src/statements/transform/type_alias.rs b/src/sql/src/statements/transform/type_alias.rs index b2d1803fb37..90157901f3c 100644 --- a/src/sql/src/statements/transform/type_alias.rs +++ b/src/sql/src/statements/transform/type_alias.rs @@ -72,7 +72,7 @@ impl TransformRule for TypeAliasTransformRule { alter_table.alter_operation_mut() { for add_column in add_columns { - replace_type_alias(&mut add_column.column_def.data_type); + replace_type_alias(add_column.column.mut_data_type()); } } } diff --git a/tests/cases/standalone/common/types/json/json2_alter.result b/tests/cases/standalone/common/types/json/json2_alter.result index 153aa9e7902..055ea90346a 100644 --- a/tests/cases/standalone/common/types/json/json2_alter.result +++ b/tests/cases/standalone/common/types/json/json2_alter.result @@ -150,3 +150,90 @@ DROP TABLE application_logs; Affected Rows: 0 +create table json2_alter_add_settings ( + ts timestamp time index +) with ( + 'append_mode' = 'true' +); + +Affected Rows: 0 + +-- ADD COLUMN preserves JSON2 settings and type hints, including for existing rows. +insert into json2_alter_add_settings values (1); + +Affected Rows: 1 + +alter table json2_alter_add_settings add column j json2(max_auto_expanded_paths = 100); + +Affected Rows: 0 + +alter table json2_alter_add_settings add column k json2(service string); + +Affected Rows: 0 + +alter table json2_alter_add_settings add column m json2(max_auto_expanded_paths = 0, nested.value bigint); + +Affected Rows: 0 + +show create table json2_alter_add_settings; + ++--------------------------+---------------------------------------------------------+ +| Table | Create Table | ++--------------------------+---------------------------------------------------------+ +| json2_alter_add_settings | CREATE TABLE IF NOT EXISTS "json2_alter_add_settings" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "j" JSON2( | +| | max_auto_expanded_paths = 100 | +| | ) NULL, | +| | "k" JSON2( | +| | max_auto_expanded_paths = 100, | +| | "service" STRING | +| | ) NULL, | +| | "m" JSON2( | +| | max_auto_expanded_paths = 0, | +| | "nested"."value" BIGINT | +| | ) NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | WITH( | +| | append_mode = 'true' | +| | ) | ++--------------------------+---------------------------------------------------------+ + +insert into json2_alter_add_settings values + (2, '{"a":1}', '{"service":"api"}', '{"nested":{"value":42},"extra":true}'); + +Affected Rows: 1 + +select ts, j, k.service, m.nested.value, m from json2_alter_add_settings order by ts; + ++-------------------------+---------+-----------------------------------------------------------------------+-------------------------------------------------------------------------+--------------------------------------+ +| ts | j | json_get(json2_alter_add_settings.k,Utf8("$.service"),Utf8View(NULL)) | json_get(json2_alter_add_settings.m,Utf8("$.nested.value"),Int64(NULL)) | m | ++-------------------------+---------+-----------------------------------------------------------------------+-------------------------------------------------------------------------+--------------------------------------+ +| 1970-01-01T00:00:00.001 | | | | | +| 1970-01-01T00:00:00.002 | {"a":1} | api | 42 | {"extra":true,"nested":{"value":42}} | ++-------------------------+---------+-----------------------------------------------------------------------+-------------------------------------------------------------------------+--------------------------------------+ + +admin flush_table('json2_alter_add_settings'); + ++-----------------------------------------------+ +| ADMIN flush_table('json2_alter_add_settings') | ++-----------------------------------------------+ +| 0 | ++-----------------------------------------------+ + +select ts, j, k.service, m.nested.value, m from json2_alter_add_settings order by ts; + ++-------------------------+---------+-----------------------------------------------------------------------+-------------------------------------------------------------------------+--------------------------------------+ +| ts | j | json_get(json2_alter_add_settings.k,Utf8("$.service"),Utf8View(NULL)) | json_get(json2_alter_add_settings.m,Utf8("$.nested.value"),Int64(NULL)) | m | ++-------------------------+---------+-----------------------------------------------------------------------+-------------------------------------------------------------------------+--------------------------------------+ +| 1970-01-01T00:00:00.001 | | | | | +| 1970-01-01T00:00:00.002 | {"a":1} | api | 42 | {"extra":true,"nested":{"value":42}} | ++-------------------------+---------+-----------------------------------------------------------------------+-------------------------------------------------------------------------+--------------------------------------+ + +drop table json2_alter_add_settings; + +Affected Rows: 0 + diff --git a/tests/cases/standalone/common/types/json/json2_alter.sql b/tests/cases/standalone/common/types/json/json2_alter.sql index 457943e93f6..83b514f6895 100644 --- a/tests/cases/standalone/common/types/json/json2_alter.sql +++ b/tests/cases/standalone/common/types/json/json2_alter.sql @@ -74,3 +74,30 @@ ORDER BY ts; ADMIN FLUSH_TABLE('application_logs'); DROP TABLE application_logs; + +create table json2_alter_add_settings ( + ts timestamp time index +) with ( + 'append_mode' = 'true' +); + +-- ADD COLUMN preserves JSON2 settings and type hints, including for existing rows. +insert into json2_alter_add_settings values (1); +alter table json2_alter_add_settings add column j json2(max_auto_expanded_paths = 100); + +alter table json2_alter_add_settings add column k json2(service string); + +alter table json2_alter_add_settings add column m json2(max_auto_expanded_paths = 0, nested.value bigint); + +show create table json2_alter_add_settings; + +insert into json2_alter_add_settings values + (2, '{"a":1}', '{"service":"api"}', '{"nested":{"value":42},"extra":true}'); + +select ts, j, k.service, m.nested.value, m from json2_alter_add_settings order by ts; + +admin flush_table('json2_alter_add_settings'); + +select ts, j, k.service, m.nested.value, m from json2_alter_add_settings order by ts; + +drop table json2_alter_add_settings; diff --git a/tests/cases/standalone/common/types/json/json2_limit.result b/tests/cases/standalone/common/types/json/json2_limit.result index 32bbfbaf0a7..555e76f1e5e 100644 --- a/tests/cases/standalone/common/types/json/json2_limit.result +++ b/tests/cases/standalone/common/types/json/json2_limit.result @@ -95,27 +95,6 @@ drop table json2_alter_non_append; Affected Rows: 0 -create table json2_alter_add_settings ( - ts timestamp time index -) with ( - 'append_mode' = 'true' -); - -Affected Rows: 0 - --- ALTER TABLE ADD COLUMN does not yet support JSON2 settings or type hints. -alter table json2_alter_add_settings add column j json2(max_auto_expanded_paths = 100); - -Error: 2000(InvalidSyntax), Invalid SQL syntax: sql parser error: Expected: type modifiers, found: = at Line: 1, Column: 81 - -alter table json2_alter_add_settings add column k json2(service string); - -Error: 2000(InvalidSyntax), SQL data type not supported yet: Custom(ObjectName([Identifier(Ident { value: "json2", quote_style: None, span: Span(Location(1,51)..Location(1,56)) })]), ["service", "string"]) - -drop table json2_alter_add_settings; - -Affected Rows: 0 - create table json2_set_append_mode_false ( ts timestamp time index, j json2 diff --git a/tests/cases/standalone/common/types/json/json2_limit.sql b/tests/cases/standalone/common/types/json/json2_limit.sql index 1bbadfd5adc..653fdcec12c 100644 --- a/tests/cases/standalone/common/types/json/json2_limit.sql +++ b/tests/cases/standalone/common/types/json/json2_limit.sql @@ -58,19 +58,6 @@ alter table json2_alter_non_append add column j json2; drop table json2_alter_non_append; -create table json2_alter_add_settings ( - ts timestamp time index -) with ( - 'append_mode' = 'true' -); - --- ALTER TABLE ADD COLUMN does not yet support JSON2 settings or type hints. -alter table json2_alter_add_settings add column j json2(max_auto_expanded_paths = 100); - -alter table json2_alter_add_settings add column k json2(service string); - -drop table json2_alter_add_settings; - create table json2_set_append_mode_false ( ts timestamp time index, j json2