Skip to content

Commit

Permalink
fix(query): table field data type in show create table incorrect (#โ€ฆ
Browse files Browse the repository at this point in the history 
โ€ฆ17112)

* fix show table

* sql_name_explicit_null

* fix

* fix
  • Loading branch information
@forsaken628
forsaken628 authored Dec 26, 2024
1 parent 6eea9b4 commit 89aa80e
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 40 deletions.
132 changes: 121 additions & 11 deletions src/query/expression/src/schema.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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());
}
}
}
16 changes: 4 additions & 12 deletions src/query/service/src/interpreters/interpreter_dictionary_show_create.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
18 changes: 4 additions & 14 deletions src/query/service/src/interpreters/interpreter_table_show_create.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down Expand Up @@ -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_recursive_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);
}
Expand Down
6 changes: 3 additions & 3 deletions tests/sqllogictests/suites/base/05_ddl/05_0003_ddl_alter_table.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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`
Expand All @@ -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`
Expand Down

0 comments on commit 89aa80e

Please sign in to comment.