Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(query): table field data type in show create table incorrect #17112

Merged
merged 4 commits into from
Dec 26, 2024
+132 −40
Merged

fix(query): table field data type in show create table incorrect #17112

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7be9d38
fix show table
forsaken628 Dec 25, 2024
37628b5
sql_name_explicit_null
forsaken628 Dec 25, 2024
fe87b05
fix
forsaken628 Dec 25, 2024
d58b894
fix
forsaken628 Dec 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 121 additions & 11 deletions src/query/expression/src/schema.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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}");
forsaken628 marked this conversation as resolved.
Show resolved Hide resolved

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
Open in desktop
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
forsaken628 marked this conversation as resolved.
Show resolved Hide resolved

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
Loading