From 3bf8b9722d42e9a5d34c4efdac21c49f9c707383 Mon Sep 17 00:00:00 2001 From: fys <40801205+fengys1996@users.noreply.github.com> Date: Wed, 8 Jul 2026 11:33:36 +0800 Subject: [PATCH] feat(json2): validate append mode for tables with JSON2 columns (#8434) * feat: validate append mode for JSON2 tables * fix: cr by codex * fix: unit test * fix: validate JSON2 append_mode for all ALTER TABLE operations --- src/operator/src/statement/ddl.rs | 57 ++++++++++++++++++- tests-integration/src/tests/instance_test.rs | 6 +- .../stats_schema_mismatch_regression.result | 2 + .../stats_schema_mismatch_regression.sql | 2 + .../standalone/common/types/json/json2.result | 49 ++++++++++++++++ .../standalone/common/types/json/json2.sql | 33 +++++++++++ .../common/types/json/json2_type_hints.result | 8 ++- .../common/types/json/json2_type_hints.sql | 4 ++ 8 files changed, 157 insertions(+), 4 deletions(-) diff --git a/src/operator/src/statement/ddl.rs b/src/operator/src/statement/ddl.rs index 3f644f5f64..9deaba6d58 100644 --- a/src/operator/src/statement/ddl.rs +++ b/src/operator/src/statement/ddl.rs @@ -84,6 +84,7 @@ use sql::statements::create::{ use sql::statements::statement::Statement; use sqlparser::ast::{Expr, Ident, UnaryOperator, Value as ParserValue}; use store_api::metric_engine_consts::{LOGICAL_TABLE_METADATA_KEY, METRIC_ENGINE_NAME}; +use store_api::mito_engine_options::APPEND_MODE_KEY; use substrait::{DFLogicalSubstraitConvertor, SubstraitPlan}; use table::TableRef; use table::dist_table::DistTable; @@ -2273,13 +2274,15 @@ pub fn verify_alter( } } - let _ = table_info + let new_meta = table_info .meta .builder_with_alter_kind(table_name, &request.alter_kind) .context(error::TableSnafu)? .build() .context(error::BuildTableMetaSnafu { table_name })?; + validate_json2_columns_append_mode(&new_meta.schema, &new_meta.options)?; + Ok(true) } @@ -2328,6 +2331,8 @@ pub fn create_table_info( let mut table_options = TableOptions::try_from_iter(&create_table.table_options) .context(UnrecognizedTableOptionSnafu)?; + validate_json2_columns_append_mode(&schema, &table_options)?; + validate_repartition_column_hint( &mut table_options, &column_name_to_index_map, @@ -2370,6 +2375,33 @@ pub fn create_table_info( Ok(table_info) } +fn validate_json2_columns_append_mode(schema: &Schema, table_options: &TableOptions) -> Result<()> { + let append_mode = table_options + .extra_options + .get(APPEND_MODE_KEY) + .is_some_and(|value| value == "true"); + + for column in schema.column_schemas() { + if column + .data_type + .as_json() + .is_some_and(|json_type| json_type.is_json2()) + { + ensure!( + append_mode, + InvalidSqlSnafu { + err_msg: format!( + "JSON2 column `{}` requires {}='true'", + column.name, APPEND_MODE_KEY + ), + } + ); + } + } + + Ok(()) +} + fn validate_repartition_column_hint( table_options: &mut TableOptions, column_name_to_index_map: &HashMap, @@ -2993,6 +3025,29 @@ WITH ('repartition.column.hint' = ' host ')", ); } + #[test] + fn test_json2_requires_append_mode() { + let cases = [ + "CREATE TABLE monitor (payload JSON2, ts TIMESTAMP TIME INDEX);", + "CREATE TABLE monitor (payload JSON2, ts TIMESTAMP TIME INDEX) WITH (append_mode='false');", + ]; + + for sql in cases { + let expr = create_expr_from_sql(sql); + let err = create_table_info(&expr, vec![]).unwrap_err(); + assert!( + err.to_string() + .contains("JSON2 column `payload` requires append_mode='true'"), + "{err}" + ); + } + + let expr = create_expr_from_sql( + "CREATE TABLE monitor (payload JSON2, ts TIMESTAMP TIME INDEX) WITH (append_mode='true');", + ); + create_table_info(&expr, vec![]).unwrap(); + } + #[test] fn test_create_table_with_empty_repartition_column_hint() { let expr = create_expr_from_sql( diff --git a/tests-integration/src/tests/instance_test.rs b/tests-integration/src/tests/instance_test.rs index 12aa145dca..fa9bad0fac 100644 --- a/tests-integration/src/tests/instance_test.rs +++ b/tests-integration/src/tests/instance_test.rs @@ -3003,7 +3003,7 @@ async fn test_create_table_with_json_datatype(instance: Arc) { CREATE TABLE b ( j JSON2, ts TIMESTAMP TIME INDEX, -)"#; +) WITH (append_mode='true')"#; let output = execute_sql(&instance, sql).await.data; assert!(matches!(output, OutputData::AffectedRows(0))); @@ -3019,7 +3019,9 @@ CREATE TABLE b ( | | ) | | | | | | ENGINE=mito | -| | | +| | WITH( | +| | append_mode = 'true' | +| | ) | +-------+----------------------------------+"#; check_output_stream(output, expected).await; } diff --git a/tests/cases/standalone/common/promql/stats_schema_mismatch_regression.result b/tests/cases/standalone/common/promql/stats_schema_mismatch_regression.result index f376dfe59c..483a1201d5 100644 --- a/tests/cases/standalone/common/promql/stats_schema_mismatch_regression.result +++ b/tests/cases/standalone/common/promql/stats_schema_mismatch_regression.result @@ -126,6 +126,8 @@ CREATE TABLE promql_instant_mismatch_nested ( ts TIMESTAMP(3) TIME INDEX, k STRING PRIMARY KEY, v JSON2, +) WITH ( + 'append_mode' = 'true' ); Affected Rows: 0 diff --git a/tests/cases/standalone/common/promql/stats_schema_mismatch_regression.sql b/tests/cases/standalone/common/promql/stats_schema_mismatch_regression.sql index db69796e74..cdc3a68931 100644 --- a/tests/cases/standalone/common/promql/stats_schema_mismatch_regression.sql +++ b/tests/cases/standalone/common/promql/stats_schema_mismatch_regression.sql @@ -108,6 +108,8 @@ CREATE TABLE promql_instant_mismatch_nested ( ts TIMESTAMP(3) TIME INDEX, k STRING PRIMARY KEY, v JSON2, +) WITH ( + 'append_mode' = 'true' ); -- TODO(LFC): Uncomment the following SQLs and results when JSON2 is ready. diff --git a/tests/cases/standalone/common/types/json/json2.result b/tests/cases/standalone/common/types/json/json2.result index d397966c21..9c1ac1ec4b 100644 --- a/tests/cases/standalone/common/types/json/json2.result +++ b/tests/cases/standalone/common/types/json/json2.result @@ -230,11 +230,43 @@ drop table json2_table; Affected Rows: 0 +create table json2_without_append_mode ( + ts timestamp time index, + j json2 +); + +Error: 1004(InvalidArguments), Invalid SQL, error: JSON2 column `j` requires append_mode='true' + +create table json2_append_mode_false ( + ts timestamp time index, + j json2 +) with ( + 'append_mode' = 'false' +); + +Error: 1004(InvalidArguments), Invalid SQL, error: JSON2 column `j` requires append_mode='true' + +create table json2_alter_non_append ( + ts timestamp time index +); + +Affected Rows: 0 + +alter table json2_alter_non_append add column j json2; + +Error: 1004(InvalidArguments), Invalid SQL, error: JSON2 column `j` requires append_mode='true' + +drop table json2_alter_non_append; + +Affected Rows: 0 + create table json2_default_null_ok ( ts timestamp time index, j json2( a int64 null default null ) +) with ( + 'append_mode' = 'true' ); Affected Rows: 0 @@ -252,3 +284,20 @@ create table json2_default_null_check ( Error: 2000(InvalidSyntax), Invalid SQL, error: invalid DEFAULT for JSON2 type hint 'a': Default value should not be null for non null column +create table json2_set_append_mode_false ( + ts timestamp time index, + j json2 +) with ( + 'append_mode' = 'true' +); + +Affected Rows: 0 + +alter table json2_set_append_mode_false set 'append_mode' = 'false'; + +Error: 1004(InvalidArguments), Invalid SQL, error: JSON2 column `j` requires append_mode='true' + +drop table json2_set_append_mode_false; + +Affected Rows: 0 + diff --git a/tests/cases/standalone/common/types/json/json2.sql b/tests/cases/standalone/common/types/json/json2.sql index e13f861e22..81cd8d4020 100644 --- a/tests/cases/standalone/common/types/json/json2.sql +++ b/tests/cases/standalone/common/types/json/json2.sql @@ -71,11 +71,33 @@ select j.d from json2_table order by ts; drop table json2_table; +create table json2_without_append_mode ( + ts timestamp time index, + j json2 +); + +create table json2_append_mode_false ( + ts timestamp time index, + j json2 +) with ( + 'append_mode' = 'false' +); + +create table json2_alter_non_append ( + ts timestamp time index +); + +alter table json2_alter_non_append add column j json2; + +drop table json2_alter_non_append; + create table json2_default_null_ok ( ts timestamp time index, j json2( a int64 null default null ) +) with ( + 'append_mode' = 'true' ); drop table json2_default_null_ok; @@ -86,3 +108,14 @@ create table json2_default_null_check ( a int64 not null default null ) ); + +create table json2_set_append_mode_false ( + ts timestamp time index, + j json2 +) with ( + 'append_mode' = 'true' +); + +alter table json2_set_append_mode_false set 'append_mode' = 'false'; + +drop table json2_set_append_mode_false; diff --git a/tests/cases/standalone/common/types/json/json2_type_hints.result b/tests/cases/standalone/common/types/json/json2_type_hints.result index f8b5d661a0..45565c56fb 100644 --- a/tests/cases/standalone/common/types/json/json2_type_hints.result +++ b/tests/cases/standalone/common/types/json/json2_type_hints.result @@ -6,6 +6,8 @@ CREATE TABLE json2_type_hints ( user.active BOOLEAN NULL, score DOUBLE NULL DEFAULT 1.5 ) +) WITH ( + 'append_mode' = 'true' ); Affected Rows: 0 @@ -27,7 +29,9 @@ SHOW CREATE TABLE json2_type_hints; | | ) | | | | | | ENGINE=mito | -| | | +| | WITH( | +| | append_mode = 'true' | +| | ) | +------------------+--------------------------------------------------+ INSERT INTO json2_type_hints @@ -64,6 +68,8 @@ CREATE TABLE json2_type_hints_required ( j JSON2 ( user.age BIGINT NOT NULL ) +) WITH ( + 'append_mode' = 'true' ); Affected Rows: 0 diff --git a/tests/cases/standalone/common/types/json/json2_type_hints.sql b/tests/cases/standalone/common/types/json/json2_type_hints.sql index 80dcc49e70..4d3e5a39d1 100644 --- a/tests/cases/standalone/common/types/json/json2_type_hints.sql +++ b/tests/cases/standalone/common/types/json/json2_type_hints.sql @@ -6,6 +6,8 @@ CREATE TABLE json2_type_hints ( user.active BOOLEAN NULL, score DOUBLE NULL DEFAULT 1.5 ) +) WITH ( + 'append_mode' = 'true' ); SHOW CREATE TABLE json2_type_hints; @@ -32,6 +34,8 @@ CREATE TABLE json2_type_hints_required ( j JSON2 ( user.age BIGINT NOT NULL ) +) WITH ( + 'append_mode' = 'true' ); INSERT INTO json2_type_hints_required