From 7be9d382f746f177fbb8695ba94d18390767644a Mon Sep 17 00:00:00 2001 From: coldWater Date: Wed, 25 Dec 2024 11:27:20 +0800 Subject: [PATCH 1/4] fix show table --- .../service/src/interpreters/interpreter_table_show_create.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/query/service/src/interpreters/interpreter_table_show_create.rs b/src/query/service/src/interpreters/interpreter_table_show_create.rs index 1b69d3ef4f815..f2e96c0382965 100644 --- a/src/query/service/src/interpreters/interpreter_table_show_create.rs +++ b/src/query/service/src/interpreters/interpreter_table_show_create.rs @@ -204,7 +204,7 @@ impl ShowCreateTableInterpreter { let column_str = format!( " {} {}{}{}{}{}", display_ident(field.name(), quoted_ident_case_sensitive, sql_dialect), - field.data_type().remove_recursive_nullable().sql_name(), + field.data_type().remove_nullable().sql_name(), nullable, default_expr, computed_expr, From 37628b544d9e86f32b6dd8c31ed63a88901c02f4 Mon Sep 17 00:00:00 2001 From: coldWater Date: Wed, 25 Dec 2024 13:51:20 +0800 Subject: [PATCH 2/4] sql_name_explicit_null --- src/query/expression/src/schema.rs | 132 ++++++++++++++++-- .../interpreter_dictionary_show_create.rs | 16 +-- .../interpreter_table_show_create.rs | 18 +-- 3 files changed, 129 insertions(+), 37 deletions(-) diff --git a/src/query/expression/src/schema.rs b/src/query/expression/src/schema.rs index 9801a06500564..e14304408aaa6 100644 --- a/src/query/expression/src/schema.rs +++ b/src/query/expression/src/schema.rs @@ -1285,23 +1285,79 @@ impl TableDataType { pub fn sql_name(&self) -> String { match self { TableDataType::Number(num_ty) => match num_ty { - NumberDataType::UInt8 => "TINYINT UNSIGNED".to_string(), - NumberDataType::UInt16 => "SMALLINT UNSIGNED".to_string(), - NumberDataType::UInt32 => "INT UNSIGNED".to_string(), - NumberDataType::UInt64 => "BIGINT UNSIGNED".to_string(), - NumberDataType::Int8 => "TINYINT".to_string(), - NumberDataType::Int16 => "SMALLINT".to_string(), - NumberDataType::Int32 => "INT".to_string(), - NumberDataType::Int64 => "BIGINT".to_string(), - NumberDataType::Float32 => "FLOAT".to_string(), - NumberDataType::Float64 => "DOUBLE".to_string(), - }, + NumberDataType::UInt8 => "TINYINT UNSIGNED", + NumberDataType::UInt16 => "SMALLINT UNSIGNED", + NumberDataType::UInt32 => "INT UNSIGNED", + NumberDataType::UInt64 => "BIGINT UNSIGNED", + NumberDataType::Int8 => "TINYINT", + NumberDataType::Int16 => "SMALLINT", + NumberDataType::Int32 => "INT", + NumberDataType::Int64 => "BIGINT", + NumberDataType::Float32 => "FLOAT", + NumberDataType::Float64 => "DOUBLE", + } + .to_string(), TableDataType::String => "VARCHAR".to_string(), TableDataType::Nullable(inner_ty) => format!("{} NULL", inner_ty.sql_name()), _ => self.to_string().to_uppercase(), } } + pub fn sql_name_explicit_null(&self) -> String { + fn name(ty: &TableDataType, is_null: bool) -> String { + let s = match ty { + TableDataType::Null => return "NULL".to_string(), + TableDataType::Nullable(inner_ty) => return name(inner_ty, true), + TableDataType::Array(inner) => { + format!("ARRAY({})", name(inner, false)) + } + TableDataType::Map(inner) => match inner.as_ref() { + TableDataType::Tuple { fields_type, .. } => { + format!( + "MAP({}, {})", + name(&fields_type[0], false), + name(&fields_type[1], false) + ) + } + _ => unreachable!(), + }, + TableDataType::Tuple { + fields_name, + fields_type, + } => { + format!( + "TUPLE({})", + fields_name + .iter() + .zip(fields_type) + .map(|(n, ty)| format!("{n} {}", name(ty, false))) + .join(", ") + ) + } + TableDataType::EmptyArray + | TableDataType::EmptyMap + | TableDataType::Number(_) + | TableDataType::String + | TableDataType::Boolean + | TableDataType::Binary + | TableDataType::Decimal(_) + | TableDataType::Timestamp + | TableDataType::Date + | TableDataType::Bitmap + | TableDataType::Variant + | TableDataType::Geometry + | TableDataType::Geography + | TableDataType::Interval => ty.sql_name(), + }; + if is_null { + format!("{} NULL", s) + } else { + format!("{} NOT NULL", s) + } + } + name(self, false) + } + // Returns the number of leaf columns of the TableDataType pub fn num_leaf_columns(&self) -> usize { match self { @@ -1535,3 +1591,57 @@ pub fn create_test_complex_schema() -> TableSchema { field1, field2, field3, field4, field5, field6, field7, field8, ]) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_sql_name_with_null() { + for (expected, data_type) in [ + ("NULL", TableDataType::Null), + ( + "ARRAY(NOTHING) NULL", + TableDataType::EmptyArray.wrap_nullable(), + ), + ( + "BIGINT UNSIGNED NOT NULL", + TableDataType::Number(NumberDataType::UInt64), + ), + ( + "VARCHAR NULL", + TableDataType::Nullable(Box::new(TableDataType::String)), + ), + ( + "ARRAY(VARCHAR NOT NULL) NOT NULL", + TableDataType::Array(Box::new(TableDataType::String)), + ), + ( + "MAP(BIGINT UNSIGNED NOT NULL, VARCHAR NOT NULL) NOT NULL", + TableDataType::Map(Box::new(TableDataType::Tuple { + fields_name: vec!["key".to_string(), "value".to_string()], + fields_type: vec![ + TableDataType::Number(NumberDataType::UInt64), + TableDataType::String, + ], + })), + ), + ( + "TUPLE(a INT NULL, b INT NOT NULL) NOT NULL", + TableDataType::Tuple { + fields_name: vec!["a".to_string(), "b".to_string()], + fields_type: vec![ + TableDataType::Nullable( + TableDataType::Number(NumberDataType::Int32).into(), + ), + TableDataType::Number(NumberDataType::Int32), + ], + }, + ), + ] + .iter() + { + assert_eq!(expected, &&data_type.sql_name_explicit_null()); + } + } +} diff --git a/src/query/service/src/interpreters/interpreter_dictionary_show_create.rs b/src/query/service/src/interpreters/interpreter_dictionary_show_create.rs index 13a0fef5ce52d..1b43a58c68ace 100644 --- a/src/query/service/src/interpreters/interpreter_dictionary_show_create.rs +++ b/src/query/service/src/interpreters/interpreter_dictionary_show_create.rs @@ -126,23 +126,15 @@ impl ShowCreateDictionaryInterpreter { { let mut create_defs = vec![]; for field in schema.fields().iter() { - let nullable = if field.is_nullable() { - " NULL".to_string() - } else { - " NOT NULL".to_string() - }; // compatibility: creating table in the old planner will not have `fields_comments` let comment = field_comments .get(&field.column_id) .and_then(|c| format!(" COMMENT '{}'", c).into()) .unwrap_or_default(); - let column_str = format!( - " {} {}{}{}", - display_ident(field.name(), quoted_ident_case_sensitive, sql_dialect), - field.data_type().remove_recursive_nullable().sql_name(), - nullable, - comment - ); + + let ident = display_ident(field.name(), quoted_ident_case_sensitive, sql_dialect); + let data_type = field.data_type().sql_name_explicit_null(); + let column_str = format!(" {ident} {data_type}{comment}",); create_defs.push(column_str); } diff --git a/src/query/service/src/interpreters/interpreter_table_show_create.rs b/src/query/service/src/interpreters/interpreter_table_show_create.rs index f2e96c0382965..ab227c57310bf 100644 --- a/src/query/service/src/interpreters/interpreter_table_show_create.rs +++ b/src/query/service/src/interpreters/interpreter_table_show_create.rs @@ -169,11 +169,6 @@ impl ShowCreateTableInterpreter { { let mut create_defs = vec![]; for (idx, field) in schema.fields().iter().enumerate() { - let nullable = if field.is_nullable() { - " NULL".to_string() - } else { - " NOT NULL".to_string() - }; let default_expr = match field.default_expr() { Some(expr) => { format!(" DEFAULT {expr}") @@ -201,15 +196,10 @@ impl ShowCreateTableInterpreter { } else { "".to_string() }; - let column_str = format!( - " {} {}{}{}{}{}", - display_ident(field.name(), quoted_ident_case_sensitive, sql_dialect), - field.data_type().remove_nullable().sql_name(), - nullable, - default_expr, - computed_expr, - comment - ); + let ident = display_ident(field.name(), quoted_ident_case_sensitive, sql_dialect); + let data_type = field.data_type().sql_name_explicit_null(); + let column_str = + format!(" {ident} {data_type}{default_expr}{computed_expr}{comment}"); create_defs.push(column_str); } From fe87b05edd03e2268e44485031658116bca69d06 Mon Sep 17 00:00:00 2001 From: coldWater Date: Wed, 25 Dec 2024 16:19:46 +0800 Subject: [PATCH 3/4] fix --- .../suites/base/05_ddl/05_0003_ddl_alter_table.test | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/sqllogictests/suites/base/05_ddl/05_0003_ddl_alter_table.test b/tests/sqllogictests/suites/base/05_ddl/05_0003_ddl_alter_table.test index 4f260ccfc7034..42efa89bc1ad1 100644 --- a/tests/sqllogictests/suites/base/05_ddl/05_0003_ddl_alter_table.test +++ b/tests/sqllogictests/suites/base/05_ddl/05_0003_ddl_alter_table.test @@ -130,7 +130,7 @@ INSERT INTO TABLE `05_0003_at_t4` values('a', 'b', ['c1', 'c2'], ('d1', 'd2')) query TT SHOW CREATE TABLE `05_0003_at_t4` ---- -05_0003_at_t4 CREATE TABLE "05_0003_at_t4" ( a VARCHAR NOT NULL, b VARCHAR NULL, c ARRAY(STRING) NULL, d TUPLE(1 STRING, 2 STRING) NULL ) ENGINE=FUSE +05_0003_at_t4 CREATE TABLE "05_0003_at_t4" ( a VARCHAR NOT NULL, b VARCHAR NULL, c ARRAY(VARCHAR NULL) NULL, d TUPLE(1 VARCHAR NULL, 2 VARCHAR NULL) NULL ) ENGINE=FUSE query TTTT SELECT * FROM `05_0003_at_t4` @@ -152,7 +152,7 @@ ALTER TABLE `05_0003_at_t4` MODIFY COLUMN d tuple(binary, binary) null query TT SHOW CREATE TABLE `05_0003_at_t4` ---- -05_0003_at_t4 CREATE TABLE "05_0003_at_t4" ( a BINARY NOT NULL, b BINARY NULL, c ARRAY(BINARY) NULL, d TUPLE(1 BINARY, 2 BINARY) NULL ) ENGINE=FUSE +05_0003_at_t4 CREATE TABLE "05_0003_at_t4" ( a BINARY NOT NULL, b BINARY NULL, c ARRAY(BINARY NULL) NULL, d TUPLE(1 BINARY NULL, 2 BINARY NULL) NULL ) ENGINE=FUSE query SELECT * FROM `05_0003_at_t4` @@ -174,7 +174,7 @@ ALTER TABLE `05_0003_at_t4` MODIFY COLUMN d tuple(string, string) null query TT SHOW CREATE TABLE `05_0003_at_t4` ---- -05_0003_at_t4 CREATE TABLE "05_0003_at_t4" ( a VARCHAR NOT NULL, b VARCHAR NULL, c ARRAY(STRING) NULL, d TUPLE(1 STRING, 2 STRING) NULL ) ENGINE=FUSE +05_0003_at_t4 CREATE TABLE "05_0003_at_t4" ( a VARCHAR NOT NULL, b VARCHAR NULL, c ARRAY(VARCHAR NULL) NULL, d TUPLE(1 VARCHAR NULL, 2 VARCHAR NULL) NULL ) ENGINE=FUSE query TTTT SELECT * FROM `05_0003_at_t4` From d58b894c3c72b9887361630a0ac690530632df71 Mon Sep 17 00:00:00 2001 From: coldWater Date: Thu, 26 Dec 2024 00:16:12 +0800 Subject: [PATCH 4/4] fix --- .../suites/base/05_ddl/05_0003_ddl_alter_table.test | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/sqllogictests/suites/base/05_ddl/05_0003_ddl_alter_table.test b/tests/sqllogictests/suites/base/05_ddl/05_0003_ddl_alter_table.test index 42efa89bc1ad1..267532e8e5321 100644 --- a/tests/sqllogictests/suites/base/05_ddl/05_0003_ddl_alter_table.test +++ b/tests/sqllogictests/suites/base/05_ddl/05_0003_ddl_alter_table.test @@ -130,7 +130,7 @@ INSERT INTO TABLE `05_0003_at_t4` values('a', 'b', ['c1', 'c2'], ('d1', 'd2')) query TT SHOW CREATE TABLE `05_0003_at_t4` ---- -05_0003_at_t4 CREATE TABLE "05_0003_at_t4" ( a VARCHAR NOT NULL, b VARCHAR NULL, c ARRAY(VARCHAR NULL) NULL, d TUPLE(1 VARCHAR NULL, 2 VARCHAR NULL) NULL ) ENGINE=FUSE +05_0003_at_t4 CREATE TABLE "05_0003_at_t4" ( a VARCHAR NOT NULL, b VARCHAR NULL, c ARRAY(VARCHAR NULL) NULL, d TUPLE(1 VARCHAR NULL, 2 VARCHAR NULL) NULL ) ENGINE=FUSE query TTTT SELECT * FROM `05_0003_at_t4` @@ -152,7 +152,7 @@ ALTER TABLE `05_0003_at_t4` MODIFY COLUMN d tuple(binary, binary) null query TT SHOW CREATE TABLE `05_0003_at_t4` ---- -05_0003_at_t4 CREATE TABLE "05_0003_at_t4" ( a BINARY NOT NULL, b BINARY NULL, c ARRAY(BINARY NULL) NULL, d TUPLE(1 BINARY NULL, 2 BINARY NULL) NULL ) ENGINE=FUSE +05_0003_at_t4 CREATE TABLE "05_0003_at_t4" ( a BINARY NOT NULL, b BINARY NULL, c ARRAY(BINARY NULL) NULL, d TUPLE(1 BINARY NULL, 2 BINARY NULL) NULL ) ENGINE=FUSE query SELECT * FROM `05_0003_at_t4` @@ -174,7 +174,7 @@ ALTER TABLE `05_0003_at_t4` MODIFY COLUMN d tuple(string, string) null query TT SHOW CREATE TABLE `05_0003_at_t4` ---- -05_0003_at_t4 CREATE TABLE "05_0003_at_t4" ( a VARCHAR NOT NULL, b VARCHAR NULL, c ARRAY(VARCHAR NULL) NULL, d TUPLE(1 VARCHAR NULL, 2 VARCHAR NULL) NULL ) ENGINE=FUSE +05_0003_at_t4 CREATE TABLE "05_0003_at_t4" ( a VARCHAR NOT NULL, b VARCHAR NULL, c ARRAY(VARCHAR NULL) NULL, d TUPLE(1 VARCHAR NULL, 2 VARCHAR NULL) NULL ) ENGINE=FUSE query TTTT SELECT * FROM `05_0003_at_t4`